[GitHub] [kafka] hshi2022 commented on a diff in pull request #12634: KAFKA-14225: Fix deadlock caused by lazy val exemptSensor

[GitBox <gi...@apache.org>]

hshi2022 commented on code in PR #12634:
URL: https://github.com/apache/kafka/pull/12634#discussion_r982817712


##########
core/src/main/scala/kafka/server/ClientRequestQuotaManager.scala:
##########
@@ -30,6 +30,9 @@ import scala.jdk.CollectionConverters._
 object ClientRequestQuotaManager {
   val QuotaRequestPercentDefault = Int.MaxValue.toDouble
   val NanosToPercentagePerSecond = 100.0 / TimeUnit.SECONDS.toNanos(1)
+  // Since exemptSensor is for all clients and has a constant name, we do not expire exemptSensor and only
+  // create once.
+  val DefaultInactiveExemptSensorExpirationTimeSeconds = Long.MaxValue

Review Comment:
   I prefer creating exemptSensor at initialization because getOrCreateSensor acquires lock and if creating exemptSensor every time it is used may have negative performance impact. And since it is just one metric, it should be good to create it during initialization and have it never expired (not a big issue for space usage).



##########
core/src/main/scala/kafka/server/ClientQuotaManager.scala:
##########
@@ -433,10 +433,10 @@ class ClientQuotaManager(private val config: ClientQuotaManagerConfig,
       .quota(new Quota(quotaLimit, true))
   }
 
-  protected def getOrCreateSensor(sensorName: String, metricName: MetricName): Sensor = {
+  protected def getOrCreateSensor(sensorName: String, expirationTime: Long, metricName: MetricName): Sensor = {

Review Comment:
   update as suggested



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org