Re: [PR] KAFKA-16664: Re-add EventAccumulator.poll(long, TimeUnit) [kafka]

["dajac (via GitHub)" <gi...@apache.org>]

dajac commented on code in PR #16144:
URL: https://github.com/apache/kafka/pull/16144#discussion_r1621959369


##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/EventAccumulator.java:
##########
@@ -162,43 +163,21 @@ public void addFirst(T event) throws RejectedExecutionException {
     }
 
     /**
-     * Returns the next {{@link Event}} available or null if no event is
-     * available.
+     * Returns the next {{@link Event}} available. This method blocks for the provided
+     * time and returns null of not event is available.

Review Comment:
   nit: `if no event is...`



##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/MultiThreadedEventProcessor.java:
##########
@@ -151,8 +157,8 @@ private void handleEvents() {
         }
 
         private void drainEvents() {
-            CoordinatorEvent event;
-            while ((event = accumulator.poll()) != null) {
+            CoordinatorEvent event = accumulator.poll(0, TimeUnit.MILLISECONDS);
+            while (event != null) {

Review Comment:
   nit: Should we keep the previous way?



##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/MultiThreadedEventProcessor.java:
##########
@@ -34,6 +35,11 @@
  */
 public class MultiThreadedEventProcessor implements CoordinatorEventProcessor {
 
+    /**
+     * The poll timeout to wait for an event by the EventProcessorThread.
+     */
+    public static final long POLL_TIMEOUT_MS = 300L;

Review Comment:
   Should it be private?



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org