[GitHub] [kafka] guozhangwang commented on a change in pull request #11231: KAFKA-13214; Consumer should not reset state after retriable error in rebalance

[GitBox <gi...@apache.org>]

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



##########
File path: clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinator.java
##########
@@ -476,7 +476,6 @@ boolean joinGroupIfNeeded(final Timer timer) {
                 else if (!future.isRetriable())
                     throw exception;
 
-                resetStateAndRejoin(String.format("rebalance failed with retriable error %s", exception));

Review comment:
       Just adding my 2c here: The reason I add that resetting in https://github.com/apache/kafka/commit/7e7bb184d2abe34280a7f0eb0f0d9fc0e32389f2# is that when rebalance failed with retriable exceptions, we would not trigger the `onJoinPrepare(generation.generationId, generation.memberId)` again upon retry. However for certain cases we do want to trigger that function in order to e.g. revoke partitions.
   
   Reading that change again, I agree this is probably not the best pattern anyways, probably we can consider resetting `needsJoinPrepare` in the `!future.succeeded()` case as well before we retry, and then remove this line?




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

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org