dajac commented on code in PR #22708:
URL: https://github.com/apache/kafka/pull/22708#discussion_r3499938783


##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/streams/StreamsGroup.java:
##########
@@ -503,17 +503,23 @@ public void updateMember(StreamsGroupMember newMember) {
         }
         StreamsGroupMember oldMember = members.put(newMember.memberId(), 
newMember);
         maybeUpdateTaskProcessId(oldMember, newMember);
-        updateStaticMember(newMember);
+        updateStaticMember(oldMember, newMember);
         maybeUpdateGroupState();
         endpointToPartitionsCache.remove(newMember.memberId());
     }
 
     /**
      * Updates the member ID stored against the instance ID if the member is a 
static member.
      *
+     * @param oldMember The old member state.
      * @param newMember The new member state.
      */
-    private void updateStaticMember(StreamsGroupMember newMember) {
+    private void updateStaticMember(StreamsGroupMember oldMember, 
StreamsGroupMember newMember) {
+        if (oldMember != null && oldMember.instanceId() != null &&
+            oldMember.instanceId().isPresent() &&
+            !oldMember.instanceId().equals(newMember.instanceId())) {

Review Comment:
   nit: All the conditions are hard to read. I wonder if using `ifPresent` 
could help. Can `instanceId` really be `null` if it is an `Optional`?



##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/consumer/ConsumerGroup.java:
##########
@@ -334,9 +334,14 @@ public void updateMember(ConsumerGroupMember newMember) {
     /**
      * Updates the member id stored against the instance id if the member is a 
static member.
      *
+     * @param oldMember The old member state.
      * @param newMember The new member state.
      */
-    private void updateStaticMember(ConsumerGroupMember newMember) {
+    private void updateStaticMember(ConsumerGroupMember oldMember, 
ConsumerGroupMember newMember) {
+        if (oldMember != null && oldMember.instanceId() != null &&
+            !oldMember.instanceId().equals(newMember.instanceId())) {

Review Comment:
   nit: Does it fit on one line? Could we just remove it anyway as we add it 
back afterwards?



-- 
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]

Reply via email to