rkhachatryan commented on a change in pull request #12714:
URL: https://github.com/apache/flink/pull/12714#discussion_r442789386
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java
##########
@@ -861,6 +869,43 @@ else if (transitionState(current, ExecutionState.FAILED,
t)) {
}
}
+ private void waitInvokableCancelCompletion() throws
java.util.concurrent.ExecutionException, java.util.concurrent.TimeoutException {
+ if (!invokableHasBeenCanceled.get()) {
+ return;
+ }
+
+ // set deadline if taskCancellationTimeout is set to have time
for cleanup (before being killing by TaskCancelerWatchDog) (best effort)
+ Optional<Deadline> deadline = Optional
+ .of((long) (taskCancellationTimeout * .9))
+ .filter(t -> t > 0)
+ .map(t -> Deadline.fromNow(Duration.ofMillis((t))));
Review comment:
The motivation was to preserve the *current* behaviour and have time for
cleanup which includes:
```
releaseResources();
memoryManager.releaseAll(invokable);
fileCache.releaseJob(jobId, executionId);
FileSystemSafetyNet.closeSafetyNetAndGuardedResourcesForThread();
notifyFinalState();
```
However, I see your point (especially given that old behavior didn't have a
thread leak issue) so I'll remove the deadline.
----------------------------------------------------------------
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:
[email protected]
