Myracle commented on a change in pull request #13227:
URL: https://github.com/apache/flink/pull/13227#discussion_r481069951
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/dispatcher/MiniDispatcher.java
##########
@@ -91,37 +91,21 @@ public MiniDispatcher(
ApplicationStatus status =
result.getSerializedThrowable().isPresent() ?
ApplicationStatus.FAILED :
ApplicationStatus.SUCCEEDED;
- LOG.debug("Shutting down per-job cluster
because someone retrieved the job result.");
+ LOG.debug("Shutting down cluster because
someone retrieved the job result.");
shutDownFuture.complete(status);
});
} else {
- LOG.debug("Not shutting down per-job cluster after
someone retrieved the job result.");
+ LOG.debug("Not shutting down cluster after someone
retrieved the job result.");
}
return jobResultFuture;
}
- @Override
- public CompletableFuture<Acknowledge> cancelJob(
- JobID jobId, Time timeout) {
- CompletableFuture<Acknowledge> cancelFuture =
super.cancelJob(jobId, timeout);
-
- cancelFuture.thenAccept((ignored) -> {
- LOG.debug("Shutting down per-job cluster because the
job was canceled.");
- shutDownFuture.complete(ApplicationStatus.CANCELED);
- });
-
- return cancelFuture;
- }
-
@Override
protected void jobReachedGloballyTerminalState(ArchivedExecutionGraph
archivedExecutionGraph) {
super.jobReachedGloballyTerminalState(archivedExecutionGraph);
- if (executionMode == ClusterEntrypoint.ExecutionMode.DETACHED) {
- // shut down since we don't have to wait for the
execution result retrieval
-
shutDownFuture.complete(ApplicationStatus.fromJobStatus(archivedExecutionGraph.getState()));
- }
+
shutDownFuture.complete(ApplicationStatus.fromJobStatus(archivedExecutionGraph.getState()));
Review comment:
@tillrohrmann You are right. We should not shutdown dispatcher here if
job result is required. But if we just do nothing here for the attached mode,
the dispatcher may be not shutdown at last. Imagine that the client is
shutdown unexpectedly, then the job is cancelled from web. In this case, no job
request is made so that the dispatcher will not be shutdown. Maybe we should
make our best to shutdown dispatcher after some time for the attached mode.
What do you think? Thank you.
----------------------------------------------------------------
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:
us...@infra.apache.org
