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 2018/01/25 17:50:40 UTC
[GitHub] flink pull request #5364: Flink 8472 1.4
GitHub user tzulitai opened a pull request:
https://github.com/apache/flink/pull/5364
Flink 8472 1.4
## What is the purpose of the change
Extend all migration tests, to include verifying restore from Flink 1.4 savepoints.
This includes extending:
- `WindowOperatorMigrationTest`
- `CEPMigrationTest`
- `StatefulJobSavepointMigrationTestITCase` (Scala API migration)
- `StatefulJobSavepointMigrationTestITCase` (Java API migration)
- `FlinkKinesisConsumerMigrationTest`
- `FlinkKafkaConsumerBaseMigrationTest`
- `ContinuousFileProcessingMigrationTest`
- `BucketingSinkMigrationTest`
This PR should also be forward-ported to the `master` branch to cover Flink 1.5.
## Brief change log
All commits except 1ce3e6c are simply adding `MigrationVersion.v1_4` to the test parameters and adding the corresponding test savepoint files.
1ce3e6c is a refactor of `StatefulJobSavepointMigrationFrom12ITCase` and `StatefulJobSavepointMigrationFrom13ITCase` to a single `StatefulJobSavepointMigrationITCase` class.
## Verifying this change
This is a test refactor / extension, so all existing tests should cover the changes.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (yes / **no**)
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes / **no**)
- The serializers: (yes / **no** / don't know)
- The runtime per-record code paths (performance sensitive): (yes / **no** / don't know)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (yes / **no** / don't know)
- The S3 file system connector: (yes / **no** / don't know)
## Documentation
- Does this pull request introduce a new feature? (yes / **no**)
- If yes, how is the feature documented? (**not applicable** / docs / JavaDocs / not documented)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/tzulitai/flink FLINK-8472-1.4
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/5364.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 #5364
----
commit 28a1c4d403cc978d60af765f1e67bfb4bedd93c4
Author: Tzu-Li (Gordon) Tai <tz...@...>
Date: 2018-01-25T15:08:53Z
[FLINK-8472] [DataStream, test] Extend WindowOperatorMigrationTest for Flink 1.4
commit 803c641b6c4e03431a960826f5a531423db3d5ef
Author: Tzu-Li (Gordon) Tai <tz...@...>
Date: 2018-01-25T15:13:14Z
[FLINK-8472] [cep, test] Extend CEPMigrationTest for Flink 1.4
commit 6f6505232c7294a0c30e7cef9fbf2f9218e478b1
Author: Tzu-Li (Gordon) Tai <tz...@...>
Date: 2018-01-25T15:20:31Z
[FLINK-8472] [scala, test] Extend StatefulJobSavepointMigrationITCase for Flink 1.4
commit 1ce3e6cd331d8791d28474d9838070005d4e37a3
Author: Tzu-Li (Gordon) Tai <tz...@...>
Date: 2018-01-25T17:25:45Z
[FLINK-8472] [DataStream, test] Refactor StatefulJobSavepointFrom*MigrationITCase to single ITCase
This commit refactors the StatefulJobSavepointFrom12MigrationITCase and
StatefulJobSavepointFrom13MigrationITCase to a single class,
StatefulJobSavepointMigrationITCase. The new ITCase is parameterized to
ensure that all previous versions and state backend variants are
covered.
commit 696b5b3f3927f63c1c0e7e550db14427dbe7a0cb
Author: Tzu-Li (Gordon) Tai <tz...@...>
Date: 2018-01-25T17:37:05Z
[FLINK-8472] [DataStream, test] Extend StatefulJobSavepointMigrationITCase for Flink 1.4
commit 1d308604bf832c856b8b6f3d1a33d189ddab80e8
Author: Tzu-Li (Gordon) Tai <tz...@...>
Date: 2018-01-25T15:23:17Z
[FLINK-8472] [kinesis, test] Extend FlinkKinesisConsumerMigrationTest for Flink 1.4
commit 2de2943ffed3f1ab8928e3d9be96c5c6d7dd4f96
Author: Tzu-Li (Gordon) Tai <tz...@...>
Date: 2018-01-25T16:23:22Z
[FLINK-8472] [kafka, test] Extend FlinkKafkaConsumerBaseMigrationTest for Flink 1.4
commit bfee6ce9f842e7e2785ffb664001b42e2f7e5a44
Author: Tzu-Li (Gordon) Tai <tz...@...>
Date: 2018-01-25T16:28:17Z
[FLINK-8472] [fs, test] Extend ContinuousFileProcessingMigrationTest for Flink 1.4
commit 141236219b0afd4093ae891398564f39539875e0
Author: Tzu-Li (Gordon) Tai <tz...@...>
Date: 2018-01-25T16:32:28Z
[FLINK-8472] [fs, test] Extend BucketingSinkMigrationTest for Flink 1.4
commit ceb00c7b210dca4f4c2e2bd5f2f399d5f0880934
Author: Tzu-Li (Gordon) Tai <tz...@...>
Date: 2018-01-25T16:45:47Z
[hotfix] [test] Remove stale savepoint files no longer used by migration tests
This includes:
- Removing MigrationVersion.v1_1, since compatilbity for 1.1 is no
longer supported (and no migration tests still test that)
- Remove all 1.1 test savepoint files (which no migration tests still
use)
- Remove all 1.2 test savepoint files that are no longer in use (e.g.,
CEPMigrationTest does not test 1.2 restores anymore)
----
---
[GitHub] flink pull request #5364: [FLINK-8472] [test] Extend all migration tests for...
Posted by tzulitai <gi...@git.apache.org>.
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5364#discussion_r165385564
--- Diff: flink-tests/src/test/java/org/apache/flink/test/state/operator/restore/keyed/KeyedJob.java ---
@@ -100,9 +100,7 @@ public static void main(String[] args) throws Exception {
.map(new StatefulStringStoringMap(mode, "first"))
.setParallelism(4);
- if (mode == ExecutionMode.MIGRATE || mode == ExecutionMode.RESTORE) {
--- End diff --
@zentol can you confirm whether these changes make sense?
From the discussions I see in https://github.com/apache/flink/pull/3844, I assume this was a leftover to write a 1.2 savepoint (when uids couldn't be added for each chained operator separately). So it should be ok to remove this?
---
[GitHub] flink pull request #5364: [FLINK-8472] [test] Extend all migration tests for...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/5364#discussion_r165459894
--- Diff: flink-tests/src/test/java/org/apache/flink/test/state/operator/restore/keyed/KeyedJob.java ---
@@ -100,9 +100,7 @@ public static void main(String[] args) throws Exception {
.map(new StatefulStringStoringMap(mode, "first"))
.setParallelism(4);
- if (mode == ExecutionMode.MIGRATE || mode == ExecutionMode.RESTORE) {
--- End diff --
yeah it should be alright to remove that, but let's chain the uid call to the operator creation as we do for the others for style points.
---
[GitHub] flink issue #5364: [FLINK-8472] [test] Extend all migration tests for Flink ...
Posted by tzulitai <gi...@git.apache.org>.
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5364
Thanks for the reviews, I'll address Chesnay's comment and the merge this then (to `master` and `release-1.4`).
---
[GitHub] flink issue #5364: [FLINK-8472] [test] Extend all migration tests for Flink ...
Posted by tzulitai <gi...@git.apache.org>.
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5364
@aljoscha @zentol
could you have another quick look at commit 8882fb7? That commit extends `KeyedComplexChainTest`, `ChainBreakTest`, `ChainLengthIncreaseTest`, ... etc. also for 1.4.
Once that is good, I'll merge this to `master` and `release-1.4`.
---
[GitHub] flink issue #5364: [FLINK-8472] [test] Extend all migration tests for Flink ...
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/5364
The changes look straightforward and good! 👍
Plus, they help me sleep better... 😅
---
[GitHub] flink issue #5364: [FLINK-8472] [test] Extend all migration tests for Flink ...
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/5364
Looks good but let's wait what @zentol has to say.
---
[GitHub] flink issue #5364: [FLINK-8472] [test] Extend all migration tests for Flink ...
Posted by tzulitai <gi...@git.apache.org>.
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5364
It seems like we'll also need to extend these migration tests for 1.4:
- All subclasses of `AbstractKeyedOperatorRestoreTestBase`
- All subclasses of `AbstractNonKeyedOperatorRestoreTestBase`
---
[GitHub] flink pull request #5364: [FLINK-8472] [test] Extend all migration tests for...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/5364
---