pnowojski commented on a change in pull request #8692: [FLINK-12804] Introduce
mailbox-based ExecutorService
URL: https://github.com/apache/flink/pull/8692#discussion_r293287752
##########
File path:
flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/tasks/mailbox/Mailbox.java
##########
@@ -20,17 +20,85 @@
import javax.annotation.Nonnull;
+import java.util.List;
+
/**
* A mailbox is basically a blocking queue for inter-thread message exchange
in form of {@link Runnable} objects between
- * multiple producer threads and a single consumer.
+ * multiple producer threads and a single consumer. This has a lifecycle of
closed -> open -> (quiesced) -> closed.
*/
public interface Mailbox extends MailboxReceiver, MailboxSender {
/**
- * The effect of this is that all pending letters are dropped and the
given priorityAction
- * is enqueued to the head of the mailbox.
+ * This enum represents the states of the mailbox lifecycle.
+ */
+ enum State {
+ OPEN, QUIESCED, CLOSED
+ }
+
+ /**
+ * Open the mailbox. In this state, the mailbox supports put and take
operations.
+ */
+ void open();
+
+ /**
+ * Quiesce the mailbox. In this state, the mailbox supports only take
operations and all pending and future put
+ * operations will throw {@link MailboxStateException}.
+ */
+ void quiesce();
+
+ /**
+ * Close the mailbox. In this state, all pending and future put
operations and all pending and future take
+ * operations will throw {@link MailboxStateException}. Returns all
letters that were still enqueued.
+ *
+ * @return list with all letters that where enqueued in the mailbox at
the time of closing.
+ */
+ @Nonnull
+ List<Runnable> close();
+
+ /**
+ * The effect of this is that all pending letters in the mailbox are
dropped and the given priorityLetter
+ * is enqueued to the head of the mailbox. Dropped letters are
returned. This method should only be invoked
+ * by code that has ownership of the mailbox object and only rarely
used, e.g. to submit special events like
+ * shutting down the mailbox loop.
+ *
+ * @param priorityLetter action to enqueue atomically after the mailbox
was cleared.
+ * @throws MailboxStateException if the mailbox is quiesced or closed.
+ */
+ @Nonnull
+ List<Runnable> clearAndPut(@Nonnull Runnable priorityLetter) throws
MailboxStateException;
+
+ /**
+ * Adds the given action to the head of the mailbox. This method will
block if the mailbox is full and
+ * should therefore only be called from outside the mailbox main-thread
to avoid deadlocks.
+ *
+ * @param priorityLetter action to enqueue to the head of the mailbox.
+ * @throws InterruptedException on interruption.
+ * @throws MailboxStateException if the mailbox is quiesced or closed.
+ */
+ void putFirst(@Nonnull Runnable priorityLetter) throws
InterruptedException, MailboxStateException;
Review comment:
If every method enqueuing the `Runnable` returned a `CompletableFuture<?>`,
we would have a better mechanism of canceling them, instead of instance of
checking the `Runnable`, right? Plus it would give us a generic mechanism for
checking whether an action has completed or not (especially useful in tests).
Are there some drawbacks of using `CompletableFuture`?
----------------------------------------------------------------
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
