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

Reply via email to