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