injae-kim commented on code in PR #19631:
URL: https://github.com/apache/kafka/pull/19631#discussion_r2081357892


##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java:
##########
@@ -1304,23 +1304,25 @@ RequestFuture<Void> sendOffsetCommitRequest(final 
Map<TopicPartition, OffsetAndM
         final Generation generation;
         final String groupInstanceId;
         if (subscriptions.hasAutoAssignedPartitions()) {
-            generation = generationIfStable();
-            groupInstanceId = rebalanceConfig.groupInstanceId.orElse(null);
-            // if the generation is null, we are not part of an active group 
(and we expect to be).
-            // the only thing we can do is fail the commit and let the user 
rejoin the group in poll().
-            if (generation == null) {
-                log.info("Failing OffsetCommit request since the consumer is 
not part of an active group");
-
-                if (rebalanceInProgress()) {
-                    // if the client knows it is already rebalancing, we can 
use RebalanceInProgressException instead of
-                    // CommitFailedException to indicate this is not a fatal 
error
-                    return RequestFuture.failure(new 
RebalanceInProgressException("Offset commit cannot be completed since the " +
-                        "consumer is undergoing a rebalance for auto partition 
assignment. You can try completing the rebalance " +
-                        "by calling poll() and then retry the operation."));
-                } else {
-                    return RequestFuture.failure(new 
CommitFailedException("Offset commit cannot be completed since the " +
-                        "consumer is not part of an active group for auto 
partition assignment; it is likely that the consumer " +
-                        "was kicked out of the group."));
+            synchronized (ConsumerCoordinator.this) {

Review Comment:
   ```suggestion
               // (the reason why we add synchronized block?)
               synchronized (ConsumerCoordinator.this) {
   ```
   nit. how about add comment about why we added `synchronized `here?
   cause someone can just easily remove this `synchronized `block without 
comment in the future!



-- 
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-unsubscr...@kafka.apache.org

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

Reply via email to