squah-confluent commented on code in PR #20055:
URL: https://github.com/apache/kafka/pull/20055#discussion_r2188610204


##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java:
##########
@@ -2243,21 +2243,30 @@ private 
CoordinatorResult<ConsumerGroupHeartbeatResponseData, CoordinatorRecord>
         // epoch 0 and that it is fully initialized.
         boolean bumpGroupEpoch = group.groupEpoch() == 0;
 
-        bumpGroupEpoch |= hasMemberSubscriptionChanged(
+        boolean subscribedTopicNamesChanged = hasMemberSubscriptionChanged(
             groupId,
             member,
             updatedMember,
             records
         );
-
-        bumpGroupEpoch |= maybeUpdateRegularExpressions(
+        UpdateRegularExpressionsResult updateRegularExpressionsResult = 
maybeUpdateRegularExpressions(
             context,
             group,
             member,
             updatedMember,
             records
         );
 
+        // The subscription has changed when either the subscribed topic names 
or subscribed topic
+        // regex has changed.
+        boolean hasSubscriptionChanged = subscribedTopicNamesChanged || 
updateRegularExpressionsResult.subscribedTopicRegexChanged;

Review Comment:
   This kind of logic originally lived inline at the usage site. I moved it 
here after @dajac [was in favor of grouping it together up 
here](https://github.com/apache/kafka/pull/20055#discussion_r2174940775).



##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java:
##########
@@ -2243,21 +2243,30 @@ private 
CoordinatorResult<ConsumerGroupHeartbeatResponseData, CoordinatorRecord>
         // epoch 0 and that it is fully initialized.
         boolean bumpGroupEpoch = group.groupEpoch() == 0;
 
-        bumpGroupEpoch |= hasMemberSubscriptionChanged(
+        boolean subscribedTopicNamesChanged = hasMemberSubscriptionChanged(
             groupId,
             member,
             updatedMember,
             records
         );
-
-        bumpGroupEpoch |= maybeUpdateRegularExpressions(
+        UpdateRegularExpressionsResult updateRegularExpressionsResult = 
maybeUpdateRegularExpressions(
             context,
             group,
             member,
             updatedMember,
             records
         );
 
+        // The subscription has changed when either the subscribed topic names 
or subscribed topic
+        // regex has changed.
+        boolean hasSubscriptionChanged = subscribedTopicNamesChanged || 
updateRegularExpressionsResult.subscribedTopicRegexChanged;

Review Comment:
   This kind of logic originally lived inline at the usage site. I moved it 
here since @dajac [was in favor of grouping it together up 
here](https://github.com/apache/kafka/pull/20055#discussion_r2174940775).



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

Reply via email to