rkhachatryan commented on a change in pull request #18382:
URL: https://github.com/apache/flink/pull/18382#discussion_r788780719
##########
File path:
flink-state-backends/flink-statebackend-changelog/src/main/java/org/apache/flink/state/changelog/ChangelogKeyedStateBackend.java
##########
@@ -453,7 +465,10 @@ public void notifyCheckpointAborted(long checkpointId)
throws Exception {
// This might change if the log ownership changes (the method
won't likely be needed).
stateChangelogWriter.reset(lastUploadedFrom, lastUploadedTo);
}
- keyedStateBackend.notifyCheckpointAborted(checkpointId);
+ Long materializationID =
materializationIdByCheckpointId.remove(checkpointId);
+ if (materializationID != null) {
Review comment:
To summarize the offline discussion:
Pros of notifying: some backends might do GC
Cons of notifiying:
- there is low probability of abortion notification reaching backend
(because of difference in intervals and cleanup on checkpoint completion)
- there is low probability of backends make good use of it (because it's
delivered **after** snapshot is done; and backends must be resilient to missing
notifications)
- there is added complexity and risk (please double check pre-marture
notification and cleanup of maps)
I've added it in 862d28e7e2ff899a022d2f2e3da37ceaf149d31f but I'm still not
sure whether it worth it.
PTAL.
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
