1u0 commented on a change in pull request #8692: [FLINK-12804] Introduce
mailbox-based ExecutorService
URL: https://github.com/apache/flink/pull/8692#discussion_r292515896
##########
File path:
flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/tasks/StreamTask.java
##########
@@ -395,6 +417,9 @@ public final void invoke() throws Exception {
// make sure no new timers can come
timerService.quiesce();
+ // let mailbox execution reject all new letters
from this point
+ taskMailboxExecutor.shutdown();
Review comment:
Is it possible that there are still some letters in the mailbox queue that
would require to be executed (including future cases to host timer service and
checkpoints via mailbox)?
Can `StreamTask::closeAllOperators()` still need the mailbox loop (e.g. for
timers)?
My impression, that the old `StreamTask::run()` method (prior #8442 PR)
finished running, when the source has ended. Other threads could still do valid
operations
So the mailbox loop run is not the same as `StreamTask::run()`, but should
span until `StreamTask.isRunning` is `true`.
Otherwise, we would need to change this code again (the `StreamTask` and the
loop in `SourceStreamTask`) to support timers in compatible way.
----------------------------------------------------------------
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
