rkhachatryan commented on code in PR #19679:
URL: https://github.com/apache/flink/pull/19679#discussion_r902807955


##########
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:
   I see the following options:
   1. Create new objects (as it is done currently in the PR)
   2. Facilitate upgrade by state objects (e.g. `RocksDBValueState`) - i.e. add 
methods to update the serializer (and make the field mutable)
   3. Use getter (`RegisteredKeyValueStateBackendMetaInfo.getStateSerializer`) 
in state objects (e.g. ` RocksDBValueState`) instead of injecting it
   4. Wrap serializer to delegate all accesses to it and allow updating the 
actual serializer "externally" (similar to what was initially described in the 
ticket)
   
   2nd option looks better to me because it is explicit, more simple than 
others and doesn't incur any overhead.
   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]

Reply via email to