ableegoldman commented on code in PR #17614:
URL: https://github.com/apache/kafka/pull/17614#discussion_r2029660460
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerMembershipManager.java:
##########
@@ -469,8 +493,17 @@ public int joinGroupEpoch() {
*/
@Override
public int leaveGroupEpoch() {
- return groupInstanceId.isPresent() ?
- ConsumerGroupHeartbeatRequest.LEAVE_GROUP_STATIC_MEMBER_EPOCH :
- ConsumerGroupHeartbeatRequest.LEAVE_GROUP_MEMBER_EPOCH;
+ boolean isStaticMember = groupInstanceId.isPresent();
+ if (REMAIN_IN_GROUP == leaveGroupOperation && isStaticMember) {
+ return
ConsumerGroupHeartbeatRequest.LEAVE_GROUP_STATIC_MEMBER_EPOCH;
+ }
+
+ if (LEAVE_GROUP == leaveGroupOperation) {
+ return ConsumerGroupHeartbeatRequest.LEAVE_GROUP_MEMBER_EPOCH;
Review Comment:
I think I understand why this condition is needed but it would be good to
leave a comment explaining this change because it's pretty obscure. IIUC
basically the problem is that there's no way to permanently leave the group
with a static consumer right now, and the only method of removing static
members is via an admin API. So we essentially have to "trick" the
GroupMetadataManager into fencing this member to kick it out of the group,
because if we use the `LEAVE_GROUP_STATIC_MEMBER_EPOCH ` then it thinks it's
just being "temporarily" removed and this won't actually result in the consumer
leaving the group. So we use `LEAVE_GROUP_MEMBER_EPOCH` because anything that
isn't the `LEAVE_GROUP_STATIC_MEMBER_EPOCH` triggers the GroupMetadataManager
to fence the consumer in its `#consumerGroupLeave` method.
First off does that summary sound right? It's also pretty hacky, and while I
think it's fine to proceed with this for now, I'm wondering if we shouldn't
just try to implement proper LeaveGroup mechanics for static membership. I know
this would require some server-side changes but maybe this can be a followup
KIP? Again, no need to block this PR on that, just putting it out there. Feel
free to pick that up yourself or file a ticket if that makes sense to you too
--
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]
