[GitHub] [kafka] hachikuji commented on a diff in pull request #12611: KAFKA-14208: Should not wake-up with non-blocking coordinator discovery

[GitBox]

hachikuji commented on code in PR #12611:
URL: https://github.com/apache/kafka/pull/12611#discussion_r968778503


##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinator.java:
##########
@@ -249,7 +249,14 @@ protected synchronized boolean 
ensureCoordinatorReady(final Timer timer) {
                 throw fatalException;
             }
             final RequestFuture<Void> future = lookupCoordinator();
-            client.poll(future, timer);
+
+            // if we do not want to block on discovering coordinator at all,
+            // then we should not try to poll in a loop, and should not throw 
wake-up exception either
+            if (timer.timeoutMs() == 0L) {

Review Comment:
   Yeah, I feel it's a bit slippery to leave the logic in place with just a 
TODO somewhere to go back and fix it. Too many times, the follow-up never 
happens. Is it really that much additional effort to add an overload?
   
   ```java
   protected synchronized boolean ensureCoordinatorReady(final Timer timer) {
     return ensureCoordinatorReady(timer, true);
   }
   
   private synchronized boolean ensureCoordinatorReady(final Timer timer, 
boolean checkWakeup) {
   ...
   ```



-- 
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-unsubscr...@kafka.apache.org

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