1996fanrui commented on code in PR #21547:
URL: https://github.com/apache/flink/pull/21547#discussion_r1070226451
##########
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/snapshot/RocksDBSnapshotStrategyBase.java:
##########
@@ -325,6 +329,7 @@ protected RocksDBSnapshotOperation(
this.checkpointStreamFactory = checkpointStreamFactory;
this.stateMetaInfoSnapshots = stateMetaInfoSnapshots;
this.localBackupDirectory = localBackupDirectory;
+ this.tmpResourcesRegistry = new CloseableRegistry();
Review Comment:
Hi @fredia , thanks a lot for your review.
As I understand, the `tmpResourcesRegistry` cannot be reused by multiple
checkpoints. If `tmpResourcesRegistry` is reused, it will be closed after chk-1
timeout, and chk-2 will call `tmpResourcesRegistry.registerCloseable()` after
writing file. However, chk-2 will throw `IOException` even if chk-2 is fine.
Current PR doesn't reuse the `tmpResourcesRegistry` by multiple checkpoints,
the `RocksDBSnapshotOperation` is created for single checkpoint, the
constructor includes the checkpointId.
The `testUploadedSstCanBeCleanedUp` reuses `tmpResourcesRegistry` by
multiple `uploader.uploadFilesToCheckpointFs`, it isn't reused by multiple
checkpoints. I want to check if all files can be cleaned up, no matter the
files written before or after `tmpResourcesRegistry.close`.
Please correct me if I'm wrong, thanks~ 🤔
--
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]
