From d42ea9f1ea5c06efdf7f14f2bad19088657549d8 Mon Sep 17 00:00:00 2001 From: Andrew Purtell Date: Fri, 18 May 2018 12:49:06 -0700 Subject: [PATCH] W-5007773 Histogram metrics should reset min and max (HBASE-20603) Reason: Operational improvement Test Plan: Unit tests --- .../hadoop/hbase/metrics/impl/HistogramImpl.java | 26 +++++++++++++--------- .../hbase/metrics/impl/TestHistogramImpl.java | 24 ++++++++++++++++++++ 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/HistogramImpl.java b/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/HistogramImpl.java index 17a179dafa..237b38ce1b 100644 --- a/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/HistogramImpl.java +++ b/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/HistogramImpl.java @@ -31,11 +31,11 @@ */ @InterfaceAudience.Private public class HistogramImpl implements Histogram { - // Double buffer the two FastLongHistograms. - // As they are reset they learn how the buckets should be spaced - // So keep two around and use them - protected final FastLongHistogram histogram; + protected FastLongHistogram histogram; private final CounterImpl counter; + private int numBins; + private long min; + private long maxExpected; public HistogramImpl() { this(Integer.MAX_VALUE << 2); @@ -47,14 +47,12 @@ public HistogramImpl(long maxExpected) { public HistogramImpl(int numBins, long min, long maxExpected) { this.counter = new CounterImpl(); + this.numBins = numBins; + this.min = min; + this.maxExpected = maxExpected; this.histogram = new FastLongHistogram(numBins, min, maxExpected); } - protected HistogramImpl(CounterImpl counter, FastLongHistogram histogram) { - this.counter = counter; - this.histogram = histogram; - } - @Override public void update(int value) { counter.increment(); @@ -72,12 +70,20 @@ public long getCount() { return counter.getCount(); } + public long getMin() { + return this.histogram.getMin(); + } + public long getMax() { return this.histogram.getMax(); } @Override public Snapshot snapshot() { - return histogram.snapshotAndReset(); + Snapshot snapshot = histogram.snapshotAndReset(); + // Our desired semantics require all histogram state except for the count to + // reset after a snapshot, regardless of the underlying implementation. + this.histogram = new FastLongHistogram(numBins, min, maxExpected); + return snapshot; } } diff --git a/hbase-metrics/src/test/java/org/apache/hadoop/hbase/metrics/impl/TestHistogramImpl.java b/hbase-metrics/src/test/java/org/apache/hadoop/hbase/metrics/impl/TestHistogramImpl.java index 5d3b1faa21..51411e62cb 100644 --- a/hbase-metrics/src/test/java/org/apache/hadoop/hbase/metrics/impl/TestHistogramImpl.java +++ b/hbase-metrics/src/test/java/org/apache/hadoop/hbase/metrics/impl/TestHistogramImpl.java @@ -99,5 +99,29 @@ public void testSnapshot() { assertEquals(198, snapshot.get98thPercentile()); assertEquals(199, snapshot.get99thPercentile()); assertEquals(199, snapshot.get999thPercentile()); + + // Ensure that points or state from before an earlier snapshot is not retained + // after snapshot; only the count should not be reset + + for (int i = 0; i < 100; i++) { + histogram.update(i); + } + + snapshot = histogram.snapshot(); + + assertEquals(300, histogram.getCount()); + + assertEquals(100, snapshot.getCount()); + assertEquals(50, snapshot.getMedian()); + assertEquals(49, snapshot.getMean()); + assertEquals(0, snapshot.getMin()); + assertEquals(99, snapshot.getMax()); + assertEquals(25, snapshot.get25thPercentile()); + assertEquals(75, snapshot.get75thPercentile()); + assertEquals(90, snapshot.get90thPercentile()); + assertEquals(95, snapshot.get95thPercentile()); + assertEquals(98, snapshot.get98thPercentile()); + assertEquals(99, snapshot.get99thPercentile()); + assertEquals(99, snapshot.get999thPercentile()); } }