gaurav-narula commented on code in PR #15836:
URL: https://github.com/apache/kafka/pull/15836#discussion_r1586362012
##########
core/src/main/scala/kafka/server/FetchSession.scala:
##########
@@ -603,14 +619,16 @@ class FetchSessionCache(private val maxEntries: Int,
// A map containing sessions which can be evicted by privileged sessions.
private val evictableByPrivileged = new util.TreeMap[EvictableKey,
FetchSession]
+ private val metricTag = Map("shard" -> s"$shardNum").asJava
+
// Set up metrics.
- metricsGroup.removeMetric(FetchSession.NUM_INCREMENTAL_FETCH_SESSIONS)
- metricsGroup.newGauge(FetchSession.NUM_INCREMENTAL_FETCH_SESSIONS, () =>
FetchSessionCache.this.size)
-
metricsGroup.removeMetric(FetchSession.NUM_INCREMENTAL_FETCH_PARTITIONS_CACHED)
- metricsGroup.newGauge(FetchSession.NUM_INCREMENTAL_FETCH_PARTITIONS_CACHED,
() => FetchSessionCache.this.totalPartitions)
-
metricsGroup.removeMetric(FetchSession.INCREMENTAL_FETCH_SESSIONS_EVICTIONS_PER_SEC)
+ metricsGroup.removeMetric(FetchSession.NUM_INCREMENTAL_FETCH_SESSIONS,
metricTag)
+ metricsGroup.newGauge(FetchSession.NUM_INCREMENTAL_FETCH_SESSIONS, () =>
FetchSessionCache.this.size, metricTag)
Review Comment:
That's a good point. The reason I added the labels was because of the
`removeMetric` calls. On further investigation, it seems that
`metricsGroup.removeMetric` isn't really needed and we can have a combined
metric for all the shards. The lambda that calculates the value for the gauges
needs to synchronize on the cache but it's a small overhead overall as this is
only when metrics are gathered.
--
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
