guozhangwang commented on a change in pull request #9671:
URL: https://github.com/apache/kafka/pull/9671#discussion_r564899776



##########
File path: 
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinator.java
##########
@@ -1359,10 +1368,15 @@ public void run() {
                         long now = time.milliseconds();
 
                         if (coordinatorUnknown()) {
-                            if (findCoordinatorFuture != null || 
lookupCoordinator().failed())
-                                // the immediate future check ensures that we 
backoff properly in the case that no
-                                // brokers are available to connect to.
+                            if (findCoordinatorFuture != null) {

Review comment:
       I think the issue is spot-on! The logic here becomes a bit hard to 
understand for other readers now and I'd suggest update the cmment as:
   
   "Clear the future so that after the backoff in the next iteration, if hb 
still sees coordinator unknown it will try re-discover the coordinator in case 
the main thread cannot"
   
   Otherwise, LGTM.




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


Reply via email to