flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [flink] 1u0 commented on a change in pull request #8523: [FLINK-12481][runtime] Invoke timer callback in task thread (via mailbox)
Date Tue, 28 May 2019 19:09:49 GMT
1u0 commented on a change in pull request #8523: [FLINK-12481][runtime] Invoke timer callback
in task thread (via mailbox)
URL: https://github.com/apache/flink/pull/8523#discussion_r288254960

 File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/tasks/StreamTask.java
 @@ -1358,4 +1358,19 @@ public void actionsUnavailable() throws InterruptedException {
+	private class TimerInvocationContext implements SystemProcessingTimeService.ScheduledCallbackExecutionContext
 Review comment:
   I don't think that the mailbox is exposed to the `SystemProcessingTimeService` at all here.
On the contrary.
   The `TimerInvocationContext` is in fact a "mailbox sender" (implementation detail, that
doesn't matter for the timer service). I'm not using the real `MailboxSender` interface, because
it would involve somehow adapting `ProcessingTimeCallback` interface to `Runnable`. Or changing
all implementers of `ProcessingTimeCallback` to new api - this may be quite involving and
may conflict with others' changes.
   Imo, things like `SystemProcessingTimeService` can be neutral about use cases. The fact
that it already had checkpoint lock and error handler callback, I find as incidental, rather
by design.
   Having additional transformation of `ProcessingTimeCallback` in the time service, into
a mailbox letter (`Runnable`), would make things even more coupled.
   An alternative could be, that there are no newly introduced `SystemProcessingTimeService.ScheduledCallbackExecutionContext`,
but keeping `SystemProcessingTimeService` implementation clean (not coupled with `StreamTask`).
   Add a proxy `ProcessingTimeService` implementation that would be exposed for others to
register timers, but upon registration, the proxy would register it's own callback in real
the timer service, which when fired, places the original `ProcessingTimeCallback` as a letter
into the mailbox.

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:

With regards,
Apache Git Services

View raw message