Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/3844#discussion_r122894136
--- Diff:
flink-tests/src/test/java/org/apache/flink/test/state/operator/restore/keyed/KeyedJob.java
---
@@ -98,10 +98,9 @@ public static void main(String[] args) throws Exception {
.map(new StatefulStringStoringMap(mode, "first"))
.setParallelism(4);
- // TODO: re-enable this when generating the actual 1.2 savepoint
- //if (mode == ExecutionMode.MIGRATE || mode ==
ExecutionMode.RESTORE) {
- map.uid("first");
- //}
+ if (mode == ExecutionMode.MIGRATE || mode ==
ExecutionMode.RESTORE) {
--- End diff --
You are completely right, the commit message/PR description isn't
sufficient to explain what this PR changes. In fact, it took me a bit to
remember that as well. I'll adjust the commit message later on.
So this PR is pretty subtle, since the changes to the code aren't the
interesting part, but the change to the `complexKeyed-flink1.2/_metadata` file
is. This file is supposed to be a 1.2 savepoint to verify the restore behavior
from them in 1.3. But this file is not a 1.2 savepoint, because at the time of
merging the restoration of keyed 1.2 state was broken, In the meantime we used
a 1.3 savepoint instead.
The main thing this PR does is replace this 1.3 savepoint with an actual
1.2 savepoint.
The second change is related to the uid's. In 1.2, it is not possible to
assign UIDs to chained operators. As "first" and "second" are both chained to
the window function we are not allowed to call `map.uid("...")` when generating
the 1.2 savepoint (! (MIGRATE || RESTORE)). However, in 1.3 it is possible and
in fact mandatory to assign UIDs.
Does that clear things up?
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
