-
Notifications
You must be signed in to change notification settings - Fork 15.4k
KAFKA-16832: LeaveGroup API for upgrading ConsumerGroup #16057
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 15 commits
4df1310
f35b650
b5f2412
b0c817c
d0e9195
b0bbada
9336963
dffb61e
5b90022
500c5f3
953f27b
047cf01
74ced67
3800f01
d5f1062
080560d
bcd9776
b341030
9512b40
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2063,6 +2063,7 @@ private void removeMember(List<CoordinatorRecord> records, String groupId, Strin | |
| private void cancelTimers(String groupId, String memberId) { | ||
| cancelConsumerGroupSessionTimeout(groupId, memberId); | ||
| cancelConsumerGroupRebalanceTimeout(groupId, memberId); | ||
| cancelConsumerGroupJoinTimeout(groupId, memberId); | ||
| cancelConsumerGroupSyncTimeout(groupId, memberId); | ||
| } | ||
|
|
||
|
|
@@ -4424,14 +4425,128 @@ private ConsumerGroupMember validateConsumerGroupMember( | |
| * @param context The request context. | ||
| * @param request The actual LeaveGroup request. | ||
| * | ||
| * @return The LeaveGroup response and the records to append. | ||
| */ | ||
| public CoordinatorResult<LeaveGroupResponseData, CoordinatorRecord> classicGroupLeave( | ||
| RequestContext context, | ||
| LeaveGroupRequestData request | ||
| ) throws UnknownMemberIdException, GroupIdNotFoundException { | ||
|
dongnuo123 marked this conversation as resolved.
Outdated
|
||
| Group group = groups.get(request.groupId(), Long.MAX_VALUE); | ||
|
|
||
| if (group == null) { | ||
| throw new UnknownMemberIdException(String.format("Group %s not found.", request.groupId())); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i'm guessing we don't use as we throw unknown member id for the old coordinator, is this correct? it seems very counterintuitive unfortunately
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it's counterintuitive..
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps, a better strategy overall would be to always use
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like that approach. It's even more confusing now since we have mixed groups
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we hold onto #16073 then?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. I will update it. @dongnuo123 Could you please change it here? |
||
|
|
||
| if (group.type() == CLASSIC) { | ||
| return classicGroupLeaveToClassicGroup((ClassicGroup) group, context, request); | ||
| } else { | ||
| return classicGroupLeaveToConsumerGroup((ConsumerGroup) group, context, request); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Handle a classic LeaveGroupRequest to a ConsumerGroup. | ||
| * | ||
| * @param group The ConsumerGroup. | ||
| * @param context The request context. | ||
| * @param request The actual LeaveGroup request. | ||
| * | ||
| * @return The LeaveGroup response and the records to append. | ||
| */ | ||
| private CoordinatorResult<LeaveGroupResponseData, CoordinatorRecord> classicGroupLeaveToConsumerGroup( | ||
| ConsumerGroup group, | ||
| RequestContext context, | ||
| LeaveGroupRequestData request | ||
| ) throws UnknownMemberIdException { | ||
| String groupId = group.groupId(); | ||
| List<MemberResponse> memberResponses = new ArrayList<>(); | ||
| Set<ConsumerGroupMember> validLeaveGroupMembers = new HashSet<>(); | ||
| List<CoordinatorRecord> records = new ArrayList<>(); | ||
|
|
||
| for (MemberIdentity memberIdentity: request.members()) { | ||
|
dongnuo123 marked this conversation as resolved.
Outdated
|
||
| String memberId = memberIdentity.memberId(); | ||
| String instanceId = memberIdentity.groupInstanceId(); | ||
| String reason = memberIdentity.reason() != null ? memberIdentity.reason() : "not provided"; | ||
|
|
||
| ConsumerGroupMember member; | ||
| try { | ||
| if (instanceId == null) { | ||
| member = group.getOrMaybeCreateMember(memberId, false); | ||
| throwIfMemberDoesNotUseClassicProtocol(member); | ||
|
|
||
| log.info("[Group {}] Dynamic Member {} has left group " + | ||
| "through explicit `LeaveGroup` request; client reason: {}", | ||
| groupId, memberId, reason); | ||
| } else { | ||
| member = group.staticMember(instanceId); | ||
| throwIfStaticMemberIsUnknown(member, instanceId); | ||
| // The LeaveGroup API allows administrative removal of members by GroupInstanceId | ||
| // in which case we expect the MemberId to be undefined. | ||
| if (!UNKNOWN_MEMBER_ID.equals(memberId)) { | ||
| throwIfInstanceIdIsFenced(member, groupId, memberId, instanceId); | ||
| } | ||
| throwIfMemberDoesNotUseClassicProtocol(member); | ||
|
|
||
| memberId = member.memberId(); | ||
| log.info("[Group {}] Static Member {} with instance id {} has left group " + | ||
| "through explicit `LeaveGroup` request; client reason: {}", | ||
| groupId, memberId, instanceId, reason); | ||
| } | ||
|
|
||
| removeMember(records, groupId, memberId); | ||
| cancelTimers(groupId, memberId); | ||
| memberResponses.add( | ||
| new MemberResponse() | ||
| .setMemberId(memberId) | ||
| .setGroupInstanceId(instanceId) | ||
| ); | ||
| validLeaveGroupMembers.add(member); | ||
| } catch (KafkaException e) { | ||
| memberResponses.add( | ||
| new MemberResponse() | ||
| .setMemberId(memberId) | ||
| .setGroupInstanceId(instanceId) | ||
| .setErrorCode(Errors.forException(e).code()) | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| if (!records.isEmpty()) { | ||
| // Maybe update the subscription metadata. | ||
| Map<String, TopicMetadata> subscriptionMetadata = group.computeSubscriptionMetadata( | ||
| group.computeSubscribedTopicNames(new ArrayList<>(validLeaveGroupMembers)), | ||
|
dongnuo123 marked this conversation as resolved.
Outdated
|
||
| metadataImage.topics(), | ||
| metadataImage.cluster() | ||
| ); | ||
|
|
||
| if (!subscriptionMetadata.equals(group.subscriptionMetadata())) { | ||
| log.info("[GroupId {}] Computed new subscription metadata: {}.", | ||
| group.groupId(), subscriptionMetadata); | ||
| records.add(newGroupSubscriptionMetadataRecord(group.groupId(), subscriptionMetadata)); | ||
| } | ||
|
|
||
| // Bump the group epoch. | ||
| records.add(newGroupEpochRecord(groupId, group.groupEpoch() + 1)); | ||
| } | ||
|
|
||
| return new CoordinatorResult<>(records, new LeaveGroupResponseData().setMembers(memberResponses)); | ||
| } | ||
|
|
||
| /** | ||
| * Handle a classic LeaveGroupRequest to a ClassicGroup. | ||
| * | ||
| * @param group The ClassicGroup. | ||
| * @param context The request context. | ||
| * @param request The actual LeaveGroup request. | ||
| * | ||
| * @return The LeaveGroup response and the GroupMetadata record to append if the group | ||
| * no longer has any members. | ||
| */ | ||
| public CoordinatorResult<LeaveGroupResponseData, CoordinatorRecord> classicGroupLeave( | ||
| private CoordinatorResult<LeaveGroupResponseData, CoordinatorRecord> classicGroupLeaveToClassicGroup( | ||
| ClassicGroup group, | ||
| RequestContext context, | ||
| LeaveGroupRequestData request | ||
| ) throws UnknownMemberIdException, GroupIdNotFoundException { | ||
|
dongnuo123 marked this conversation as resolved.
Outdated
|
||
| ClassicGroup group = getOrMaybeCreateClassicGroup(request.groupId(), false); | ||
| if (group.isInState(DEAD)) { | ||
| return new CoordinatorResult<>( | ||
| Collections.emptyList(), | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.