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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
