StefanRRichter commented on a change in pull request #7674: [FLINK-10043]
[State Backends] Refactor RocksDBKeyedStateBackend object
construction/initialization/restore code
URL: https://github.com/apache/flink/pull/7674#discussion_r255936225
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/state/filesystem/FsStateBackend.java
##########
@@ -458,7 +460,8 @@ public CheckpointStorage createCheckpointStorage(JobID
jobId) throws IOException
KeyGroupRange keyGroupRange,
TaskKvStateRegistry kvStateRegistry,
TtlTimeProvider ttlTimeProvider,
- MetricGroup metricGroup) {
+ MetricGroup metricGroup,
+ Collection<KeyedStateHandle> stateHandles) {
Review comment:
This looks like an inconsistency now: in the case of RocksDBStateBackend, we
use the `stateHandles` to restore the backend, so the backend returned by this
method is already setup with the state. However, this method ignores the
parameter here. I would suggest that all backends should work the same and that
we always have a fully configured and restored backend returned by this method.
This means that we should also get rid of the
`restore(Collection<KeyedStateHandle> restoreState)` method, which is called
anyways right after the backend was built. So instead i suggest that the
restore happens in the builder as well, because that also should make disposing
resources on restore failure easier.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
