[GitHub] [kafka] ableegoldman commented on a change in pull request #10232: KAFKA-12352: Make sure all rejoin group and reset state has a reason

[GitBox]

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



##########
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:
       Ok cool, thanks. One last question then: after this refactoring, since 
we no longer call `requestRejoinOnResponseError` below, should we re-add the 
`requestRejoin()` call here? Or add a `requestRejoin` to the specific cases in 
the SyncGroup handler, eg
   ```
   } else if (error == Errors.REBALANCE_IN_PROGRESS) {
       log.info("SyncGroup failed: The group began another rebalance. Need to 
re-join the group. " +
                         "Sent generation was {}", sentGeneration);
       future.raise(error);
   }
   ```




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