jeffkbkim commented on code in PR #18717:
URL: https://github.com/apache/kafka/pull/18717#discussion_r1934491508
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorShard.java:
##########
@@ -719,7 +719,7 @@ private void scheduleClassicGroupSizeCounter() {
* Cancels the group size counter for the classic groups.
*/
private void cancelClassicGroupSizeCounter() {
Review Comment:
nit: should we also remove classic here?
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetricsShard.java:
##########
@@ -145,17 +133,12 @@ public void incrementNumOffsets() {
}
/**
- * Increment the number of consumer groups.
+ * Set the number of consumer groups.
Review Comment:
Should we also mention that this should be the only way to update this map?
also for setClassicGroupGauges
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetricsShard.java:
##########
@@ -71,7 +71,7 @@ public TimelineGaugeCounter(TimelineLong timelineLong,
AtomicLong atomicLong) {
/**
* Consumer group size gauge counters keyed by the metric name.
*/
- private final Map<ConsumerGroupState, TimelineGaugeCounter>
consumerGroupGauges;
+ private volatile Map<ConsumerGroupState, Long> consumerGroupGauges;
Review Comment:
confirming the concurrency pattern here:
- we have only one reader (metrics collector) and writer (runtime) thread
for a shard at any given time.
- we are atomically updating the map to a new map
does this sound right?
--
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]
