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