[GitHub] [flink] azagrebin commented on issue #10387: [FLINK-14949] [Runtime/Task] Task cancellation can be stuck against out-of-thread error

[GitBox <gi...@apache.org>]

azagrebin commented on issue #10387: [FLINK-14949] [Runtime/Task] Task cancellation can be stuck against out-of-thread error
URL: https://github.com/apache/flink/pull/10387#issuecomment-563279007
 
 
   Thanks for working on this @pnowojski and @hwanju !
   No big concerns from my side.
   
   I was thinking a bit more into direction of refactoring the `else if (current == ExecutionState.RUNNING)` branch in `Task#cancelOrFailAndCancelInvokable` into a separate method. That method could call methods starting threads and wrap those calls with the try/catch because other if-branches are not expected to throw and cause the problem at hand. On the other hand if they are changed to throw in such a way then it is already covered.
   
   Anyways this kind of refactoring requires a bit more effort which we could probably consider during some bigger change like moving the whole cancelation logic into a separate component from the already quite huge Task class but this is another story.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services