rkhachatryan commented on code in PR #19679:
URL: https://github.com/apache/flink/pull/19679#discussion_r904958544
##########
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBKeyedStateBackend.java:
##########
@@ -477,6 +478,50 @@ KeyGroupedInternalPriorityQueue<T> create(
}
}
+ @Override
+ public <N, S extends State, V> S upgradeKeyedState(
+ TypeSerializer<N> namespaceSerializer, StateDescriptor<S, V>
stateDescriptor)
+ throws Exception {
+ StateFactory stateFactory = getStateFactory(stateDescriptor);
+ Tuple2<ColumnFamilyHandle, RegisteredKeyValueStateBackendMetaInfo<N,
V>> registerResult =
+ tryRegisterKvStateInformation(stateDescriptor,
namespaceSerializer, noTransform());
+
Preconditions.checkState(kvStateInformation.containsKey(stateDescriptor.getName()));
+ kvStateInformation.computeIfPresent(
+ stateDescriptor.getName(),
+ (stateName, kvStateInfo) ->
+ new RocksDbKvStateInfo(
+ kvStateInfo.columnFamilyHandle,
+ new RegisteredKeyValueStateBackendMetaInfo<>(
+ kvStateInfo.metaInfo.snapshot())));
+ return stateFactory.createState(
+ stateDescriptor, registerResult,
RocksDBKeyedStateBackend.this);
Review Comment:
1. I have the following concerns regarding re-creating state objects:
a) Old objects might leak some resources
b) New objects might not inherit all the "meta' state (such as
currentNamespace, changelogger.metadataFlag), or even state updates
c) Re-creating state objects might not be enough; in particular, for
HeapStateBackend we might also want to update the unerlying
`CopyOnWriteStateMap`
Probably, those are not immediate issue per se, but at the very least they
make updating serializers error-prone for future changes.
2. Good point, I think unexpected updates and migrations can be prevented by
formalizing the lifecycle of state objects (and checking whether the current
phase allows migration or not)
Maybe we don't need to make the fields non-final, but make the lifecycle
explicit, e.g. by adding `RocksDbValueState.withNewSerializer(...)` if it works
better.
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]
