codope commented on code in PR #9259:
URL: https://github.com/apache/hudi/pull/9259#discussion_r1270750658
##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/client/functional/TestHoodieClientOnCopyOnWriteStorage.java:
##########
@@ -2736,8 +2738,12 @@ public void
testClusteringCommitInPresenceOfInflightCommit() throws Exception {
assertThrows(HoodieClusteringException.class, () ->
clusteringWriteClient.cluster(clusteringCommitTime, true));
// Do a rollback on the replacecommit that is failed
- clusteringWriteClient.rollback(clusteringCommitTime);
+ // clusteringWriteClient.rollback(clusteringCommitTime);
Review Comment:
as per my understanding, there should not be a need to rollback the
clustering commit explicitly.. it should be done automatically if a conflict is
detected by the new strategy. But, both replacecommit inflight and requested
remain in the timeline.
##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/client/functional/TestHoodieClientOnCopyOnWriteStorage.java:
##########
@@ -2713,6 +2713,8 @@ public void
testClusteringCommitInPresenceOfInflightCommit() throws Exception {
// Schedule and execute a clustering plan on the same partition. During
conflict resolution the commit should fail.
HoodieClusteringConfig clusteringConfig =
HoodieClusteringConfig.newBuilder().withClusteringMaxNumGroups(10)
.withClusteringTargetPartitions(0).withInlineClusteringNumCommits(1)
+
.withClusteringUpdatesStrategy("org.apache.hudi.client.clustering.update.strategy.SparkAllowUpdateStrategy")
+ .withRollbackPendingClustering(true)
Review Comment:
this change can be ignored.. i was trying to check whether explicitly
setting rollback pending clustering reverts the inflight replacecommit or not.
But, this config does not have any effect.
--
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]
