Repository: kafka Updated Branches: refs/heads/trunk 50eacb7b4 -> a511a4702
KAFKA-5203; Metrics: fix resetting of histogram sample Without the histogram cleanup, the percentiles are calculated incorrectly after purging of one or more samples: event counts go out of sync with counts in histogram buckets, and bucket with lower value gets chosen for the given quantile. This change adds the necessary histogram cleanup. Author: Ivan A. Melnikov <[email protected]> Reviewers: Ismael Juma <[email protected]>, Jun Rao <[email protected]> Closes #3002 from iv-m/kafka-5203-percentiles-fix Project: http://git-wip-us.apache.org/repos/asf/kafka/repo Commit: http://git-wip-us.apache.org/repos/asf/kafka/commit/a511a470 Tree: http://git-wip-us.apache.org/repos/asf/kafka/tree/a511a470 Diff: http://git-wip-us.apache.org/repos/asf/kafka/diff/a511a470 Branch: refs/heads/trunk Commit: a511a47020a1a3f58c7778b5d5aba08d2b4876d6 Parents: 50eacb7 Author: Ivan A. Melnikov <[email protected]> Authored: Mon May 15 10:50:33 2017 -0700 Committer: Jun Rao <[email protected]> Committed: Mon May 15 10:50:33 2017 -0700 ---------------------------------------------------------------------- .../org/apache/kafka/common/metrics/stats/Percentiles.java | 6 ++++++ .../java/org/apache/kafka/common/metrics/MetricsTest.java | 8 ++++++++ 2 files changed, 14 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kafka/blob/a511a470/clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentiles.java ---------------------------------------------------------------------- diff --git a/clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentiles.java b/clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentiles.java index c4c8d39..80236e9 100644 --- a/clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentiles.java +++ b/clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentiles.java @@ -115,6 +115,12 @@ public class Percentiles extends SampledStat implements CompoundStat { super(0.0, now); this.histogram = new Histogram(scheme); } + + @Override + public void reset(long now) { + super.reset(now); + this.histogram.clear(); + } } } http://git-wip-us.apache.org/repos/asf/kafka/blob/a511a470/clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java ---------------------------------------------------------------------- diff --git a/clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java b/clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java index 1ef98a2..dac9570 100644 --- a/clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java +++ b/clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java @@ -424,6 +424,14 @@ public class MetricsTest { assertEquals(0.0, p25.value(), 1.0); assertEquals(0.0, p50.value(), 1.0); assertEquals(0.0, p75.value(), 1.0); + + // record two more windows worth of sequential values + for (int i = 0; i < buckets; i++) + sensor.record(i); + + assertEquals(25, p25.value(), 1.0); + assertEquals(50, p50.value(), 1.0); + assertEquals(75, p75.value(), 1.0); } @Test
