rkhachatryan commented on a change in pull request #18514:
URL: https://github.com/apache/flink/pull/18514#discussion_r792455364
##########
File path:
flink-state-backends/flink-statebackend-changelog/src/main/java/org/apache/flink/state/changelog/ChangelogKeyedStateBackend.java
##########
@@ -526,18 +510,16 @@ public void notifyCheckpointAborted(long checkpointId)
throws Exception {
}
Long materializationID =
materializationIdByCheckpointId.remove(checkpointId);
if (materializationID != null) {
- Set<Long> checkpoints =
pendingMaterializationConfirmations.get(materializationID);
- checkpoints.remove(checkpointId);
- if (checkpoints.isEmpty()) {
Review comment:
The purpose of maintaining the `Set` of checkpoints was to not notfiy of
abortion if any of the checkpoints is still not confirmed.
For example, there is
1. single materialization
1. two pending checkpoints using it
1. 1st get's aborted (and notified)
1. later, 2nd get's confirmed (and notified)
In (3), we'd notify of abortion and of completion in (4) - of the same
materialization.
The Set would prevent this.
I don't see how the updated version addresses this. Or am I missing
something?
--
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
