kirktrue commented on code in PR #19199:
URL: https://github.com/apache/kafka/pull/19199#discussion_r2002151201
##########
clients/src/main/java/org/apache/kafka/clients/Metadata.java:
##########
@@ -422,21 +423,17 @@ public synchronized Set<TopicPartition>
updatePartitionLeadership(Map<TopicParti
updatePartitionMetadata.add(updatedMetadata);
lastSeenLeaderEpochs.put(partition, newLeader.epoch.get());
+
+ final String updatedTopic = updatedMetadata.topic();
+ if (this.metadataSnapshot.topicIds().containsKey(updatedTopic))
+ topicIdsForUpdatedTopics.put(updatedTopic,
this.metadataSnapshot.topicIds().get(updatedTopic));
Review Comment:
This is bit uglier, but it avoids the second hash map lookup.
```suggestion
Uuid topicUuid =
this.metadataSnapshot.topicIds().getOrDefault(updatedTopic, Uuid.ZERO_UUID);
if (!topicUuid.equals(Uuid.ZERO_UUID))
topicIdsForUpdatedTopics.put(updatedTopic, topicUuid);
```
##########
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:
The existing code calls `newMetadataByPartition.putIfAbsent()`. Is that
something that should be retained here?
##########
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.
Review Comment:
Is it possible to reword the comments rather that removing them completely?
--
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
