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

Narrow down interface for compatibility hook method in VersionedIOReadableWritable

    Details

      Description

      The VersionedIOReadableWritable provides a isCompatibleVersion method that allows users to override to resolve older versions.

      This method allows too much space for the user to mess up the implementation, and is much more relevant now because we have an user-facing class TypeSerializerConfigSnapshot which extends VersionedIOReadableWritable.

      Instead of a isCompatibleVersion method, it should only expose a narrower int[] getCompatibleVersions that the internal version check uses.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user tzulitai opened a pull request:

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

          FLINK-6566 [core] More restricted interface for VersionedIOReadableWritable hooks

          This PR makes the method hooks for defining compatible
          serialization versions of `VersionedIOReadableWritable`s more restricted.

          Functionally everything remains the same, but with lesser space for
          error-prone user implementations. It also allows for a better error
          message to indicate version mismatch.

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

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

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

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


          commit ac35ce3632ae9b3f15b5eafe9df99c399e5bde5d
          Author: Tzu-Li (Gordon) Tai <tzulitai@apache.org>
          Date: 2017-05-12T12:06:01Z

          FLINK-6566 [core] More restricted interface for VersionedIOReadableWritable hooks

          This commit makes the method hooks for defining compatibile
          serialization versions of VersionedIOReadableWritables more restricted.

          Functionally everything remains the same, but with lesser space for
          error-prone user implementations. It also allows for a better error
          message to indicate version mismatch.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user tzulitai opened a pull request: https://github.com/apache/flink/pull/3883 FLINK-6566 [core] More restricted interface for VersionedIOReadableWritable hooks This PR makes the method hooks for defining compatible serialization versions of `VersionedIOReadableWritable`s more restricted. Functionally everything remains the same, but with lesser space for error-prone user implementations. It also allows for a better error message to indicate version mismatch. You can merge this pull request into a Git repository by running: $ git pull https://github.com/tzulitai/flink FLINK-6566 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3883.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 #3883 commit ac35ce3632ae9b3f15b5eafe9df99c399e5bde5d Author: Tzu-Li (Gordon) Tai <tzulitai@apache.org> Date: 2017-05-12T12:06:01Z FLINK-6566 [core] More restricted interface for VersionedIOReadableWritable hooks This commit makes the method hooks for defining compatibile serialization versions of VersionedIOReadableWritables more restricted. Functionally everything remains the same, but with lesser space for error-prone user implementations. It also allows for a better error message to indicate version mismatch.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/flink/pull/3883#discussion_r116249086

          — Diff: flink-core/src/main/java/org/apache/flink/core/io/VersionedIOReadableWritable.java —
          @@ -18,55 +18,66 @@

          package org.apache.flink.core.io;

          -import org.apache.flink.annotation.PublicEvolving;
          +import org.apache.flink.annotation.Internal;
          import org.apache.flink.core.memory.DataInputView;
          import org.apache.flink.core.memory.DataOutputView;

          import java.io.IOException;
          +import java.util.Arrays;

          /**

          • This is the abstract base class for {@link IOReadableWritable}

            which allows to differentiate between serialization

          • versions. Concrete subclasses should typically override the {@link #write(DataOutputView)}

            and

          • {@link #read(DataInputView)}

            , thereby calling super to ensure version checking.
            */
            -@PublicEvolving
            +@Internal
            public abstract class VersionedIOReadableWritable implements IOReadableWritable, Versioned {

          + private Integer readVersion;
          — End diff –

          I think I would not use `Integer` here but `int` instead and have some undefined value, e.g. `Integer.MIN_VALUE`

          Show
          githubbot ASF GitHub Bot added a comment - Github user StefanRRichter commented on a diff in the pull request: https://github.com/apache/flink/pull/3883#discussion_r116249086 — Diff: flink-core/src/main/java/org/apache/flink/core/io/VersionedIOReadableWritable.java — @@ -18,55 +18,66 @@ package org.apache.flink.core.io; -import org.apache.flink.annotation.PublicEvolving; +import org.apache.flink.annotation.Internal; import org.apache.flink.core.memory.DataInputView; import org.apache.flink.core.memory.DataOutputView; import java.io.IOException; +import java.util.Arrays; /** This is the abstract base class for {@link IOReadableWritable} which allows to differentiate between serialization versions. Concrete subclasses should typically override the {@link #write(DataOutputView)} and {@link #read(DataInputView)} , thereby calling super to ensure version checking. */ -@PublicEvolving +@Internal public abstract class VersionedIOReadableWritable implements IOReadableWritable, Versioned { + private Integer readVersion; — End diff – I think I would not use `Integer` here but `int` instead and have some undefined value, e.g. `Integer.MIN_VALUE`
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tzulitai commented on the issue:

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

          Will address and merge this!

          Show
          githubbot ASF GitHub Bot added a comment - Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3883 Will address and merge this!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

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

          Fixed for master via 347100de7527dc4ba3664b8e8306a081834f84a4.
          Fixed for 1.3 via 1ac5f8421cb289c5e52172f1ecc854e1c9252b23.

          Show
          tzulitai Tzu-Li (Gordon) Tai added a comment - Fixed for master via 347100de7527dc4ba3664b8e8306a081834f84a4. Fixed for 1.3 via 1ac5f8421cb289c5e52172f1ecc854e1c9252b23.

            People

            • Assignee:
              tzulitai Tzu-Li (Gordon) Tai
              Reporter:
              tzulitai Tzu-Li (Gordon) Tai
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development