dajac commented on code in PR #17958:
URL: https://github.com/apache/kafka/pull/17958#discussion_r1867245234
##########
core/src/test/scala/unit/kafka/server/ConsumerGroupDescribeRequestTest.scala:
##########
@@ -213,4 +219,108 @@ class ConsumerGroupDescribeRequestTest(cluster:
ClusterInstance) extends GroupCo
admin.close()
}
}
+
+ @ClusterTest(
+ types = Array(Type.KRAFT),
+ serverProperties = Array(
+ new ClusterConfigProperty(key =
GroupCoordinatorConfig.OFFSETS_TOPIC_PARTITIONS_CONFIG, value = "1"),
+ new ClusterConfigProperty(key =
GroupCoordinatorConfig.OFFSETS_TOPIC_REPLICATION_FACTOR_CONFIG, value = "1")
+ )
+ )
+ def testConsumerGroupDescribeWithMigrationMember(): Unit = {
+ // Creates the __consumer_offsets topics because it won't be created
automatically
+ // in this test because it does not use FindCoordinator API.
+ createOffsetsTopic()
+
+ // Create the topic.
+ val topicName = "foo"
+ createTopic(
+ topic = topicName,
+ numPartitions = 3
+ )
+
+ val groupId = "grp"
+
+ // Classic member 1 joins the classic group.
+ val memberId1 = joinDynamicConsumerGroupWithOldProtocol(
+ groupId = groupId,
+ metadata = ConsumerProtocol.serializeSubscription(
+ new ConsumerPartitionAssignor.Subscription(
+ Collections.singletonList(topicName),
+ null,
+ List().asJava
+ )
+ ).array,
+ assignment = ConsumerProtocol.serializeAssignment(
+ new ConsumerPartitionAssignor.Assignment(
+ List(0, 1, 2).map(p => new TopicPartition(topicName, p)).asJava
+ )
+ ).array
+ )._1
+
+ // The joining request with a consumer group member 2 is accepted.
+ val memberId2 = consumerGroupHeartbeat(
+ groupId = groupId,
+ memberId = "member-2",
+ rebalanceTimeoutMs = 5 * 60 * 1000,
+ subscribedTopicNames = List(topicName),
+ topicPartitions = List.empty,
+ expectedError = Errors.NONE
+ ).memberId
+
+ for (version <- ApiKeys.CONSUMER_GROUP_DESCRIBE.oldestVersion() to
ApiKeys.CONSUMER_GROUP_DESCRIBE.latestVersion(isUnstableApiEnabled)) {
+ val actual = consumerGroupDescribe(
+ groupIds = List(groupId),
+ includeAuthorizedOperations = true,
+ version = version.toShort,
+ )
+ assertEquals(1, actual.size)
+ val group = actual.head
+ val member1 = group.members().asScala.find(_.memberId == memberId1)
Review Comment:
nit: You can omit the `()` after `members`.
##########
core/src/test/scala/unit/kafka/server/ConsumerGroupDescribeRequestTest.scala:
##########
@@ -213,4 +219,108 @@ class ConsumerGroupDescribeRequestTest(cluster:
ClusterInstance) extends GroupCo
admin.close()
}
}
+
+ @ClusterTest(
+ types = Array(Type.KRAFT),
+ serverProperties = Array(
+ new ClusterConfigProperty(key =
GroupCoordinatorConfig.OFFSETS_TOPIC_PARTITIONS_CONFIG, value = "1"),
+ new ClusterConfigProperty(key =
GroupCoordinatorConfig.OFFSETS_TOPIC_REPLICATION_FACTOR_CONFIG, value = "1")
+ )
+ )
+ def testConsumerGroupDescribeWithMigrationMember(): Unit = {
+ // Creates the __consumer_offsets topics because it won't be created
automatically
+ // in this test because it does not use FindCoordinator API.
+ createOffsetsTopic()
+
+ // Create the topic.
+ val topicName = "foo"
+ createTopic(
+ topic = topicName,
+ numPartitions = 3
+ )
+
+ val groupId = "grp"
+
+ // Classic member 1 joins the classic group.
+ val memberId1 = joinDynamicConsumerGroupWithOldProtocol(
+ groupId = groupId,
+ metadata = ConsumerProtocol.serializeSubscription(
+ new ConsumerPartitionAssignor.Subscription(
+ Collections.singletonList(topicName),
+ null,
+ List().asJava
+ )
+ ).array,
+ assignment = ConsumerProtocol.serializeAssignment(
+ new ConsumerPartitionAssignor.Assignment(
+ List(0, 1, 2).map(p => new TopicPartition(topicName, p)).asJava
+ )
+ ).array
+ )._1
+
+ // The joining request with a consumer group member 2 is accepted.
+ val memberId2 = consumerGroupHeartbeat(
+ groupId = groupId,
+ memberId = "member-2",
+ rebalanceTimeoutMs = 5 * 60 * 1000,
+ subscribedTopicNames = List(topicName),
+ topicPartitions = List.empty,
+ expectedError = Errors.NONE
+ ).memberId
+
+ for (version <- ApiKeys.CONSUMER_GROUP_DESCRIBE.oldestVersion() to
ApiKeys.CONSUMER_GROUP_DESCRIBE.latestVersion(isUnstableApiEnabled)) {
+ val actual = consumerGroupDescribe(
+ groupIds = List(groupId),
+ includeAuthorizedOperations = true,
+ version = version.toShort,
+ )
+ assertEquals(1, actual.size)
+ val group = actual.head
+ val member1 = group.members().asScala.find(_.memberId == memberId1)
+ assertFalse(member1.isEmpty)
+ // Version 0 doesn't have isClassic field, so isClassic field on member
1 is false.
+ // After version 1, there is isClassic field and it should be true for
member 1.
+ assertEquals(if (version == 0) -1.toByte else 1.toByte,
member1.get.isClassic)
+
+ val member2 = group.members().asScala.find(_.memberId == memberId2)
Review Comment:
nit: You can omit the `()` after `members`.
##########
core/src/test/scala/unit/kafka/server/ConsumerGroupDescribeRequestTest.scala:
##########
@@ -213,4 +219,108 @@ class ConsumerGroupDescribeRequestTest(cluster:
ClusterInstance) extends GroupCo
admin.close()
}
}
+
+ @ClusterTest(
+ types = Array(Type.KRAFT),
+ serverProperties = Array(
+ new ClusterConfigProperty(key =
GroupCoordinatorConfig.OFFSETS_TOPIC_PARTITIONS_CONFIG, value = "1"),
+ new ClusterConfigProperty(key =
GroupCoordinatorConfig.OFFSETS_TOPIC_REPLICATION_FACTOR_CONFIG, value = "1")
+ )
+ )
+ def testConsumerGroupDescribeWithMigrationMember(): Unit = {
+ // Creates the __consumer_offsets topics because it won't be created
automatically
+ // in this test because it does not use FindCoordinator API.
+ createOffsetsTopic()
+
+ // Create the topic.
+ val topicName = "foo"
+ createTopic(
+ topic = topicName,
+ numPartitions = 3
+ )
+
+ val groupId = "grp"
+
+ // Classic member 1 joins the classic group.
+ val memberId1 = joinDynamicConsumerGroupWithOldProtocol(
+ groupId = groupId,
+ metadata = ConsumerProtocol.serializeSubscription(
+ new ConsumerPartitionAssignor.Subscription(
+ Collections.singletonList(topicName),
+ null,
+ List().asJava
+ )
+ ).array,
+ assignment = ConsumerProtocol.serializeAssignment(
+ new ConsumerPartitionAssignor.Assignment(
+ List(0, 1, 2).map(p => new TopicPartition(topicName, p)).asJava
+ )
+ ).array
+ )._1
+
+ // The joining request with a consumer group member 2 is accepted.
+ val memberId2 = consumerGroupHeartbeat(
+ groupId = groupId,
+ memberId = "member-2",
+ rebalanceTimeoutMs = 5 * 60 * 1000,
+ subscribedTopicNames = List(topicName),
+ topicPartitions = List.empty,
+ expectedError = Errors.NONE
+ ).memberId
+
+ for (version <- ApiKeys.CONSUMER_GROUP_DESCRIBE.oldestVersion() to
ApiKeys.CONSUMER_GROUP_DESCRIBE.latestVersion(isUnstableApiEnabled)) {
+ val actual = consumerGroupDescribe(
+ groupIds = List(groupId),
+ includeAuthorizedOperations = true,
+ version = version.toShort,
+ )
+ assertEquals(1, actual.size)
+ val group = actual.head
+ val member1 = group.members().asScala.find(_.memberId == memberId1)
+ assertFalse(member1.isEmpty)
+ // Version 0 doesn't have isClassic field, so isClassic field on member
1 is false.
+ // After version 1, there is isClassic field and it should be true for
member 1.
+ assertEquals(if (version == 0) -1.toByte else 1.toByte,
member1.get.isClassic)
+
+ val member2 = group.members().asScala.find(_.memberId == memberId2)
+ assertFalse(member2.isEmpty)
+ assertEquals(if (version == 0) -1.toByte else 0.toByte,
member2.get.isClassic)
+ }
+
+ // Classic member 1 leaves group.
+ leaveGroup(
+ groupId = groupId,
+ memberId = memberId1,
+ useNewProtocol = false,
+ version = ApiKeys.LEAVE_GROUP.latestVersion(isUnstableApiEnabled)
+ )
+
+ // Member 1 joins as consumer group member.
+ consumerGroupHeartbeat(
+ groupId = groupId,
+ memberId = memberId1,
+ rebalanceTimeoutMs = 5 * 60 * 1000,
+ subscribedTopicNames = List(topicName),
+ topicPartitions = List.empty,
+ expectedError = Errors.NONE
+ )
+
+ // There is no classic member in the group.
+ for (version <- ApiKeys.CONSUMER_GROUP_DESCRIBE.oldestVersion() to
ApiKeys.CONSUMER_GROUP_DESCRIBE.latestVersion(isUnstableApiEnabled)) {
+ val actual = consumerGroupDescribe(
+ groupIds = List(groupId),
+ includeAuthorizedOperations = true,
+ version = version.toShort,
+ )
+ assertEquals(1, actual.size)
+ val group = actual.head
+ val member1 = group.members().asScala.find(_.memberId == memberId1)
Review Comment:
nit: You can omit the `()` after `members`.
##########
core/src/test/scala/unit/kafka/server/ConsumerGroupDescribeRequestTest.scala:
##########
@@ -213,4 +219,108 @@ class ConsumerGroupDescribeRequestTest(cluster:
ClusterInstance) extends GroupCo
admin.close()
}
}
+
+ @ClusterTest(
+ types = Array(Type.KRAFT),
+ serverProperties = Array(
+ new ClusterConfigProperty(key =
GroupCoordinatorConfig.OFFSETS_TOPIC_PARTITIONS_CONFIG, value = "1"),
+ new ClusterConfigProperty(key =
GroupCoordinatorConfig.OFFSETS_TOPIC_REPLICATION_FACTOR_CONFIG, value = "1")
+ )
+ )
+ def testConsumerGroupDescribeWithMigrationMember(): Unit = {
+ // Creates the __consumer_offsets topics because it won't be created
automatically
+ // in this test because it does not use FindCoordinator API.
+ createOffsetsTopic()
+
+ // Create the topic.
+ val topicName = "foo"
+ createTopic(
+ topic = topicName,
+ numPartitions = 3
+ )
+
+ val groupId = "grp"
+
+ // Classic member 1 joins the classic group.
+ val memberId1 = joinDynamicConsumerGroupWithOldProtocol(
+ groupId = groupId,
+ metadata = ConsumerProtocol.serializeSubscription(
+ new ConsumerPartitionAssignor.Subscription(
+ Collections.singletonList(topicName),
+ null,
+ List().asJava
+ )
+ ).array,
+ assignment = ConsumerProtocol.serializeAssignment(
+ new ConsumerPartitionAssignor.Assignment(
+ List(0, 1, 2).map(p => new TopicPartition(topicName, p)).asJava
+ )
+ ).array
+ )._1
+
+ // The joining request with a consumer group member 2 is accepted.
+ val memberId2 = consumerGroupHeartbeat(
+ groupId = groupId,
+ memberId = "member-2",
+ rebalanceTimeoutMs = 5 * 60 * 1000,
+ subscribedTopicNames = List(topicName),
+ topicPartitions = List.empty,
+ expectedError = Errors.NONE
+ ).memberId
+
+ for (version <- ApiKeys.CONSUMER_GROUP_DESCRIBE.oldestVersion() to
ApiKeys.CONSUMER_GROUP_DESCRIBE.latestVersion(isUnstableApiEnabled)) {
+ val actual = consumerGroupDescribe(
+ groupIds = List(groupId),
+ includeAuthorizedOperations = true,
+ version = version.toShort,
+ )
+ assertEquals(1, actual.size)
+ val group = actual.head
+ val member1 = group.members().asScala.find(_.memberId == memberId1)
+ assertFalse(member1.isEmpty)
+ // Version 0 doesn't have isClassic field, so isClassic field on member
1 is false.
+ // After version 1, there is isClassic field and it should be true for
member 1.
+ assertEquals(if (version == 0) -1.toByte else 1.toByte,
member1.get.isClassic)
+
+ val member2 = group.members().asScala.find(_.memberId == memberId2)
+ assertFalse(member2.isEmpty)
+ assertEquals(if (version == 0) -1.toByte else 0.toByte,
member2.get.isClassic)
+ }
+
+ // Classic member 1 leaves group.
+ leaveGroup(
+ groupId = groupId,
+ memberId = memberId1,
+ useNewProtocol = false,
+ version = ApiKeys.LEAVE_GROUP.latestVersion(isUnstableApiEnabled)
+ )
+
+ // Member 1 joins as consumer group member.
+ consumerGroupHeartbeat(
+ groupId = groupId,
+ memberId = memberId1,
+ rebalanceTimeoutMs = 5 * 60 * 1000,
+ subscribedTopicNames = List(topicName),
+ topicPartitions = List.empty,
+ expectedError = Errors.NONE
+ )
+
+ // There is no classic member in the group.
+ for (version <- ApiKeys.CONSUMER_GROUP_DESCRIBE.oldestVersion() to
ApiKeys.CONSUMER_GROUP_DESCRIBE.latestVersion(isUnstableApiEnabled)) {
+ val actual = consumerGroupDescribe(
+ groupIds = List(groupId),
+ includeAuthorizedOperations = true,
+ version = version.toShort,
+ )
+ assertEquals(1, actual.size)
+ val group = actual.head
+ val member1 = group.members().asScala.find(_.memberId == memberId1)
+ assertFalse(member1.isEmpty)
+ assertEquals(if (version == 0) -1.toByte else 0.toByte,
member1.get.isClassic)
+
+ val member2 = group.members().asScala.find(_.memberId == memberId2)
Review Comment:
nit: You can omit the `()` after `members`.
--
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
