suryaprasanna commented on code in PR #8849:
URL: https://github.com/apache/hudi/pull/8849#discussion_r1225863737
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/BaseRollbackActionExecutor.java:
##########
@@ -240,22 +244,21 @@ protected void finishRollback(HoodieInstant
inflightInstant, HoodieRollbackMetad
this.txnManager.beginTransaction(Option.of(inflightInstant),
Option.empty());
}
- // If publish the rollback to the timeline, we first write the rollback
metadata
- // to metadata table
+ // If publish the rollback to the timeline, we first write the rollback
metadata to metadata table
+ // Then transition the inflight rollback to completed state.
if (!skipTimelinePublish) {
writeTableMetadata(rollbackMetadata);
- }
-
- // Then we delete the inflight instant in the data table timeline if
enabled
- deleteInflightAndRequestedInstant(deleteInstants,
table.getActiveTimeline(), resolvedInstant);
-
- // If publish the rollback to the timeline, we finally transition the
inflight rollback
- // to complete in the data table timeline
- if (!skipTimelinePublish) {
table.getActiveTimeline().transitionRollbackInflightToComplete(inflightInstant,
TimelineMetadataUtils.serializeRollbackMetadata(rollbackMetadata));
LOG.info("Rollback of Commits " +
rollbackMetadata.getCommitsRollback() + " is complete");
}
+
+ // Commit to rollback instant files are deleted after the rollback
commit is transitioned from inflight to completed
+ // If job were to fail after transitioning rollback from inflight to
complete and before delete the instant files,
+ // then subsequent retries of the rollback for this instant will see if
there is a completed rollback present for this instant
+ // and then directly delete the files and abort.
Review Comment:
I committed the cleanup logic. Also, added test for this change.
##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/table/action/rollback/TestCopyOnWriteRollbackActionExecutor.java:
##########
@@ -216,7 +228,7 @@ public void
testCopyOnWriteRollbackWithReplaceCommits(boolean isUsingMarkers) th
}
@ParameterizedTest
- @ValueSource(booleans = {true, false})
+ @ValueSource(booleans = {true})
Review Comment:
Reverted this change.
--
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]
