majialoong commented on code in PR #20055:
URL: https://github.com/apache/kafka/pull/20055#discussion_r2441565642
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/share/ShareGroupAssignmentBuilder.java:
##########
@@ -83,11 +125,38 @@ public ShareGroupMember build() {
// when the member is updated.
return new ShareGroupMember.Builder(member)
.setState(MemberState.STABLE)
- .setAssignedPartitions(targetAssignment.partitions())
+ // If we have client-side assignors, the latest target
assignment may not
+ // be consistent with the latest subscribed topics, so we must
always
+ // filter the assigned partitions to ensure they are
consistent with the
+ // subscribed topics.
+
.setAssignedPartitions(filterAssignedPartitions(targetAssignment.partitions(),
member.subscribedTopicNames()))
.updateMemberEpoch(targetAssignmentEpoch)
.build();
+ } else if (hasSubscriptionChanged) {
+ return new ShareGroupMember.Builder(member)
+
.setAssignedPartitions(filterAssignedPartitions(targetAssignment.partitions(),
member.subscribedTopicNames()))
+ .build();
+ } else {
+ return member;
}
+ }
- return member;
+ private Map<Uuid, Set<Integer>> filterAssignedPartitions(
+ Map<Uuid, Set<Integer>> partitions,
+ Set<String> subscribedTopicNames
+ ) {
+ TopicIds subscribedTopicIds = new
TopicIds(member.subscribedTopicNames(), metadataImage);
Review Comment:
Hi @squah-confluent , I noticed that in the `filterAssignedPartitions`
method, the parameter `subscribedTopicNames` is not used directly; instead,
`member.subscribedTopicNames()` is called.
Although their values are currently the same, this could be confusing for
future readers of the code. For clarity and to avoid potential
misunderstandings, could you please consider using the passed-in
`subscribedTopicNames` parameter directly in this method?
When you have time, please take a look. If you think it's necessary to make
this change, I would be glad to submit a minor patch to address it. Thank you!
--
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]
