Github user tzulitai commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5885#discussion_r183309701
  
    --- Diff: 
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBKeyedStateBackend.java
 ---
    @@ -1177,7 +1177,7 @@ private void 
restoreKeyGroupsShardWithTemporaryHelperInstance(
                                throw new StateMigrationException("State 
migration isn't supported, yet.");
                        } else {
                                stateInfo.f1 = newMetaInfo;
    -                           return stateInfo.f0;
    +                           return Tuple2.of(stateInfo.f0, 
newMetaInfo.getStateSerializer());
    --- End diff --
    
    I agree here, that we should let the meta info be immutable, and let the 
compatibility check result carry the compatible, reconfigured serializer.
    
    However, one issue is that this requires changes to the 
`CompatibilityResult` interface which is part of the public API. I would prefer 
not to touch the API now as we're approaching release. It would be possible to 
by-pass this by maybe introducing an internal compat result class, but 
downsides are - 1) that would have almost identical implementation to 
`CompatibilityResult`, and 2) that would entail touching a lot of our more 
complex serializer's code, because they use 
`CompatibilityUtil.resolveCompatibilityResult`.


---

Reply via email to