mjsax commented on code in PR #20574:
URL: https://github.com/apache/kafka/pull/20574#discussion_r2373691400
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -4463,6 +4463,47 @@ class PlaintextAdminIntegrationTest extends
BaseAdminIntegrationTest {
}
}
+ @Test
+ def testDescribeStreamsGroupsNotReady(): Unit = {
+ val streamsGroupId = "stream_group_id"
+ val testTopicName = "test_topic"
+
+ val config = createConfig
+ client = Admin.create(config)
+
+ val streams = createStreamsGroup(
+ inputTopic = testTopicName,
+ streamsGroupId = streamsGroupId
+ )
+ streams.poll(JDuration.ofMillis(500L))
+
+ try {
+ TestUtils.waitUntilTrue(() => {
+ val firstGroup = client.listGroups().all().get().stream()
+ .filter(g => g.groupId() == streamsGroupId).findFirst().orElse(null)
+ firstGroup.groupState().orElse(null) == GroupState.NOT_READY &&
firstGroup.groupId() == streamsGroupId
+ }, "Stream group not NOT_READY yet")
Review Comment:
How does this test ensure, that the group stays in NOT_READY state (ie, does
not go into ASSIGNING, RECONCILING, or STABLE) ?
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/streams/StreamsGroup.java:
##########
@@ -1039,7 +1039,16 @@ public StreamsGroupDescribeResponseData.DescribedGroup
asDescribedGroup(
.setGroupEpoch(groupEpoch.get(committedOffset))
.setGroupState(state.get(committedOffset).toString())
.setAssignmentEpoch(targetAssignmentEpoch.get(committedOffset))
-
.setTopology(configuredTopology.get(committedOffset).map(ConfiguredTopology::asStreamsGroupDescribeTopology).orElse(null));
+ .setTopology(
+ configuredTopology.get(committedOffset)
+ .filter(ConfiguredTopology::isReady)
+ .map(ConfiguredTopology::asStreamsGroupDescribeTopology)
+ .orElse(
+ topology.get(committedOffset)
+
.map(StreamsTopology::asStreamsGroupDescribeTopology)
+ .orElse(null)
Review Comment:
Is `orElse(null)` a valid case? Should we not always have a topology, even
if it's not configured yet?
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/streams/StreamsTopology.java:
##########
@@ -95,4 +97,43 @@ public static StreamsTopology
fromHeartbeatRequest(StreamsGroupHeartbeatRequestD
.collect(Collectors.toMap(StreamsGroupTopologyValue.Subtopology::subtopologyId,
x -> x));
return new StreamsTopology(topology.epoch(), subtopologyMap);
}
+
+ public StreamsGroupDescribeResponseData.Topology
asStreamsGroupDescribeTopology() {
+ return new StreamsGroupDescribeResponseData.Topology()
+ .setEpoch(topologyEpoch)
+ .setSubtopologies(
+ subtopologies.entrySet().stream()
+ .sorted(Map.Entry.comparingByKey())
+ .map(entry ->
asStreamsGroupDescribeSubtopology(entry.getKey(), entry.getValue()))
+ .toList()
+ );
+ }
+
+ private StreamsGroupDescribeResponseData.Subtopology
asStreamsGroupDescribeSubtopology(String subtopologyId,
StreamsGroupTopologyValue.Subtopology subtopology) {
+ return new StreamsGroupDescribeResponseData.Subtopology()
+ .setSubtopologyId(subtopologyId)
+
.setSourceTopics(subtopology.sourceTopics().stream().sorted().toList())
+
.setRepartitionSinkTopics(subtopology.repartitionSinkTopics().stream().sorted().toList())
+
.setRepartitionSourceTopics(subtopology.repartitionSourceTopics().stream()
+ .map(this::asStreamsGroupDescribeTopicInfo)
+
.sorted(Comparator.comparing(StreamsGroupDescribeResponseData.TopicInfo::name)).toList())
+
.setStateChangelogTopics(subtopology.stateChangelogTopics().stream()
+ .map(this::asStreamsGroupDescribeTopicInfo)
+
.sorted(Comparator.comparing(StreamsGroupDescribeResponseData.TopicInfo::name)).toList());
+ }
+
+ private StreamsGroupDescribeResponseData.TopicInfo
asStreamsGroupDescribeTopicInfo(StreamsGroupTopologyValue.TopicInfo topicInfo) {
+ return new StreamsGroupDescribeResponseData.TopicInfo()
+ .setName(topicInfo.name())
+ .setPartitions(topicInfo.partitions())
+ .setReplicationFactor(topicInfo.replicationFactor())
+ .setTopicConfigs(
+ topicInfo.topicConfigs().stream().map(
+ y -> new StreamsGroupDescribeResponseData.KeyValue()
Review Comment:
Why `y` ? Can't we find a better name?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
