[GitHub] [kafka] dajac commented on a change in pull request #9417: MINOR: Fix flaky testQuotaMetric

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

dajac commented on a change in pull request #9417:
URL: https://github.com/apache/kafka/pull/9417#discussion_r506363762



##########
File path: core/src/main/scala/kafka/server/ClientQuotaManager.scala
##########
@@ -360,8 +360,14 @@ class ClientQuotaManager(private val config: ClientQuotaManagerConfig,
    * Note: this method is expensive, it is meant to be used by tests only
    */
   def quota(userPrincipal: KafkaPrincipal, clientId: String): Quota = {
-    val metricTags = quotaCallback.quotaMetricTags(clientQuotaType, userPrincipal, clientId)
-    Quota.upperBound(quotaLimit(metricTags))
+    // acquire read lock to ensure that both quota limit and metric config are updated atomically
+    lock.readLock().lock()
+    try {
+      val metricTags = quotaCallback.quotaMetricTags(clientQuotaType, userPrincipal, clientId)
+      Quota.upperBound(quotaLimit(metricTags))
+    } finally {
+      lock.readLock().unlock()
+    }

Review comment:
       While this works, I wonder if this is the right way to overcome the issue. My concern is that there are other read paths which remain unprotected so we are not consistent. I would rather prefer to update the test as you 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.

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