jsancio commented on code in PR #20724:
URL: https://github.com/apache/kafka/pull/20724#discussion_r2466592203
##########
metadata/src/main/java/org/apache/kafka/image/loader/MetadataLoader.java:
##########
@@ -120,11 +120,12 @@ public MetadataLoader build() {
throw new RuntimeException("You must set the high water mark
accessor.");
}
if (metrics == null) {
- metrics = new MetadataLoaderMetrics(Optional.empty(),
- __ -> { },
- __ -> { },
- new AtomicReference<>(MetadataProvenance.EMPTY),
- time);
+ metrics = new MetadataLoaderMetrics(
+ Optional.empty(),
+ time,
+ __ -> { },
+ __ -> { },
+ new AtomicReference<>(MetadataProvenance.EMPTY));
Review Comment:
Extra 4 spaces of indentation.
##########
metadata/src/main/java/org/apache/kafka/image/loader/metrics/MetadataLoaderMetrics.java:
##########
@@ -96,6 +106,20 @@ public Long value() {
return handleLoadSnapshotCount();
}
}));
+ registry.ifPresent(r -> r.newGauge(AVERAGE_IDLE_RATIO, new
Gauge<Double>() {
+ @Override
+ public Double value() {
+ synchronized (avgIdleTimeRatio) {
+ return avgIdleTimeRatio.measure();
+ }
+ }
+ }));
+ }
+
+ public void updateIdleTime(long idleDurationMs) {
+ synchronized (avgIdleTimeRatio) {
+ avgIdleTimeRatio.record((double) idleDurationMs,
time.milliseconds());
+ }
}
Review Comment:
I didn't notice this in the previous PR but the signature should be `void
updateIdleTime(long idleDurationMs, long currentTimeMs)`. Otherwise the current
time use to compute the idle time is different from the current time used by
`TimeRatio`.
Can you fix this in this PR? This mean that KafkaEventQueue should take a
`BiConsumer<Long, Long>`
--
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]
