dajac commented on code in PR #15835:
URL: https://github.com/apache/kafka/pull/15835#discussion_r1587342492
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorRuntimeMetrics.java:
##########
@@ -120,19 +121,14 @@ public GroupCoordinatorRuntimeMetrics(Metrics metrics) {
"The average time it took to load the partitions in the last
30 sec."
), new Avg());
- this.threadIdleRatioSensor = metrics.sensor("ThreadIdleRatio");
- this.threadIdleRatioSensor.add(
- metrics.metricName(
- "thread-idle-ratio-min",
+ this.threadIdleSensor = metrics.sensor("ThreadIdleRatio");
+ this.threadIdleSensor.add(new Meter(TimeUnit.MILLISECONDS,
+ metrics.metricName("thread-idle-ratio",
METRICS_GROUP,
- "The minimum thread idle ratio over the last 30 seconds."
- ), new Min());
- this.threadIdleRatioSensor.add(
- metrics.metricName(
- "thread-idle-ratio-avg",
+ "The fraction of time the threads spent waiting for an
event."),
+ metrics.metricName("thread-idle-time-ms-total",
Review Comment:
I am not sure to understand the value brought by this one. Does it bring any
value?
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorRuntimeMetrics.java:
##########
@@ -120,19 +121,14 @@ public GroupCoordinatorRuntimeMetrics(Metrics metrics) {
"The average time it took to load the partitions in the last
30 sec."
), new Avg());
- this.threadIdleRatioSensor = metrics.sensor("ThreadIdleRatio");
- this.threadIdleRatioSensor.add(
- metrics.metricName(
- "thread-idle-ratio-min",
+ this.threadIdleSensor = metrics.sensor("ThreadIdleRatio");
+ this.threadIdleSensor.add(new Meter(TimeUnit.MILLISECONDS,
+ metrics.metricName("thread-idle-ratio",
Review Comment:
If I understand it correctly, we can an average out of this one. Should we
keep the `-avg` suffix here?
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/MultiThreadedEventProcessor.java:
##########
@@ -126,9 +126,16 @@ private class EventProcessorThread extends Thread {
private void handleEvents() {
while (!shuttingDown) {
- recordPollStartTime(time.milliseconds());
+ // We use a single meter for aggregate idle percentage for the
thread pool.
+ // Since meter is calculated as total_recorded_value /
time_window and
+ // time_window is independent of the number of threads, each
recorded idle
+ // time should be discounted by # threads.
+
+ long idleStartTimeMs = time.milliseconds();
CoordinatorEvent event = accumulator.take();
Review Comment:
This call could block for long period of time. Does it have an impact?
--
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
