jeffkbkim commented on code in PR #18717:
URL: https://github.com/apache/kafka/pull/18717#discussion_r1935906597


##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetricsShard.java:
##########
@@ -145,17 +133,14 @@ public void incrementNumOffsets() {
     }
 
     /**
-     * Increment the number of consumer groups.
+     * Set the number of consumer groups. The method is the only way to update
+     * the map and is called by the scheduled task that updates the metrics
+     * in {@link org.apache.kafka.coordinator.group.GroupCoordinatorShard}.
      *
-     * @param state the consumer group state.
+     * @param consumerGroupGauges The map counting the number of consumer 
groups in each state.

Review Comment:
   nit: can we update the classicGroupGauges parameter similarly to this?



##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetricsShard.java:
##########
@@ -145,17 +133,14 @@ public void incrementNumOffsets() {
     }
 
     /**
-     * Increment the number of consumer groups.
+     * Set the number of consumer groups. The method is the only way to update
+     * the map and is called by the scheduled task that updates the metrics
+     * in {@link org.apache.kafka.coordinator.group.GroupCoordinatorShard}.

Review Comment:
   nit: "This method should be the only way.." and add "..breaking this will 
result in inconsistent behavior" as well as for `setClassicGroupGauges`



-- 
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

Reply via email to