Fix to only get an interval histogram once per recorder

Netflix · May 23, 2015 · 2639ce4 · 2639ce4
1 parent 65ab233
commit 2639ce4
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 2 deletions.
diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/util/HystrixRollingPercentile.java b/hystrix-core/src/main/java/com/netflix/hystrix/util/HystrixRollingPercentile.java
@@ -23,6 +23,7 @@
 import java.util.concurrent.atomic.AtomicReferenceArray;
 import java.util.concurrent.locks.ReentrantLock;
 
+import org.HdrHistogram.Histogram;
 import org.HdrHistogram.IntCountsHistogram;
 import org.HdrHistogram.Recorder;
 import org.slf4j.Logger;
@@ -284,6 +285,7 @@ public void reset() {
 
     /*package-private*/ static class PercentileBucketData {
         final Recorder recorder;
+        final AtomicReference<Histogram> stableHistogram = new AtomicReference<Histogram>(null);
 
         public PercentileBucketData() {
             this.recorder = new Recorder(4);
@@ -300,7 +302,7 @@ public void addValue(int... latency) {
      * @NotThreadSafe
      */
     /* package for testing */ static class PercentileSnapshot {
-        private final IntCountsHistogram aggregateHistogram;
+        /* package-private*/ final IntCountsHistogram aggregateHistogram;
         private final long count;
         private final int mean;
         private final int p0;
@@ -328,7 +330,10 @@ public void addValue(int... latency) {
         /* package for testing */ PercentileSnapshot(Bucket[] buckets) {
             aggregateHistogram = new IntCountsHistogram(4);
             for (Bucket bucket: buckets) {
-                aggregateHistogram.add(bucket.bucketData.recorder.getIntervalHistogram());
+                PercentileBucketData bucketData = bucket.bucketData;
+                //if stable snapshot not already generated, generate it now
+                bucketData.stableHistogram.compareAndSet(null, bucketData.recorder.getIntervalHistogram());
+                aggregateHistogram.add(bucket.bucketData.stableHistogram.get());
             }
 
             count = aggregateHistogram.getTotalCount();

diff --git a/hystrix-core/src/test/java/com/netflix/hystrix/util/HystrixRollingPercentileTest.java b/hystrix-core/src/test/java/com/netflix/hystrix/util/HystrixRollingPercentileTest.java
@@ -162,9 +162,16 @@ public void testSampleDataOverTime1() {
         MockedTime time = new MockedTime();
         HystrixRollingPercentile p = new HystrixRollingPercentile(time, timeInMilliseconds, numberOfBuckets, enabled);
         int previousTime = 0;
+
+        int maxSoFar = -1;
+
         for (int i = 0; i < SampleDataHolder1.data.length; i++) {
             int timeInMillisecondsSinceStart = SampleDataHolder1.data[i][0];
             int latency = SampleDataHolder1.data[i][1];
+            if (latency > maxSoFar) {
+                System.out.println("New MAX latency : " + latency);
+                maxSoFar = latency;
+            }
             time.increment(timeInMillisecondsSinceStart - previousTime);
             previousTime = timeInMillisecondsSinceStart;
             p.addValue(latency);
@@ -181,6 +188,8 @@ public void testSampleDataOverTime1() {
         System.out.println("Median: " + p.getPercentile(50));
         System.out.println("Median: " + p.getPercentile(50));
 
+        System.out.println("MAX : " + p.currentPercentileSnapshot.aggregateHistogram.getMaxValue());
+
         /*
          * In a loop as a use case was found where very different values were calculated in subsequent requests.
          */
@@ -216,6 +225,8 @@ public void testSampleDataOverTime2() {
         System.out.println("99.5th: " + p.getPercentile(99.5));
         System.out.println("99.99: " + p.getPercentile(99.99));
 
+        System.out.println("MAX : " + p.currentPercentileSnapshot.aggregateHistogram.getMaxValue());
+
         if (p.getPercentile(50) > 90 || p.getPercentile(50) < 50) {
             fail("We expect around 60-70 but got: " + p.getPercentile(50));
         }