akalash commented on a change in pull request #16885:
URL: https://github.com/apache/flink/pull/16885#discussion_r692799261
##########
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:
Super, it will be great if you will do that. Both names `Invokable` and
`TaskInvokable` looks good to me but perhaps `TaskInvokable` is a little better
since it is more specific and give more information about usage which is good.
--
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]
