danny0405 commented on a change in pull request #4420:
URL: https://github.com/apache/hudi/pull/4420#discussion_r801254435



##########
File path: 
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java
##########
@@ -464,27 +464,43 @@ protected void postCommit(HoodieTable<T, I, K, O> table, 
HoodieCommitMetadata me
   }
 
   protected void runTableServicesInline(HoodieTable<T, I, K, O> table, 
HoodieCommitMetadata metadata, Option<Map<String, String>> extraMetadata) {
-    if (config.areAnyTableServicesInline()) {
+    if (config.areAnyTableServicesInline() || 
config.scheduleInlineTableServices()) {
       if (config.isMetadataTableEnabled()) {
         table.getHoodieView().sync();
       }
       // Do an inline compaction if enabled
       if (config.inlineCompactionEnabled()) {
         runAnyPendingCompactions(table);
         metadata.addMetadata(HoodieCompactionConfig.INLINE_COMPACT.key(), 
"true");
-        inlineCompact(extraMetadata);
+        inlineScheduleCompactAndOptionallyExecute(extraMetadata, 
!config.scheduleInlineCompaction());

Review comment:
       I think the reason that i feel confusing here is that there is 
assumption:
   `config.scheduleInlineCompaction()` === `only schedule and no execute 
compaction`, that means either we only schedule the compaction or we schedule 
and execute it. These two config options have responsibility overlap.
   
   But from the config option itself, we can not infer such assumption.
   
   What about we config both option inline compaction as true but inline 
schedule as false ? Should we throw exception there ?
   
   Actually, in very early Flink options, we already split the `inline compact` 
option into 2: one for inline scheduling and one for inline execution. They are 
both default as true, people can disable one of them or two, and in general 
,these 2 options are mutually exclusive.
   
   > Infact I did had it that way, on my first attempt. but did not feel the 
need and later removed it.
   
   I still feel that a separate method make things more clear, one method only 
have single responsibility, WDYT ?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to