YuweiXiao commented on code in PR #5681:
URL: https://github.com/apache/hudi/pull/5681#discussion_r882353269


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/compact/ScheduleCompactionActionExecutor.java:
##########
@@ -101,9 +105,14 @@ public Option<HoodieCompactionPlan> execute() {
       } catch (IOException ioe) {
         throw new HoodieIOException("Exception scheduling compaction", ioe);
       }
-      return Option.of(plan);
+      option = Option.of(plan);
     }
-    return Option.empty();
+
+    if (config.isTableManagerEnabled() && 
config.getTableManagerConfig().getTableManagerActions().contains(ActionType.compaction.name()))
 {

Review Comment:
   Maybe we could abstract this as an interface in config, e.g., 
isTableManagerIncludeAction(xxx)



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanActionExecutor.java:
##########
@@ -236,6 +237,11 @@ private HoodieCleanMetadata runClean(HoodieTable<T, I, K, 
O> table, HoodieInstan
 
   @Override
   public HoodieCleanMetadata execute() {
+    if (config.isTableManagerEnabled() && 
config.getTableManagerConfig().getTableManagerActions().contains(ActionType.clean.name()))
 {
+      LOG.warn("Compaction delegate to table management service, do not clean 
for client!");

Review Comment:
   typo, -> 'clean delegate to'



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/cluster/ClusteringPlanActionExecutor.java:
##########
@@ -102,6 +107,22 @@ public Option<HoodieClusteringPlan> execute() {
         throw new HoodieIOException("Exception scheduling clustering", ioe);
       }
     }
+
+    if (config.isTableManagerEnabled() && 
config.getTableManagerConfig().getTableManagerActions().contains(ActionType.replacecommit.name()))
 {
+      submitClusteringToService();
+    }
+
     return planOption;
   }
+
+  private void submitClusteringToService() {
+    HoodieTableMetaClient metaClient = table.getMetaClient();
+    List<String> instantsToSubmit = metaClient.getActiveTimeline()
+        .filterPendingReplaceTimeline()

Review Comment:
   Replace instant may also include `insert overwrite` operation. Will the 
manager service double-check to ensure it is a clustering instant?



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/HoodieWriteMetadata.java:
##########
@@ -37,6 +37,8 @@
 
   // Will be set when auto-commit happens
   private boolean isCommitted;
+  // Will be set when table management service enable
+  private boolean isEmpty;

Review Comment:
   `isEmpty` seems a little bit confusing. How about `isSkipped`?



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanPlanActionExecutor.java:
##########
@@ -152,9 +156,26 @@ protected Option<HoodieCleanerPlan> requestClean(String 
startCleanTime) {
         LOG.error("Got exception when saving cleaner requested file", e);
         throw new HoodieIOException(e.getMessage(), e);
       }
-      return Option.of(cleanerPlan);
+      option = Option.of(cleanerPlan);
     }
-    return Option.empty();
+
+    if (config.isTableManagerEnabled() && 
config.getTableManagerConfig().getTableManagerActions().contains(ActionType.clean.name()))
 {
+      submitCleanToService();
+    }
+
+    return option;
+  }
+
+  private void submitCleanToService() {

Review Comment:
   These submit function will submit all pending clean/compaction/clustering 
instant to the service. What happens if we submit the same instant multiple 
times?



-- 
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