Zakelly commented on code in PR #26202:
URL: https://github.com/apache/flink/pull/26202#discussion_r1979297416
##########
flink-state-backends/flink-statebackend-forst/src/main/java/org/apache/flink/state/forst/ForStKeyedStateBackendBuilder.java:
##########
@@ -259,10 +259,14 @@ public ForStKeyedStateBackend<K> build() throws
BackendBuildingException {
defaultColumnFamilyHandle =
restoreResult.getDefaultColumnFamilyHandle();
nativeMetricMonitor = restoreResult.getNativeMetricMonitor();
- // TODO: init materializedSstFiles and lastCompletedCheckpointId
when implement restore
SortedMap<Long,
Collection<IncrementalKeyedStateHandle.HandleAndLocalPath>>
materializedSstFiles = new TreeMap<>();
long lastCompletedCheckpointId = -1L;
+ if (restoreOperation instanceof ForStIncrementalRestoreOperation) {
+ backendUID = restoreResult.getBackendUID();
+ materializedSstFiles = restoreResult.getRestoredSstFiles();
+ lastCompletedCheckpointId =
restoreResult.getLastCompletedCheckpointId();
+ }
Review Comment:
> > But your approach should also be feasible; it just differs from the
definition of responsibility boundaries between modules in RocksDB.
>
> It is strange to me that, in here we record the files from previous
snapshot but these has been copied, so these original ones are meaningless to
current DB instance or following checkpoints. It would be fine if we keep the
same logic as RocksDB, but I'd suggest clear it right away if the copy strategy
is applied.
It makes more sense to split it into inherit and clear part. WDYT?
--
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]
