junrao commented on code in PR #15889:
URL: https://github.com/apache/kafka/pull/15889#discussion_r1610602287
##########
clients/src/main/java/org/apache/kafka/common/metrics/stats/SampledStat.java:
##########
@@ -106,44 +109,51 @@ public String toString() {
public abstract double combine(List<Sample> samples, MetricConfig config,
long now);
- /* Timeout any windows that have expired in the absence of any events */
+ // purge any samples that lack observed events within the monitored window
protected void purgeObsoleteSamples(MetricConfig config, long now) {
long expireAge = config.samples() * config.timeWindowMs();
for (Sample sample : samples) {
- if (now - sample.lastWindowMs >= expireAge)
+ // samples overlapping the monitored window are kept,
+ // even if they started before it
+ if (now - sample.lastEventMs >= expireAge) {
sample.reset(now);
+ }
}
}
protected static class Sample {
public double initialValue;
public long eventCount;
- public long lastWindowMs;
+ public long startTimeMs;
+ public long lastEventMs;
public double value;
public Sample(double initialValue, long now) {
this.initialValue = initialValue;
this.eventCount = 0;
- this.lastWindowMs = now;
+ this.startTimeMs = now;
+ this.lastEventMs = now;
this.value = initialValue;
}
public void reset(long now) {
this.eventCount = 0;
- this.lastWindowMs = now;
+ this.startTimeMs = now;
+ this.lastEventMs = now;
this.value = initialValue;
}
public boolean isComplete(long timeMs, MetricConfig config) {
- return timeMs - lastWindowMs >= config.timeWindowMs() ||
eventCount >= config.eventWindow();
+ return timeMs - startTimeMs >= config.timeWindowMs() || eventCount
>= config.eventWindow();
Review Comment:
> in this case, totalElapsedTimeMs will be equal to (config.samples() - 1) *
timeWindowMs, and then we will get smaller measure due to the larger denominator
@chia7712: Could you explain your point a bit more? In the current design,
we always ensure at least `(config.samples() - 1) * timeWindowMs` in
`totalElapsedTimeMs`. This is to avoid the potential extreme large measured
value when the accumulated sample window is small (e.g. when the metric is
first recorded).
--
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
