dajac commented on code in PR #15835:
URL: https://github.com/apache/kafka/pull/15835#discussion_r1589068864
##########
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:
Yeah, I agree. We should perhaps revert my commit that removed the timeout
now that we have a use case for it.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorRuntimeMetrics.java:
##########
@@ -208,8 +209,8 @@ public void recordEventQueueTime(long durationMs) { }
public void recordEventQueueProcessingTime(long durationMs) { }
@Override
- public void recordThreadIdleRatio(double ratio) {
- threadIdleRatioSensor.record(ratio);
+ public synchronized void recordThreadIdleTime(long idleTimeMs, long
currentTimeMs) {
Review Comment:
Why do we need to synchronize here?
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorRuntimeMetrics.java:
##########
@@ -208,8 +209,8 @@ public void recordEventQueueTime(long durationMs) { }
public void recordEventQueueProcessingTime(long durationMs) { }
@Override
- public void recordThreadIdleRatio(double ratio) {
- threadIdleRatioSensor.record(ratio);
+ public synchronized void recordThreadIdleTime(long idleTimeMs, long
currentTimeMs) {
+ threadIdleTimeRate.record(metrics.config(), idleTimeMs, currentTimeMs);
Review Comment:
I am confused here. Shouldn't we call `record` on the sensor?
--
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
