ableegoldman commented on code in PR #17614:
URL: https://github.com/apache/kafka/pull/17614#discussion_r1942365317
##########
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:
technically it doesn't make sense to send a LeaveGroup if the
`!coordinatorUnknown() && state != MemberState.UNJOINED &&
generation.hasMemberId()` condition is false. I'm not actually sure what the
impact of doing so would be, though, so maybe it's not a big deal? But eg if
the coordinator is not known then who are we sending that LeaveGroup to? Will
the consumer end up blocking in `#close` until it can rediscover the
coordinator just so it can issue this LeaveGroup request?
Just wondering whether we should treat `LEAVE_GROUP` as a hard rule that
requires the consumer to attempt this LeaveGroup or if we should keep some of
the original logic and skip it under these conditions. To be specific, I'm
suggesting something like this:
```
if (!coordinatorUnknown() && state != MemberState.UNJOINED &&
generation.hasMemberId()) {
return opt == LEAVE_GROUP || (isDynamicMember && opt == DEFAULT);
} else {
return false;
}
```
--
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]
