apoorvmittal10 commented on code in PR #15251: URL: https://github.com/apache/kafka/pull/15251#discussion_r1470278382
########## server/src/main/java/org/apache/kafka/server/ClientMetricsManager.java: ########## @@ -288,6 +307,9 @@ private ClientMetricsInstance createClientInstanceAndUpdateCache(Uuid clientInst ClientMetricsInstanceMetadata instanceMetadata) { ClientMetricsInstance clientInstance = createClientInstance(clientInstanceId, instanceMetadata); + // Maybe add client metrics, if metrics not already added. Metrics might be already added + // if the client instance was evicted from the cache because of size limit. Review Comment: The problem is that the eviction on size happens inside common LRUCache class where the code from this manager cannot be instrumented. I do not see any custom override that can be provided to LRUCache class. Having said that, there won't be any hanging additional instance metrics sensors when eviction happens on size as anyways time-based eviction will remove such instance metrics. Also, if the respective instance reports metrics prior time-based eviction, then the metrics numbers for that instance will be correctly reported as well. Hence, I do think we should not evict metrics sensors on size-based eviction. -- 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