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



##########
File path: 
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinator.java
##########
@@ -775,8 +776,6 @@ public void handle(SyncGroupResponse syncResponse,
                     }
                 }
             } else {
-                requestRejoin();

Review comment:
       I added that function for sync group handler that handles retriable 
`COORDINATOR_NOT_AVAILABLE / NOT_COORDINATOR` and any unexpected error. After 
the refactoring PR they are not all fall into the `joinGroupIfNeeded` in
   
   ```
   final RuntimeException exception = future.exception();
   
                   resetJoinGroupFuture();
   
                   if (exception instanceof UnknownMemberIdException ||
                       exception instanceof IllegalGenerationException ||
                       exception instanceof RebalanceInProgressException ||
                       exception instanceof MemberIdRequiredException)
                       continue;
                   else if (!future.isRetriable())
                       throw exception;
   
                   resetStateAndRejoin(String.format("rebalance failed with 
retriable error %s", exception));
                   timer.sleep(rebalanceConfig.retryBackoffMs);
   ```
   
   This is part of the principle I mentioned:
   
   ```
   We may reset generationa and request rejoin in two different places: 1) in 
join/sync-group handler, and 2) in joinGroupIfNeeded, when the future is 
received. The principle is that these two should not overlap, and 2) is used as 
a fallback for those common errors from join/sync that we do not handle 
specifically.
   ```
   
   But I forgot to remove this function as part of the second pass; will remove.




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to