[FLINK-5019] Proper isRestored result for tasks that did not write state #2746
Conversation
StefanRRichter
force-pushed
the
hotfix-has-state
branch
4 times, most recently
from
8e12dd7
to
db3760d
Compare
StefanRRichter
changed the title
| assertEquals(errMsg, subtaskState.getLegacyOperatorState(), | ||
| tdd.getTaskStateHandles().getLegacyOperatorState()); | ||
|
|
||
| if (subtaskState.hasState()) { |
There was a problem hiding this comment.
I think this check can actually be replaced by subtaskState.getLegacyOperatorState() and if you do that there is no place in the code that calls SubtaskState.hasState(), making it obsolete.
Please correct me if I'm wrong. 😅
There was a problem hiding this comment.
Didn't run a find-usage, but if that is true then I agree to your proposal :)
| " has been taken and the current maximum parallelism (" + | ||
| executionJobVertex.getMaxParallelism() + ") changed. This " + | ||
| "is currently not supported."); | ||
| if (executionJobVertex == null) { |
There was a problem hiding this comment.
There is no change to the code, right? Just changing the three-way if to a two-way if with a continue?
There was a problem hiding this comment.
The code should do the same, I just found the if-nesting was a little deep and checking the preconditions first makes it more readable imo.
There was a problem hiding this comment.
That's what I thought. I just wanted to double check because Github makes it look very scary. 😃
|
Overall, this change is very good! 👍 I had some inline comments/questions. |
StefanRRichter
force-pushed
the
hotfix-has-state
branch
6 times, most recently
from
19248e7
to
b0fc1e2
Compare
StefanRRichter
force-pushed
the
hotfix-has-state
branch
from
b0fc1e2
to
43e234c
Compare
When a subtask is restored from a checkpoint that does not contain any state (e.g. because the subtask did not write state in the previous run), the result of
StateInitializationContext::isRestoredwill incorrectly return false.This PR ensures that empty state is reflected in a checkpoint and that true is returned from the method on restore, independent from the presence of state.
Furthermore,
SubtaskState::hasStateignored raw operator state and returned false even if there was raw operator state. PR includes a unit test to catch this problem.