[GitHub] flink pull request #5945: [FLINK-9169] [runtime] Allow KeyedBackendSerializa...

[tzulitai <gi...@git.apache.org>]

GitHub user tzulitai opened a pull request:

    https://github.com/apache/flink/pull/5945

    [FLINK-9169] [runtime] Allow KeyedBackendSerializationProxy to specify whether serializer presence is required

    ## What is the purpose of the change
    
    Previously, in both the RocksDB and heap state backends, on restore of a savepoint if the previous state serializer can not be read, the restore will fail.
    
    This is a must in the heap side, but not for RocksDB, since the restore can still proceed with the new serializer (given that the serializer is compatible).
    This PR relaxes the requirement of serializer presence at restore time for RocksDB.
    
    ## Brief change log
    
    - Let `KeyedBackendSerializationProxy` allow specifying a flag whether or not serializer presence is strictly required when restoring the keyed backend. For heap backends, this flag would be `true`, while for RocksDB this flag is `false`.
    - `TypeSerializerSerializationUtil.tryReadSerializer(...)` now always returns a `UnloadableDummyTypeSerializer` if the `useDummyPlaceholder` is set to `true` and an exception occurred (regardless of what exception) while reading serializers. It only returns `null` if `useDummyPlaceholder` is `false`. This flag corresponds to the serializer presence flag mentioned above.
    
    ## Verifying this change
    
    There are already a few existing tests related to this issue:
    - `MemoryStateBackendTest.testKeyedStateRestoreFailsIfSerializerDeserializationFails`
    - `SerializationProxiesTest.testKeyedBackendSerializationProxyRoundtripWithSerializerSerializationFailures`
    
    What may still be missing is an e2e test, that verifies RocksDB can restore correctly even if a previous serializer class is no longer in the classpath (i.e. is replaced by a new compatible serializer of a different class).
    
    ## Does this pull request potentially affect one of the following parts:
    
      - Dependencies (does it add or upgrade a dependency): (yes / **no**)
      - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes / **no**)
      - The serializers: (yes / **no** / don't know)
      - The runtime per-record code paths (performance sensitive): (yes / **no** / don't know)
      - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (**yes** / no / don't know)
      - The S3 file system connector: (yes / **no** / don't know)
    
    ## Documentation
    
      - Does this pull request introduce a new feature? (yes / **no**)
      - If yes, how is the feature documented? (**not applicable** / docs / JavaDocs / not documented)


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/tzulitai/flink FLINK-9169

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/5945.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #5945
    
----
commit 0eb2918a7551094b712173e332df7f2663ecf0cc
Author: Tzu-Li (Gordon) Tai <tz...@...>
Date:   2018-05-02T05:37:22Z

    [FLINK-9169] [runtime] Allow KeyedBackendSerializationProxy to specify whether serializer presence is required

----


---

[GitHub] flink pull request #5945: [FLINK-9169] [runtime] Allow KeyedBackendSerializa...

[tzulitai <gi...@git.apache.org>]

Github user tzulitai closed the pull request at:

    https://github.com/apache/flink/pull/5945


---

[GitHub] flink issue #5945: [FLINK-9169] [runtime] Allow KeyedBackendSerializationPro...

[tzulitai <gi...@git.apache.org>]

Github user tzulitai commented on the issue:

    https://github.com/apache/flink/pull/5945
  
    I've opened a new PR #5950 which has a cleaner approach to this.
    That new PR subsumes this one.


---