lianetm commented on code in PR #17614:
URL: https://github.com/apache/kafka/pull/17614#discussion_r1946680653
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinator.java:
##########
@@ -1187,6 +1183,21 @@ public synchronized RequestFuture<Void>
maybeLeaveGroup(String leaveReason) {
return future;
}
+ private boolean
shouldSendLeaveGroupRequest(CloseOptions.GroupMembershipOperation
membershipOperation) {
+ if (CloseOptions.GroupMembershipOperation.REMAIN_IN_GROUP ==
membershipOperation) {
+ return false;
+ } else if (CloseOptions.GroupMembershipOperation.LEAVE_GROUP ==
membershipOperation) {
+ // According to KIP-1092, static members can leave the group if
the consumer group membership operation is LEAVE_GROUP.
+ return true;
Review Comment:
Hey folks, sorry I haven't had the time to go through these changes in
detail, but my take on this in case it helps. Currently both consumers do
attempt to rediscover a coordinator on close, but for committing offsets before
closing, not for sending the leave group after having committed those offsets.
So if at the time of sending the leave group there is no coord (or even if the
leave fails) both consumers just won't send the leave (won't attempt
rediscovering the coordinator even).
So I agree with @ableegoldman 's initial comment here, that (if we want to
keep the current logic)
> we should keep some of the original logic and skip it under these
conditions
(so if no coordinator, or the member is not in the group anymore, no leave
request needs to be sent)
This is a recent PR discussion on this same point, digging into how the
classic behaves, to have the same in the new async consumer:
https://github.com/apache/kafka/pull/18590#issuecomment-2611089054
--
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]
