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

Reply via email to