akalash commented on code in PR #21923: URL: https://github.com/apache/flink/pull/21923#discussion_r1124556907
########## 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: As I understand right now all transitions are correct. The only problem is we can overwrite the final status with another final status which is incorrect behaviour. To fix that, I agree we can use CAS to update TaskStateStatus and allow only transitions described in javadoc. I also don't think that we should get back to 4 flags since we already spend a lot of time understanding which combinations of them are really possible and it seems nobody knows for sure. So it is better to decrease these combinations and I think the enum with clear transitions is good choice. I still think that we should even get rid of `failing` flag since I feel we can combine it with enum but I'm ok if we do it later -- 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