1u0 commented on a change in pull request #9772: Flink 14199: Only use
dedicated/named classes for mailbox letters.
URL: https://github.com/apache/flink/pull/9772#discussion_r328147447
##########
File path:
flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/tasks/mailbox/execution/MailboxExecutor.java
##########
@@ -48,17 +54,42 @@
* Submits the given command for execution in the future in the mailbox
thread and returns a Future representing
* that command. The Future's {@code get} method will return {@code
null} upon <em>successful</em> completion.
*
+ * <p>Note that for debugging, it is is strongly recommended to use a
{@code Runnable} with proper {@code
+ * toString()} implementation, such as {@link NamedRunnable}.</p>
+ *
* @param command the command to submit
* @return a Future representing pending completion of the task
* @throws RejectedExecutionException if this task cannot be accepted
for execution, e.g. because the mailbox is
* quiesced or closed.
*/
default @Nonnull Future<?> submit(@Nonnull Runnable command) {
- return submit(Executors.callable(command, null));
+ NamedLetterUtil.checkOverwrittenToString(command);
+ return submit(namedCallable(() -> {
+ command.run();
+ return null;
+ }, command::toString));
Review comment:
With change in this PR and without, I think we can totally remove the
`submit(Runnable)` method, as `submit(Callable<T>)` is general enough.
In case of this PR, I'm a little bit concerned that there are two layers of
wrappers (the caller has already wrapped using `namedRunnable()`, and now the
method itself adds `namedCallable()`).
----------------------------------------------------------------
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
