nileshkumar3 commented on code in PR #22356:
URL: https://github.com/apache/kafka/pull/22356#discussion_r3298190625


##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java:
##########


Review Comment:
   I think this path may still have the same issue that this PR fixes for the 
heartbeat flow.
   request.groupInstanceId() is already available here, but the max-size check 
still runs before static-member replacement is considered. So if a static 
member rejoins through the classic JoinGroup path while the group is full, it 
looks like it will get GroupMaxSizeReachedException even though it would only 
replace its existing slot.
   Would it make sense to pass the instanceId here as well?
   throwIfConsumerGroupIsFull(group, memberId, instanceId);
   If so, the old 2-arg overload could probably be removed afterward since this 
seems the only remaining caller.
   This path exists specifically to support classic-protocol members in a 
ConsumerGroup, so a static-member rejoin here seems like a expected scenario.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to