dajac commented on code in PR #15785:
URL: https://github.com/apache/kafka/pull/15785#discussion_r1576317772
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/consumer/ConsumerGroup.java:
##########
@@ -918,40 +935,53 @@ private void
maybeUpdateClassicProtocolMembersSupportedProtocols(
/**
* Updates the subscribed topic names count.
+ * Changes to the subscription model as a consequence
+ * of this update is reflected as well.
*
* @param oldMember The old member.
* @param newMember The new member.
*/
- private void maybeUpdateSubscribedTopicNames(
+ private void maybeUpdateSubscribedTopicNamesAndGroupSubscriptionModel(
ConsumerGroupMember oldMember,
ConsumerGroupMember newMember
) {
- maybeUpdateSubscribedTopicNames(subscribedTopicNames, oldMember,
newMember);
+
maybeUpdateSubscribedTopicNamesAndGroupSubscriptionModel(subscribedTopicNames,
oldMember, newMember);
}
/**
- * Updates the subscription count.
+ * Updates the subscription count and the subscription model, if required.
*
* @param subscribedTopicCount The map to update.
* @param oldMember The old member.
* @param newMember The new member.
*/
- private static void maybeUpdateSubscribedTopicNames(
+ private void maybeUpdateSubscribedTopicNamesAndGroupSubscriptionModel(
Review Comment:
Should we add unit test to validate this logic?
--
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
