lorcanj commented on code in PR #19199:
URL: https://github.com/apache/kafka/pull/19199#discussion_r2004919701
##########
clients/src/main/java/org/apache/kafka/clients/MetadataSnapshot.java:
##########
@@ -173,12 +173,15 @@ MetadataSnapshot mergeWith(String newClusterId,
Map<TopicPartition, PartitionMetadata> newMetadataByPartition = new
HashMap<>(addPartitions.size());
- // We want the most recent topic ID. We start with the previous ID
stored for retained topics and then
- // update with newest information from the MetadataResponse. We always
take the latest state, removing existing
- // topic IDs if the latest state contains the topic name but not a
topic ID.
- Map<String, Uuid> newTopicIds = this.topicIds.entrySet().stream()
- .filter(entry -> shouldRetainTopic.test(entry.getKey()))
- .collect(Collectors.toMap(Map.Entry::getKey,
Map.Entry::getValue));
+ Map<String, Uuid> newTopicIds = new HashMap<>(this.topicIds.size());
+
+ for (Map.Entry<TopicPartition, PartitionMetadata> entry :
metadataByPartition.entrySet()) {
+ if (shouldRetainTopic.test(entry.getKey().topic())) {
+ newMetadataByPartition.put(entry.getKey(), entry.getValue());
Review Comment:
I think I changed it because previously it was primarily using
`putIfAbsent()` to ensure that the newest information from the MetadataResponse
wasn't being overwritten, which is now different because of the code re-order
where this is now done first.
I guess if there are duplicate entries there isn't any need to put if there
is an entry for the topic partition, but I don't have a full grasp of what this
map would look like populated with real-world data. I think I tested with both
and it didn't seem to affect the unit tests.
Happy to go with either approach.
--
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
