[GitHub] flink pull request #5495: [FLINK-8659] Add migration itcases for broadcast s...

[kl0u <gi...@git.apache.org>]

GitHub user kl0u opened a pull request:

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

    [FLINK-8659] Add migration itcases for broadcast state.

    ## What is the purpose of the change
    
    Adds migration tests from `1.5` to `1.5`. 
    
    This is for future changes to guarantee backwards compatibility of the newly introduced broadcast state.
    
    ## Brief change log
    
    Adds tests a la `StatefulJobSavepointMigrationITCase`.
    
    
    ## Verifying this change
    
    This change just adds tests.
    
    ## 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: no
      - 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 @aljoscha 

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

    $ git pull https://github.com/kl0u/flink broadcast-migration-inv

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

    https://github.com/apache/flink/pull/5495.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 #5495
    
----
commit d0cbab60d14a46aac2af3ad534f6c0b49989c0f1
Author: kkloudas <kk...@...>
Date:   2018-02-14T16:33:15Z

    [FLINK-8659] Add migration itcases for broadcast state.

----


---

[GitHub] flink pull request #5495: [FLINK-8659] Add migration itcases for broadcast s...

[kl0u <gi...@git.apache.org>]

Github user kl0u closed the pull request at:

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


---

[GitHub] flink issue #5495: [FLINK-8659] Add migration itcases for broadcast state.

[aljoscha <gi...@git.apache.org>]

Github user aljoscha commented on the issue:

    https://github.com/apache/flink/pull/5495
  
    It's hard to know the actual changes from the existing savepoint it case. I created a new history by tweezing apart the copying and the actual modifications: https://github.com/aljoscha/flink/tree/finish-pr-5495-broadcast-migration-test. I first only copy the existing test to create the new one, then I apply the changes. Could you update your PR to that?
    
    Another thing is that in general reusing code is good but in this test you should simply copy all the user code, the new test refers to classes in the old test which will make it hard if we want to remove the old test at some point and also makes things hard with serialisation, as you noticed. What do you think?


---

[GitHub] flink issue #5495: [FLINK-8659] Add migration itcases for broadcast state.

[kl0u <gi...@git.apache.org>]

Github user kl0u commented on the issue:

    https://github.com/apache/flink/pull/5495
  
    The new tests are in separate classes than the `StatefulJobSavepointMigrationITCase`. The reason is that in the scala case, and due to the way we serialize scala classes, introducing new code in the class seems to be breaking the already existing migration `ITCases`.


---

[GitHub] flink issue #5495: [FLINK-8659] Add migration itcases for broadcast state.

[kl0u <gi...@git.apache.org>]

Github user kl0u commented on the issue:

    https://github.com/apache/flink/pull/5495
  
    I close this and I will open an updated one.


---