Uploaded image for project: 'Flink'
  1. Flink
  2. FLINK-6922

Enum(Value)SerializerConfigSnapshot uses Java serialization to store enum values

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.3.0, 1.4.0
    • Fix Version/s: 1.3.1
    • Labels:
      None

      Description

      The EnumSerializer and the EnumValueSerializer (Scala) uses Java serialization to store enum values. From a user perspective this can be a bit of a problem if the user forgot to add a serial version UID. I think it would be better to store the enum information without Java serialization to avoid this problem.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user tzulitai opened a pull request:

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

          FLINK-6922 [serde] Remove Java serialization from Enum(Value)SerializerConfigSnapshot

          This PR subsumes #4142.

          It additionally removes Java serialization from the Java `EnumSerializerConfigSnapshot` and `EnumValueSerializerConfigSnapshot`, which was in the way for some compatible migration paths (e.g. adding constants to the enm, but not setting a serialVersionUID for the enum class).

          This PR also ports the `EnumValueSerializerUpgradeTest` for the Java `EnumSerializer`, with the only difference that `testDifferentFieldOrder` should not require migration on the Java side (the Java `EnumSerializer` reconfigures its enum constant ordering).

          The original `o.a.f.api.common.typeutils.base.EnumSerializerTest` already covers more sophisticated ordering tests for the `EnumSerializer`, as well as snapshot + compatibility check idempotency. Hence, there are no new tests for that. The ported `o.a.f.api.common.typeutils.base.EnumSerializerUpgradeTest` serves as an end-to-end behavioural test.

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

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

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

          https://github.com/apache/flink/pull/4147.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 #4147


          commit 9a86e4b78dba3d2dd9d09702c1fa684fabac70b0
          Author: Till Rohrmann <trohrmann@apache.org>
          Date: 2017-06-14T14:53:49Z

          FLINK-6921 [serializer] Allow EnumValueSerializer to deal with appended enum values

          The problem was that we don't check the bounds of the array with the enum names contained
          in the ScalaEnumSerializerConfigSnapshot.

          This PR also adds an Enumeration upgrade test which makes sure that appended fields are
          supported without migration. Moreover, it checks that a field removal and an order change
          leads to a required migration.

          This closes #4126.

          commit 126dcc828b2fca1cdb16c228b5ad8483d970e106
          Author: Till Rohrmann <trohrmann@apache.org>
          Date: 2017-06-19T10:49:05Z

          FLINK-6948 Harden EnumValueSerializer to detect changed enum indices

          This PR changes the seriailization format of the ScalaEnumSerializerConfigSnapshot to also include the
          ordinal value of an enum value when being deserialized. This allows to detect if the ordinal values
          have been changed and, thus, if migration is required.

          IMPORTANT: This PR changes the serialization format of ScalaEnumSerializerConfigSnapshot.

          Remove backwards compatibility path for 1.3.1

          commit 07d468b88b31741961d20c665a13b735fecd6b73
          Author: Tzu-Li (Gordon) Tai <tzulitai@apache.org>
          Date: 2017-06-20T10:17:21Z

          FLINK-6922 [serde] Remove Java serialization from Enum(Value)SerializerConfigSnapshot

          This commit removes the use of Java serialization for serializing the
          enum class and constants in the Java EnumSerializerConfigSnapshot and
          Scala ScalaEnumSerializerConfigSnapshot.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user tzulitai opened a pull request: https://github.com/apache/flink/pull/4147 FLINK-6922 [serde] Remove Java serialization from Enum(Value)SerializerConfigSnapshot This PR subsumes #4142. It additionally removes Java serialization from the Java `EnumSerializerConfigSnapshot` and `EnumValueSerializerConfigSnapshot`, which was in the way for some compatible migration paths (e.g. adding constants to the enm, but not setting a serialVersionUID for the enum class). This PR also ports the `EnumValueSerializerUpgradeTest` for the Java `EnumSerializer`, with the only difference that `testDifferentFieldOrder` should not require migration on the Java side (the Java `EnumSerializer` reconfigures its enum constant ordering). The original `o.a.f.api.common.typeutils.base.EnumSerializerTest` already covers more sophisticated ordering tests for the `EnumSerializer`, as well as snapshot + compatibility check idempotency. Hence, there are no new tests for that. The ported `o.a.f.api.common.typeutils.base.EnumSerializerUpgradeTest` serves as an end-to-end behavioural test. You can merge this pull request into a Git repository by running: $ git pull https://github.com/tzulitai/flink FLINK-6922 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/4147.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 #4147 commit 9a86e4b78dba3d2dd9d09702c1fa684fabac70b0 Author: Till Rohrmann <trohrmann@apache.org> Date: 2017-06-14T14:53:49Z FLINK-6921 [serializer] Allow EnumValueSerializer to deal with appended enum values The problem was that we don't check the bounds of the array with the enum names contained in the ScalaEnumSerializerConfigSnapshot. This PR also adds an Enumeration upgrade test which makes sure that appended fields are supported without migration. Moreover, it checks that a field removal and an order change leads to a required migration. This closes #4126. commit 126dcc828b2fca1cdb16c228b5ad8483d970e106 Author: Till Rohrmann <trohrmann@apache.org> Date: 2017-06-19T10:49:05Z FLINK-6948 Harden EnumValueSerializer to detect changed enum indices This PR changes the seriailization format of the ScalaEnumSerializerConfigSnapshot to also include the ordinal value of an enum value when being deserialized. This allows to detect if the ordinal values have been changed and, thus, if migration is required. IMPORTANT: This PR changes the serialization format of ScalaEnumSerializerConfigSnapshot. Remove backwards compatibility path for 1.3.1 commit 07d468b88b31741961d20c665a13b735fecd6b73 Author: Tzu-Li (Gordon) Tai <tzulitai@apache.org> Date: 2017-06-20T10:17:21Z FLINK-6922 [serde] Remove Java serialization from Enum(Value)SerializerConfigSnapshot This commit removes the use of Java serialization for serializing the enum class and constants in the Java EnumSerializerConfigSnapshot and Scala ScalaEnumSerializerConfigSnapshot.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/4147
          Hide
          tzulitai Tzu-Li (Gordon) Tai added a comment -

          Fixed for master via f24a499b37515b43789ae23d83c1cd9eb2f827b3.
          Fixed for 1.3.1 via 732d9c14252ebaded32b4d96cb25926e83254892.

          Show
          tzulitai Tzu-Li (Gordon) Tai added a comment - Fixed for master via f24a499b37515b43789ae23d83c1cd9eb2f827b3. Fixed for 1.3.1 via 732d9c14252ebaded32b4d96cb25926e83254892.

            People

            • Assignee:
              tzulitai Tzu-Li (Gordon) Tai
              Reporter:
              till.rohrmann Till Rohrmann
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development