umehrot2 commented on a change in pull request #3956:
URL: https://github.com/apache/hudi/pull/3956#discussion_r750833083
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/AbstractHoodieWriteClient.java
##########
@@ -856,13 +856,9 @@ protected void rollbackFailedWrites(List<String>
instantsToRollback, boolean ski
} else {
rollback(instant, skipLocking);
}
- }
- // Delete any heartbeat files for already rolled back commits
- try {
-
HeartbeatUtils.cleanExpiredHeartbeats(this.heartbeatClient.getAllExistingHeartbeatInstants(),
- createMetaClient(true), basePath);
- } catch (IOException io) {
- LOG.error("Unable to delete heartbeat files", io);
+
+ // Delete the heartbeat for rolled back instant
+ HeartbeatUtils.deleteHeartbeatFile(fs, basePath, instant, config);
Review comment:
Moving it outside the loop has the risk that if there are multiple
commits to rollback, and if for some reason rollback succeeds for earlier
commits but fails for a commit later in the list, then the job would not clear
the heartbeat files even for commits that were rolled back correctly. Its best
to also clear the heartbeat as soon as the commit is rolled back, so other
commits do not have any affect on this. Also in terms of just time, moving it
outside the loop could create a time lag between when a commit is rolled back
and when its heartbeat file is deleted.
--
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]
