[jira] [Commented] (FLINK-8841) Duplicate MapSerializer and HashMapSerializer.

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/FLINK-8841?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452115#comment-16452115 ] 

ASF GitHub Bot commented on FLINK-8841:
---------------------------------------

GitHub user kl0u opened a pull request:

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

    [FLINK-8841] [state] Remove HashMapSerializer and use MapSerializer instead.

    ## What is the purpose of the change
    
    So far we had the `MapSerializer` and the `HashMapSerializer`. The two had almost identical code and the second was only used on the `HeapStateBackend`/`FSStateBackend` when creating a `MapState`. This PR removes the `HashMapSerializer` and replaces its uses with the `MapSerializer`. It also guarantees backwards compatibility.
    
    ## Brief change log
    
    It introduces the `MigrationUtil` as an inner class of the `InstantiationUtil`. This class contains mapping between deprecated/deleted serializers and their replacements.
    
    Also the removal of the `HashMapSerializer` uniformizes a bit the `HeapMapState` and the `RocksDBMapState`.
    
    ## Verifying this change
    
    Added the `HeapKeyedStateBackendSnapshotMigrationTest#testMapStateMigrationAfterHashMapSerRemoval()`.
    
    ## Does this pull request potentially affect one of the following parts:
    
      - Dependencies (does it add or upgrade a dependency): no
      - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: no
      - The serializers: yes
      - The runtime per-record code paths (performance sensitive): no
      - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: no
      - The S3 file system connector: no
    
    ## Documentation
    
      - Does this pull request introduce a new feature? no
      - If yes, how is the feature documented? not applicable
    
    R @StefanRRichter or @aljoscha 


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

    $ git pull https://github.com/kl0u/flink map-serializer-inv

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

    https://github.com/apache/flink/pull/5910.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 #5910
    
----
commit 7de83de4765384080cea6d94b64a81e1584ce82e
Author: kkloudas <kk...@...>
Date:   2018-04-24T12:48:34Z

    [FLINK-8841] Remove HashMapSerializer and use MapSerializer instead.

----


> Duplicate MapSerializer and HashMapSerializer.
> ----------------------------------------------
>
>                 Key: FLINK-8841
>                 URL: https://issues.apache.org/jira/browse/FLINK-8841
>             Project: Flink
>          Issue Type: Bug
>          Components: State Backends, Checkpointing, Type Serialization System
>    Affects Versions: 1.5.0
>            Reporter: Kostas Kloudas
>            Assignee: Kostas Kloudas
>            Priority: Major
>             Fix For: 1.5.0
>
>
> Currently there are two class MapSerializer and HashMapSerializer whose code is the same with the only difference being that one includes elements of type Map and the other HashMap. 
> In addition, these two were merged on the same commit. 
> I would like to remove the HashMapSerializer. I already created a branch without the HashMapSerialzer and nothing seems to be failing on Travis. The reasons why I hesitate to do it, is because I am not sure if this may create problems with Backwards Compatibility.
> [~xiaogang.sxg] could you elaborate a bit on why they were both added and if there is any danger in removing the HashMapSerializer?
> Also [~StephanEwen] and [~stefanrichter83@gmail.com] it is worth having a look and if you are ok, I can remove the redundant serializer.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)