chia7712 commented on code in PR #16426: URL: https://github.com/apache/kafka/pull/16426#discussion_r1668488960
########## core/src/main/scala/kafka/server/KafkaRequestHandler.scala: ########## @@ -284,26 +284,35 @@ class BrokerTopicMetrics(name: Option[String], remoteStorageEnabled: Boolean = f } Review Comment: @stanislavkozlovski @ijuma Sorry for pinging you on this PR, but I have a question about this sync of `MeterWrapper` (related to #7198) This PR tries to align the behavior of new wrapper with `MeterWrapper`, and so I noticed the lock used by `MeterWrapper`. `MetricsRegistry` is a global variable in our code base, and both methods `MetricsRegistry#newXXX` and `MetricsRegistry#removeMetric` should be thread-safe. That means `newMeter` does NOT return `null` (the impl leverage `ConcurrentHashMap` to avoid race condition) Hence, my question is "why we add extra lock to MeterWrapper"? Please correct me if I misunderstand anything. Thanks! -- 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]
