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

Reply via email to