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

User code ClassLoader not used when KryoSerializer fallbacks to serialization for copying

    Details

      Description

      Reported in ML: http://apache-flink-user-mailing-list-archive.2336050.n4.nabble.com/AWS-exception-serialization-problem-td12063.html

      This is caused by a known Kryo issue with its JavaSerializer: https://github.com/EsotericSoftware/kryo/pull/483.

      It happens when a Throwable is to be copied by the KryoSerialzer. Since we use the JavaSerializer for throwables, and JavaSerializer doesn't support copying, the KryoSerializer fallbacks to use de-/serialization for the throwable. The problem is that on deserialization, the classloader that the ObjectInputStream uses may be overriden, and doesn't specifically uses Kryo's configured classloader (i.e., the user code class loader), and results in ClassNotFoundException.

      Generally, this may happen if the user also registers to use the JavaSerializer for their types.

      To fix the problem for Throwable serializing in the KryoSerializer, we could either consider registering our own fixed JavaSerializer for throwables, or wait for the Kryo fix to be released (to be fixed in Kryo 4.0.1 release).

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user tzulitai opened a pull request:

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

          FLINK-6025 [core] Add Flink's own JavaSerializer for Kryo serialization

          (This PR should also be backported for `release-1.1` and `release-1.2`. Separate PRs are not opened for the backports because it is a relatively self-contained change.)

          This PR adds a reimplemented `JavaSerializer` to be registered with
          Kryo. This is due to a know issue with Kryo's `JavaSerializer` that may
          use the wrong classloader for deserialzation (see https://github.com/EsotericSoftware/kryo/pull/483).

          Since we by default register Kryo's `JavaSerializer` for `Throwable`s, users who need to serialize exception types as messages bump into `ClassNotFoundException`s.
          Instead of registering Kryo's `JavaSerializer` for `Throwables`, it is now changed to register the reimplemented `JavaSerializer`.

          Generally, users who bump into `ClassNotFoundException`s if they are using Kryo's `JavaSerializer` for
          their own custom types are also recommended to change to Flink's `JavaSerializer`.

          The change is tested using the minimal issue reproducing example provided by our users in http://apache-flink-user-mailing-list-archive.2336050.n4.nabble.com/AWS-exception-serialization-problem-td12063.html.

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

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

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

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


          commit 0e85ec1bf0a11c67e2af3625e0858e22b41600cd
          Author: Tzu-Li (Gordon) Tai <tzulitai@apache.org>
          Date: 2017-03-12T14:46:27Z

          FLINK-6025 [core] Add Flink's own JavaSerializer for Kryo serialization

          This commit adds a reimplemented JavaSerializer to be registered with
          Kryo. This is due to a know issue with Kryo's JavaSerializer that may
          use the wrong classloader for deserialzation.

          Instead of registering Kryo's JavaSerializer for Throwables, it is now
          changed to register the reimplemented JavaSerializer. Users who bump
          into ClassNotFoundExceptions if they are using Kryo's JavaSerializer for
          their own types are also recommended to change to Flink's JavaSerializer.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user tzulitai opened a pull request: https://github.com/apache/flink/pull/3517 FLINK-6025 [core] Add Flink's own JavaSerializer for Kryo serialization (This PR should also be backported for `release-1.1` and `release-1.2`. Separate PRs are not opened for the backports because it is a relatively self-contained change.) This PR adds a reimplemented `JavaSerializer` to be registered with Kryo. This is due to a know issue with Kryo's `JavaSerializer` that may use the wrong classloader for deserialzation (see https://github.com/EsotericSoftware/kryo/pull/483 ). Since we by default register Kryo's `JavaSerializer` for `Throwable`s, users who need to serialize exception types as messages bump into `ClassNotFoundException`s. Instead of registering Kryo's `JavaSerializer` for `Throwables`, it is now changed to register the reimplemented `JavaSerializer`. Generally, users who bump into `ClassNotFoundException`s if they are using Kryo's `JavaSerializer` for their own custom types are also recommended to change to Flink's `JavaSerializer`. The change is tested using the minimal issue reproducing example provided by our users in http://apache-flink-user-mailing-list-archive.2336050.n4.nabble.com/AWS-exception-serialization-problem-td12063.html . You can merge this pull request into a Git repository by running: $ git pull https://github.com/tzulitai/flink FLINK-6025 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3517.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 #3517 commit 0e85ec1bf0a11c67e2af3625e0858e22b41600cd Author: Tzu-Li (Gordon) Tai <tzulitai@apache.org> Date: 2017-03-12T14:46:27Z FLINK-6025 [core] Add Flink's own JavaSerializer for Kryo serialization This commit adds a reimplemented JavaSerializer to be registered with Kryo. This is due to a know issue with Kryo's JavaSerializer that may use the wrong classloader for deserialzation. Instead of registering Kryo's JavaSerializer for Throwables, it is now changed to register the reimplemented JavaSerializer. Users who bump into ClassNotFoundExceptions if they are using Kryo's JavaSerializer for their own types are also recommended to change to Flink's JavaSerializer.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user tzulitai opened a pull request:

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

          [backport-1.2] FLINK-6025 [core] Add Flink's own JavaSerializer for Kryo serialization

          This is a backport of #3517 for `release-1.2`.

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

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

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

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


          commit 888940847254b48252bc1587944d8c0f6089447f
          Author: Tzu-Li (Gordon) Tai <tzulitai@apache.org>
          Date: 2017-03-12T14:46:27Z

          FLINK-6025 [core] Add Flink's own JavaSerializer for Kryo serialization

          This commit adds a reimplemented JavaSerializer to be registered with
          Kryo. This is due to a know issue with Kryo's JavaSerializer that may
          use the wrong classloader for deserialzation.

          Instead of registering Kryo's JavaSerializer for Throwables, it is now
          changed to register the reimplemented JavaSerializer. Users who bump
          into ClassNotFoundExceptions if they are using Kryo's JavaSerializer for
          their own types are also recommended to change to Flink's JavaSerializer.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user tzulitai opened a pull request: https://github.com/apache/flink/pull/3518 [backport-1.2] FLINK-6025 [core] Add Flink's own JavaSerializer for Kryo serialization This is a backport of #3517 for `release-1.2`. You can merge this pull request into a Git repository by running: $ git pull https://github.com/tzulitai/flink FLINK-6025 -1.2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3518.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 #3518 commit 888940847254b48252bc1587944d8c0f6089447f Author: Tzu-Li (Gordon) Tai <tzulitai@apache.org> Date: 2017-03-12T14:46:27Z FLINK-6025 [core] Add Flink's own JavaSerializer for Kryo serialization This commit adds a reimplemented JavaSerializer to be registered with Kryo. This is due to a know issue with Kryo's JavaSerializer that may use the wrong classloader for deserialzation. Instead of registering Kryo's JavaSerializer for Throwables, it is now changed to register the reimplemented JavaSerializer. Users who bump into ClassNotFoundExceptions if they are using Kryo's JavaSerializer for their own types are also recommended to change to Flink's JavaSerializer.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user tzulitai opened a pull request:

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

          [backport-1.1] FLINK-6025 [core] Add Flink's own JavaSerializer for Kryo serialization

          This is a backport of #3518 for `release-1.1`.

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

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

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

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


          commit 934ff6713a05f87138dd065460b5e6e67e3477d9
          Author: Tzu-Li (Gordon) Tai <tzulitai@apache.org>
          Date: 2017-03-12T14:46:27Z

          FLINK-6025 [core] Add Flink's own JavaSerializer for Kryo serialization

          This commit adds a reimplemented JavaSerializer to be registered with
          Kryo. This is due to a know issue with Kryo's JavaSerializer that may
          use the wrong classloader for deserialzation.

          Instead of registering Kryo's JavaSerializer for Throwables, it is now
          changed to register the reimplemented JavaSerializer. Users who bump
          into ClassNotFoundExceptions if they are using Kryo's JavaSerializer for
          their own types are also recommended to change to Flink's
          JavaSerializer.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user tzulitai opened a pull request: https://github.com/apache/flink/pull/3519 [backport-1.1] FLINK-6025 [core] Add Flink's own JavaSerializer for Kryo serialization This is a backport of #3518 for `release-1.1`. You can merge this pull request into a Git repository by running: $ git pull https://github.com/tzulitai/flink FLINK-6025 -1.1 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3519.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 #3519 commit 934ff6713a05f87138dd065460b5e6e67e3477d9 Author: Tzu-Li (Gordon) Tai <tzulitai@apache.org> Date: 2017-03-12T14:46:27Z FLINK-6025 [core] Add Flink's own JavaSerializer for Kryo serialization This commit adds a reimplemented JavaSerializer to be registered with Kryo. This is due to a know issue with Kryo's JavaSerializer that may use the wrong classloader for deserialzation. Instead of registering Kryo's JavaSerializer for Throwables, it is now changed to register the reimplemented JavaSerializer. Users who bump into ClassNotFoundExceptions if they are using Kryo's JavaSerializer for their own types are also recommended to change to Flink's JavaSerializer.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user rmetzger commented on the issue:

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

          Good change, +1 to merge

          Show
          githubbot ASF GitHub Bot added a comment - Github user rmetzger commented on the issue: https://github.com/apache/flink/pull/3517 Good change, +1 to merge
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user rmetzger commented on the issue:

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

          +1 to merge

          Show
          githubbot ASF GitHub Bot added a comment - Github user rmetzger commented on the issue: https://github.com/apache/flink/pull/3518 +1 to merge
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user rmetzger commented on the issue:

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

          +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user rmetzger commented on the issue: https://github.com/apache/flink/pull/3519 +1
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tzulitai commented on the issue:

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

          Thank you for the reviews @rmetzger! I'll proceed to merge this and the backport PRs.

          Show
          githubbot ASF GitHub Bot added a comment - Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3517 Thank you for the reviews @rmetzger! I'll proceed to merge this and the backport PRs.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/3517
          Show
          tzulitai Tzu-Li (Gordon) Tai added a comment - Resolved for master with http://git-wip-us.apache.org/repos/asf/flink/commit/f214317 Resolved for release-1.2 with http://git-wip-us.apache.org/repos/asf/flink/commit/b7d288f Resolved for release-1.1 with http://git-wip-us.apache.org/repos/asf/flink/commit/e50bf65
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user rmetzger commented on the issue:

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

          @tzulitai The merge didn't close the PR I think the "this closes #xxx" thing only works for commits to `master`.

          Show
          githubbot ASF GitHub Bot added a comment - Github user rmetzger commented on the issue: https://github.com/apache/flink/pull/3518 @tzulitai The merge didn't close the PR I think the "this closes #xxx" thing only works for commits to `master`.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tzulitai commented on the issue:

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

          Thanks for the reminder :-D

          Show
          githubbot ASF GitHub Bot added a comment - Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3518 Thanks for the reminder :-D
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tzulitai closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user tzulitai closed the pull request at: https://github.com/apache/flink/pull/3518
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tzulitai closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user tzulitai closed the pull request at: https://github.com/apache/flink/pull/3519

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development