rkhachatryan commented on a change in pull request #16885:
URL: https://github.com/apache/flink/pull/16885#discussion_r692376188
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java
##########
@@ -762,17 +762,21 @@ private void doRun() {
executingThread.setContextClassLoader(userCodeClassLoader.asClassLoader());
AbstractInvokable finalInvokable = invokable;
- runWithSystemExitMonitoring(finalInvokable::restore);
+ try {
+ runWithSystemExitMonitoring(finalInvokable::restore);
- if (!transitionState(ExecutionState.INITIALIZING,
ExecutionState.RUNNING)) {
- throw new CancelTaskException();
- }
+ if (!transitionState(ExecutionState.INITIALIZING,
ExecutionState.RUNNING)) {
+ throw new CancelTaskException();
+ }
- // notify everyone that we switched to running
- taskManagerActions.updateTaskExecutionState(
- new TaskExecutionState(executionId,
ExecutionState.RUNNING));
+ // notify everyone that we switched to running
+ taskManagerActions.updateTaskExecutionState(
+ new TaskExecutionState(executionId,
ExecutionState.RUNNING));
- runWithSystemExitMonitoring(finalInvokable::invoke);
+ runWithSystemExitMonitoring(finalInvokable::invoke);
+ } finally {
+ runWithSystemExitMonitoring(finalInvokable::cleanUp);
+ }
Review comment:
Thanks for sharing and raising this!
I like the idea and I agree we should improve the contract
`AbstractInvokable` and not postpone it till larger refactorings.
Maybe we should name it just `Invokable` or `TaskInvokable` (as it's
actually invokable part of the `Task`)?
`cancel()` I think should also belong to this interface.
Another set of methods I'd extract are related to checkpointing (those
throwing `UnsupportedOperationException` currently). We could have
`CheckpointableInvokable extends AbstractInvokable` implemented only by a
subset of classes.
--
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
