azagrebin commented on a change in pull request #12980:
URL: https://github.com/apache/flink/pull/12980#discussion_r460717855
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/slot/TaskSlot.java
##########
@@ -295,22 +303,25 @@ public String toString() {
// and set the slot state to releasing so that
it gets eventually freed
tasks.values().forEach(task ->
task.failExternally(cause));
}
+
final CompletableFuture<Void> cleanupFuture =
FutureUtils
.waitForAll(tasks.values().stream().map(TaskSlotPayload::getTerminationFuture).collect(Collectors.toList()))
- .thenRun(() -> {
- verifyMemoryFreed();
- this.memoryManager.shutdown();
- });
-
+ .thenRun(memoryManager::shutdown);
+ verifyAllManagedMemoryIsReleasedAfter(cleanupFuture);
FutureUtils.forward(cleanupFuture, closingFuture);
}
return closingFuture;
}
- private void verifyMemoryFreed() {
- if (!memoryManager.verifyEmpty()) {
- LOG.warn("Not all slot memory is freed, potential
memory leak at {}", this);
- }
+ private void
verifyAllManagedMemoryIsReleasedAfter(CompletableFuture<Void> after) {
+ FutureUtils.runAfterwardsAsync(
+ after,
+ () -> {
+ if (!memoryManager.verifyEmpty()) {
+ LOG.warn("Not all slot memory is freed,
potential memory leak at {}", this);
+ }
+ },
+ asyncExecutor);
Review comment:
yes, I think so
----------------------------------------------------------------
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
