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:
us...@infra.apache.org
