Re: [PR] MINOR: ConsumerGroup#getOrMaybeCreateMember should not add the member to the group [kafka]

[via GitHub]

chia7712 commented on code in PR #15847:
URL: https://github.com/apache/kafka/pull/15847#discussion_r1588652533


##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/consumer/TargetAssignmentBuilder.java:
##########
@@ -252,14 +252,16 @@ public TargetAssignmentResult build() throws 
PartitionAssignorException {
             if (updatedMemberOrNull == null) {
                 memberSpecs.remove(memberId);
             } else {
-                ConsumerGroupMember member = members.get(memberId);
-                Assignment assignment;
+                Assignment assignment = 
targetAssignment.getOrDefault(memberId, Assignment.EMPTY);
+
                 // A new static member joins and needs to replace an existing 
departed one.
-                if (member == null && 
staticMembers.containsKey(updatedMemberOrNull.instanceId())) {
-                    assignment = 
targetAssignment.getOrDefault(staticMembers.get(updatedMemberOrNull.instanceId()),
 Assignment.EMPTY);
-                } else {
-                    assignment = targetAssignment.getOrDefault(memberId, 
Assignment.EMPTY);
+                if (updatedMemberOrNull.instanceId() != null) {
+                    String previousMemberId = 
staticMembers.get(updatedMemberOrNull.instanceId());
+                    if (previousMemberId != null) {
+                        assignment = 
targetAssignment.getOrDefault(updatedMemberOrNull.instanceId(), 
Assignment.EMPTY);

Review Comment:
   not sure whether `updatedMemberOrNull.instanceId()` should be replaced by 
`previousMemberId`?



##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/consumer/ConsumerGroup.java:
##########
@@ -347,9 +348,7 @@ public ConsumerGroupMember getOrMaybeCreateMember(
                     memberId, groupId));
             }
             member = new ConsumerGroupMember.Builder(memberId).build();

Review Comment:
   Maybe we can do a bit refactor:
   ```java
           ConsumerGroupMember member = members.get(memberId);
           if (member != null) return member;
   
           if (!createIfNotExists)
               throw new UnknownMemberIdException(String.format("Member %s is 
not a member of group %s.", memberId, groupId));
   
           return new ConsumerGroupMember.Builder(memberId).build();
   ```



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org