akalash commented on code in PR #21923:
URL: https://github.com/apache/flink/pull/21923#discussion_r1124761808
##########
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:
Potentially we can have CANCELLING status but right now I don't see where we
can actually use it. For example, if we consider the above-described scenario,
we have a situation when canceled=true at the same time as isRestoring=true as
well. isRestoring we use only inside `handleAsyncException` and if we want to
keep this logic we indeed need some extra status like CANCELLING but I actually
think that we don't need to keep that logic since if the job is canceled
already we can easily ignore other exceptions which can happen since they can
be actually because of cancelation. I have the same thoughts about
canceled=true + isRunning=true.(I don't see where we should execute logic
isRunning=true if the canceled is true already.)
Long story short, if we indeed understand where we need CANCELLING status
let's add it but if we need it only for keeping the old behavior let's discuss
this old behavior since I am not really sure that the old behavior is correct.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
