zhangyue19921010 commented on a change in pull request #3956:
URL: https://github.com/apache/hudi/pull/3956#discussion_r746260110
##########
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:
In this patch, we delete the rollback instant in the `for (String
instant : instantsToRollback)` loop.
Is there may something un-deleted when going into
https://github.com/apache/hudi/blob/add8acecb367ee7b8506e5abda9137d203e34645/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/AbstractHoodieWriteClient.java#L855
branch ?
Maybe we could do the delete HeartbeatsFiles out of the loop :)
##########
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(),
Review comment:
Did we need to remove these unused function like
`getAllExistingHeartbeatInstants` and `cleanExpiredHeartbeats` after this patch?
--
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]
