showuon commented on a change in pull request #11688:
URL: https://github.com/apache/kafka/pull/11688#discussion_r802633329
##########
File path:
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java
##########
@@ -648,10 +648,14 @@ private void maybeUpdateGroupSubscription(String
assignorName,
updateGroupSubscription(allSubscribedTopics);
isLeader = true;
- assignmentSnapshot = metadataSnapshot;
- if (skipAssignment)
+
Review comment:
nit: extra empty line here
##########
File path:
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinatorTest.java
##########
@@ -1601,6 +1604,36 @@ public void
testStaticLeaderRejoinsGroupAndCanTriggersRebalance() {
assertTrue(coordinator.rejoinNeededOrPending());
}
+ @Test
+ public void
testStaticLeaderRejoinsGroupAndCanDetectMetadataChangesForOtherMembers() {
Review comment:
nice new test added.
##########
File path:
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java
##########
@@ -648,10 +648,14 @@ private void maybeUpdateGroupSubscription(String
assignorName,
updateGroupSubscription(allSubscribedTopics);
isLeader = true;
- assignmentSnapshot = metadataSnapshot;
- if (skipAssignment)
+
+ if (skipAssignment) {
+ log.info("Skipped assignment for returning static leader at
generation {}. The static leader " +
+ "will collect its existing assignment.",
generation().generationId);
Review comment:
I'm not sure if we need to put the 2nd sentence. Maybe the 1st one is
enough? Or maybe change the 2nd one with:
`The static leader will collect its existing assignment with empty
assignment syncGroup request.`
WDYT?
--
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
