You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by tzulitai <gi...@git.apache.org> on 2017/06/08 13:45:37 UTC

[GitHub] flink pull request #4090: [FLINK-6869] [scala] Specify serialVersionUID for ...

GitHub user tzulitai opened a pull request:

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

    [FLINK-6869] [scala] Specify serialVersionUID for all Scala serializers

    This PR fixes 2 issues:
    
    1. Configuration snapshots of Scala serializers were not readable:
    Prior to this PR, the configuration snapshot classes of Scala serializers did not have the proper default empty constructor that is used for deserializing the configuration snapshot.
    
    Since some Scala serializers' config snapshots extend the Java `CompositeTypeSerializerConfigSnapshot`, their config snapshot classes are also changed to be implemented in Java since in Scala we can only call a single base class constructor from subclasses.
    
    2. Scala serializers did not specify the serialVersionUID:
    Previously, Scala serializers did not specify the `serialVersionUID`, and therefore prohibited restore from previous Flink version snapshots because the serializers' implementations changed in 1.3.
    
    The `serialVersionUID`s added in this PR are identical to what they were (as generated by Java) in Flink 1.2, so that we can at least restore state that were written with the Scala serializers as of 1.2.


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

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

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

    https://github.com/apache/flink/pull/4090.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 #4090
    
----
commit 416bd3b122e79bdd8b5876e8d645b346110b67f0
Author: Tzu-Li (Gordon) Tai <tz...@apache.org>
Date:   2017-06-08T06:52:04Z

    [hotfix] [scala] Fix instantiation of Scala serializers' config snapshot classes
    
    Prior to this commit, the configuration snapshot classes of Scala
    serializers did not have the proper default empty constructor that is
    used for deserializing the configuration snapshot.
    
    Since some Scala serializers' config snapshots extend the Java
    CompositeTypeSerializerConfigSnapshot, their config snapshot classes are
    also changed to be implemented in Java since in Scala we can only call a
    single base class constructor from subclasses.

commit 16574c6623dd64846c888e6a608deb9ae3f081bd
Author: Tzu-Li (Gordon) Tai <tz...@apache.org>
Date:   2017-06-08T13:29:45Z

    [FLINK-6869] [scala] Specify serialVersionUID for all Scala serializers
    
    Previously, Scala serializers did not specify the serialVersionUID, and
    therefore prohibited restore from previous Flink version snapshots
    because the serializers' implementations changed.
    
    The serialVersionUIDs added in this commit are identical to what they
    were (as generated by Java) in Flink 1.2, so that we can at least
    restore state that were written with the Scala serializers as of 1.2.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4090: [FLINK-6869] [scala] Specify serialVersionUID for ...

Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4090#discussion_r121384869
  
    --- Diff: flink-core/src/main/java/org/apache/flink/api/common/typeutils/TypeSerializerSerializationUtil.java ---
    @@ -362,8 +407,14 @@ public void read(DataInputView in) throws IOException {
     			int serializerBytes = in.readInt();
     			byte[] buffer = new byte[serializerBytes];
     			in.readFully(buffer);
    -			try {
    -				typeSerializer = InstantiationUtil.deserializeObject(buffer, userClassLoader);
    +
    +			ClassLoader old = Thread.currentThread().getContextClassLoader();
    --- End diff --
    
    Maybe rename to `previousClassLoader`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #4090: [FLINK-6869] [scala] Specify serialVersionUID for all Sca...

Posted by tzulitai <gi...@git.apache.org>.
Github user tzulitai commented on the issue:

    https://github.com/apache/flink/pull/4090
  
    R: @StefanRRichter @aljoscha tagging you because I talked to you about the issue offline :) Could you have a quick look?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4090: [FLINK-6869] [scala] Specify serialVersionUID for ...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #4090: [FLINK-6869] [scala] Specify serialVersionUID for all Sca...

Posted by tzulitai <gi...@git.apache.org>.
Github user tzulitai commented on the issue:

    https://github.com/apache/flink/pull/4090
  
    One caveat that this PR does not yet fully fix:
    the deserialization of the anonymous class serializers (`CaseClassSerializer` and `TraversableSerializer`), even with the `serialVersionUID` specified, can still fail because there is no guarantee of the generated classname of anonymous classes (it depends on the order of when the anonymous classes were instantiated, and format seems to also change across compilers).
    
    At this moment, I've hit a bit of a wall trying to resolve this. The problem was always there pre-1.3, as if users simply change the order of their Scala type serializer generation (simply changing call order of `createTypeInformation` for their Scala types), the classnames would change and they wouldn't be able to restore state.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #4090: [FLINK-6869] [scala] Specify serialVersionUID for all Sca...

Posted by StefanRRichter <gi...@git.apache.org>.
Github user StefanRRichter commented on the issue:

    https://github.com/apache/flink/pull/4090
  
    Overall, I think this is ok as a best effort until we have some eager registration that helps with the remaining problems in the heap backend.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---