rkhachatryan commented on code in PR #22669: URL: https://github.com/apache/flink/pull/22669#discussion_r1218292582
########## flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/snapshot/RocksDBSnapshotStrategyBase.java: ########## @@ -395,18 +394,16 @@ public void release() { /** Previous snapshot with uploaded sst files. */ protected static class PreviousSnapshot { - @Nullable private final Map<StateHandleID, Long> confirmedSstFiles; + @Nullable private final Map<StateHandleID, StreamStateHandle> confirmedSstFiles; - protected PreviousSnapshot(@Nullable Map<StateHandleID, Long> confirmedSstFiles) { + protected PreviousSnapshot( + @Nullable Map<StateHandleID, StreamStateHandle> confirmedSstFiles) { this.confirmedSstFiles = confirmedSstFiles; } protected Optional<StreamStateHandle> getUploaded(StateHandleID stateHandleID) { if (confirmedSstFiles != null && confirmedSstFiles.containsKey(stateHandleID)) { - // we introduce a placeholder state handle, that is replaced with the - // original from the shared state registry (created from a previous checkpoint) - return Optional.of( - new PlaceholderStreamStateHandle(confirmedSstFiles.get(stateHandleID))); Review Comment: > So I think String is clearer and enough ? I think `String` should work too. > About 'only use PlaceholderStreamStateHandle while the origin handle is ByteStreamStateHandle': In fact I want to suggest developers to do this in the doc of PlaceholderStreamStateHandle and SharedStateRegistry. Perhaps some bugs could have been avoided if developers followed this advice and only performed replacements on PlaceholderStreamStateHandle . I'm not sure if this is over-designed, if so please correct me. I'm sorry but I'm still missing what's the issue with using `PlaceholderStreamStateHandle` for **any** kind of handles? -- 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