lucasbru commented on code in PR #23762:
URL: https://github.com/apache/kafka/pull/23762#discussion_r4313392564
##########
tools/src/main/java/org/apache/kafka/tools/streams/TopologyDescriptionFormatter.java:
##########
@@ -44,14 +46,32 @@ public static String format(final
StreamsGroupTopologyDescription topology) {
sb.append(" ");
appendSubtopology(sb, subtopology);
}
- int globalStoreId = topology.subtopologies().size();
+ // Kafka Streams numbers subtopologies and global stores from a single
counter, and both lists are sent
+ // sorted by id, so the global store ids are exactly the ids not taken
by a subtopology, in ascending order.
Review Comment:
This depends on `TopologyDescriptionConverter.toWire` sorting both lists and
on the topology description plugin keeping that order. Neither the protocol nor
`StreamsGroupTopologyDescriptionPlugin` guarantees it. Could we say it is an
assumption and point to where the sort happens?
Longer term it would be more robust to carry the global store id in
`TopologyDescriptionGlobalStore`, but since these RPCs shipped in 4.4.0 that
needs new versions and a KIP, so not for this PR.
##########
tools/src/main/java/org/apache/kafka/tools/streams/TopologyDescriptionFormatter.java:
##########
@@ -44,14 +46,32 @@ public static String format(final
StreamsGroupTopologyDescription topology) {
sb.append(" ");
appendSubtopology(sb, subtopology);
}
- int globalStoreId = topology.subtopologies().size();
+ // Kafka Streams numbers subtopologies and global stores from a single
counter, and both lists are sent
+ // sorted by id, so the global store ids are exactly the ids not taken
by a subtopology, in ascending order.
+ final Set<Integer> subtopologyIds =
numericIds(topology.subtopologies());
+ int globalStoreId = 0;
for (final GlobalStore globalStore : topology.globalStores()) {
Review Comment:
Now that the labels are right, the order still differs from
`Topology#describe()`, which interleaves subtopologies and global stores by id
(see `InternalTopologyBuilder.TopologyDescription#toString`). With a global
store at 0 and subtopologies 1, 2, the app logs show the global store first,
while here it comes last. It would be nice to match since the class javadoc
says we mirror `describe()`.
--
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]
