zentol commented on code in PR #19427:
URL: https://github.com/apache/flink/pull/19427#discussion_r848295455
##########
flink-runtime/src/main/java/org/apache/flink/runtime/dispatcher/Dispatcher.java:
##########
@@ -1215,15 +1241,10 @@ private CompletableFuture<Void> waitForTerminatingJob(
getMainThreadExecutor());
}
+ @VisibleForTesting
CompletableFuture<Void> getJobTerminationFuture(JobID jobId) {
- if (jobManagerRunnerRegistry.isRegistered(jobId)) {
Review Comment:
I don't think we're weakening any variants. The previous ones stated that
either
a) the job is still running
b) there _may_ be a termination future for the job.
Since we now no longer care about a) there isn't anything to assert, because
b) already allowed no termination future to be present.
I'm not even sure if the previous behavior is correct; AFAICT if
`waitForTerminatingJob` returns a failed future because the job is already
running we then initiate the cleanup which could actually wreck the running
job. Unless there's some safeguard in the resource cleaner.
--
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]
