dajac commented on a change in pull request #9449:
URL: https://github.com/apache/kafka/pull/9449#discussion_r507630062



##########
File path: 
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java
##########
@@ -3938,25 +3938,15 @@ private Call 
getAlterConsumerGroupOffsetsCall(ConsumerGroupOperationContext<Map<
             void handleResponse(AbstractResponse abstractResponse) {
                 final OffsetCommitResponse response = (OffsetCommitResponse) 
abstractResponse;
 
-                // If coordinator changed since we fetched it, retry
-                if 
(ConsumerGroupOperationContext.hasCoordinatorMoved(response)) {
+                // 1) If coordinator changed since we fetched it, retry
+                // 2) If there is a coordinator error, retry
+                if 
(ConsumerGroupOperationContext.hasCoordinatorMoved(response) ||
+                    
ConsumerGroupOperationContext.shouldRefreshCoordinator(response)) {

Review comment:
       While the code is cleaner, the error counts map (`errorCounts`) is 
re-computed 3 times now. Once in `hasCoordinatorMoved` and twice in 
`shouldRefreshCoordinator`. It would be better if we could get it once and 
reuse it.




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