rkhachatryan commented on code in PR #21923:
URL: https://github.com/apache/flink/pull/21923#discussion_r1123652411
##########
flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/tasks/StreamTask.java:
##########
@@ -733,8 +764,7 @@ void restoreInternal() throws Exception {
// needed
channelIOExecutor.shutdown();
- isRunning = true;
- isRestoring = false;
+ taskState.status = TaskState.Status.RUNNING;
Review Comment:
At this point, the task might be already cancelled "externally" (by
`TaskCanceler`), right?
Previously, this was captured by `isRunning = true` and `isCancelled = true`.
But now, we overwrite `CANCELLED` by `RUNNING`.
This might break checks like [this
one](https://github.com/apache/flink/blob/master/flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/tasks/SourceStreamTask.java#L339)
in `SourceStreamTask` and cause task and then job failure.
---
I guess this is basically the same concern expressed previously that we
might lose some transitions.
I see only two ways to be on the safe side:
1. Have all the same flags, but inside the inner `TaskState` class
1. Model the enum exactly as `2^number_of_flags` AND transition state via
CAS (not just assignment)
WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
