gaurav-narula commented on code in PR #16704:
URL: https://github.com/apache/kafka/pull/16704#discussion_r1705699187
##########
core/src/main/scala/kafka/controller/KafkaController.scala:
##########
@@ -111,7 +111,11 @@ class KafkaController(val config: KafkaConfig,
tokenManager: DelegationTokenManager,
brokerFeatures: BrokerFeatures,
featureCache: ZkFinalizedFeatureCache,
- threadNamePrefix: Option[String] = None)
+ threadNamePrefix: Option[String] = None,
+ // KafkaYammerMetrics uses a static singleton that is
shared across the JVM
+ // Therefore, these are populated in tests with brokerId
to disambiguate between metrics for
+ // different nodes
Review Comment:
It's true that there can be only a single active controller, but each node
within the test harness registers the controller metrics during
[KafkaServer::startup](https://github.com/apache/kafka/blob/3ddd8d0a0ec02eab8d9083d341ece14961fc0d1c/core/src/main/scala/kafka/server/KafkaServer.scala#L417).
The same metrics are also removed on
[KafkaServer::shutdown](https://github.com/apache/kafka/blob/3ddd8d0a0ec02eab8d9083d341ece14961fc0d1c/core/src/main/scala/kafka/server/KafkaServer.scala#L1041).
Since the metrics are registered using `KafkaYammerMetrics` which uses a
static variable behind the scenes, shutting down a broker during the test leads
to removal of controller metrics. The disambiguation is therefore required so
that "controller-metrics" are removed only for the broker that's being shut
down.
This isn't an issue in real-world scenarios because we don't run multiple
brokers within the same JVM.
--
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
