ahuang98 commented on code in PR #22505:
URL: https://github.com/apache/kafka/pull/22505#discussion_r3397545619


##########
raft/src/main/java/org/apache/kafka/raft/internals/BlockingMessageQueue.java:
##########
@@ -17,41 +17,82 @@
 package org.apache.kafka.raft.internals;
 
 import org.apache.kafka.common.errors.InterruptException;
+import org.apache.kafka.raft.RaftMessage;
 import org.apache.kafka.raft.RaftMessageQueue;
 
 import java.util.Optional;
 import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionStage;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 
 public class BlockingMessageQueue implements RaftMessageQueue {
-    private final BlockingQueue<QueueEntry> queue = new 
LinkedBlockingQueue<>();
+    // Marker object for wakeup events
+    private static final Object WAKEUP = new Object();
+
+    private final BlockingQueue<Object> queue = new LinkedBlockingQueue<>();

Review Comment:
   I guess the whole point is that we can have more explicit types and make the 
behavior slightly more readable? 
   
   e.g. reader has to imply the following equates to wakeup
   ```
   while (entry != null && !(entry instanceof QueueEntry)) {
   ```
   vs explicit
   ```
   while (entry instanceof WakeupEntry) {
   ```
   since you're going to be refactoring this part later I don't have a strong 
preference



##########
raft/src/main/java/org/apache/kafka/raft/internals/FuturePurgatory.java:
##########
@@ -16,68 +16,64 @@
  */
 package org.apache.kafka.raft.internals;
 
-import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionStage;
 
 /**
  * Simple purgatory interface which supports waiting with expiration for a 
given threshold
  * to be reached. The threshold is specified through {@link #await(Comparable, 
long)}.
  * The returned future can be completed in the following ways:

Review Comment:
   nit: one more substitution to `stage` (think there are a few more references 
to 'future' that could be swapped out in this class)



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to