AndrewJSchofield commented on code in PR #17367: URL: https://github.com/apache/kafka/pull/17367#discussion_r1836935229
########## clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java: ########## @@ -407,6 +408,7 @@ public class KafkaAdminClient extends AdminClient { private final ExponentialBackoff retryBackoff; private final boolean clientTelemetryEnabled; private final MetadataRecoveryStrategy metadataRecoveryStrategy; + private final Map<TopicPartition, Integer> partitionLeaderCache; Review Comment: I've been thinking about this and I'm not so sure. With `metadata.max.age.ms`, there is typically a reason to need to refresh the metadata, such as discovering new topics that match a wildcard or learning about new partitions on an existing topic. Here's a cache entry is good until the partition leader changes. With this cache, it could contain an entry for each topic-partition whose leader information was required in order to perform admin actions using the PartitionLeaderStrategy. If an entry is stale, the fulfilment stage will fail with `NOT_LEADER_OR_FOLLOWER` and the lookup will be performed again. Would it really be better to mistrust the cached data after say 5 minutes and lookup again anyway? I'm not convinced that's true. -- 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