rkhachatryan commented on a change in pull request #17774:
URL: https://github.com/apache/flink/pull/17774#discussion_r767851024
##########
File path:
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/snapshot/RocksIncrementalSnapshotStrategy.java
##########
@@ -258,30 +268,37 @@ private SnapshotDirectory
prepareLocalSnapshotDirectory(long checkpointId) throw
}
Review comment:
Aborted **files** can be re-used, but not `StateHandleID`s held by
`materializedSstFiles` (because JM might not remember these keys on abortion).
> edit: actually I think the logic maybe is correct, but something feels
inconsistent here. It's really hard for me to name what is inside
materializedSstFiles. Those are confirmed files and also those awaiting
confirmation. But aborted files are being removed from it.
Right. This logic is the same as before. The purpose of
`notifyCheckpointAborted` is purely cleanup. It's called from state backend, so
it's not dead code.
The purpose of `lastUploadedSstFiles` is not to handle abortion:
- if a file was not confirmed for any reason, then `StreamStateHandle` must
be sent
- if it was, then a placeholder can be sent
--
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
