tillrohrmann commented on a change in pull request #10682:
[FLINK-15247][Runtime] Wait for all slots to be free before task executor
services shutdown upon stopping
URL: https://github.com/apache/flink/pull/10682#discussion_r362852268
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskExecutor.java
##########
@@ -376,7 +372,7 @@ private void
handleStartTaskExecutorServicesException(Exception e) throws Except
return FutureUtils
.runAfterwards(
-
taskCompletionTracker.failIncompleteTasksAndGetTerminationFuture(),
+ taskSlotTable.freeAllSlots(new
FlinkException("The task executor is shutting down.")),
Review comment:
The current way of ensuring that all tasks are being terminated before the
returned future is completed is super hard to understand because the code path
is too convoluted and too many components depend on each other to make it
happen. It took me actually quite some time to figure this out and I'm somewhat
familiar with the code. Instead of introducing additional APIs which need to be
called in a very specific manner, I would suggest that we simply call
`taskSlotTable.closeAsync()` which returns a future which is completed once all
slots are closed. A slot is closed if all its task have terminated. The
information about a task being closed does not need to be passed into the
`TaskSlotTable` via the `freeSlot` but we could simple take a look at the
`Task#getTerminationFuture` in order to control when a `TaskSlot` is completely
released.
A good design principle is context independence. If possible a component
should work independent of its context meaning that it tries to decouple it
self as good as possible from other components. In this case, for example, we
require that `TaskExecutor` calls `TaskSlotTable#freeSlot` after we have called
`TaskSlotTable#freeAllSlots` and before we can call `close`.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
