[GitHub] [hudi] SteNicholas commented on a diff in pull request #7035: [HUDI-5075] Adding support to rollback residual clustering after disabling clustering

[GitBox <gi...@apache.org>]

SteNicholas commented on code in PR #7035:
URL: https://github.com/apache/hudi/pull/7035#discussion_r1006819770


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java:
##########
@@ -588,6 +588,19 @@ protected void runTableServicesInline(HoodieTable table, HoodieCommitMetadata me
         metadata.addMetadata(HoodieClusteringConfig.SCHEDULE_INLINE_CLUSTERING.key(), "true");
         inlineScheduleClustering(extraMetadata);
       }
+
+      // if clustering is disabled, but we might need to rollback any inflight clustering when clustering was enabled previously.
+      if (!config.inlineClusteringEnabled() && !config.isAsyncClusteringEnabled() && !config.scheduleInlineClustering()

Review Comment:
   Could this condition simplify to `config.isRollbackPendingClustering()`? IMO, regardless of whether Clustering is enabled or not, only setting hoodie.rollback.pending.clustering to true, it should rollback any pending clustering, meanwhile, if disable clustering previously, there is no any pending clustering.



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org