dmvk commented on a change in pull request #17693:
URL: https://github.com/apache/flink/pull/17693#discussion_r745478062
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/PendingCheckpoint.java
##########
@@ -336,6 +336,8 @@ public CompletedCheckpoint finalizeCheckpoint(
props,
finalizedLocation);
+ // Mark this pending checkpoint as disposed, but do NOT drop
the state.
+ dispose(false, checkpointsCleaner, postCleanup, executor);
Review comment:
How do you want to ensure this? We already fail any call to
`#cleanupCheckpoint` / `#cleanCheckpointOnFailedStoring` after
CheckpointsCleaner has been closed.
This `dispose()` method you're referring to is private to the
PendingCheckpoint implementation. PendingCheckpoint implementation is the one
that needs to say, "I'm done with the checkpoint and you can treat it as
completed." by completing the checkpoint future. If this class then registers
checkpoint for cleanup (which is basically mutation of the result it has
already returned), then it's IMO a problem with the implementation and not the
CheckpointsCleaner contract.
WDYT?
--
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]
