add test case to check merge assumptions

Netflix · Oct 14, 2015 · 75da06c · 75da06c
1 parent da5436c
commit 75da06c
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 4 deletions.
diff --git a/spectator-reg-tdigest/src/main/java/com/netflix/spectator/tdigest/StepDigest.java b/spectator-reg-tdigest/src/main/java/com/netflix/spectator/tdigest/StepDigest.java
@@ -70,8 +70,8 @@ class StepDigest {
     this.step = step;
     lastInitPos = new AtomicLong(0L);
     lastPollTime = new AtomicLong(0L);
-    previous = new AtomicReference<TDigest>(TDigest.createDigest(init));
-    current = new AtomicReference<TDigest>(TDigest.createDigest(init));
+    previous = new AtomicReference<>(TDigest.createDigest(init));
+    current = new AtomicReference<>(TDigest.createDigest(init));
   }
 
   private void roll(long now) {

diff --git a/spectator-reg-tdigest/src/main/resources/reference.conf b/spectator-reg-tdigest/src/main/resources/reference.conf
@@ -1,8 +1,9 @@
 
 spectator.tdigest {
 
-  // Compression factor to use constructing digests
-  compression-factor = 100.0
+  // Compression factor to use constructing digests. Set to 10.0 by default as a usually
+  // reasonable trade-off between performance, memory overhead, and accuracy.
+  compression-factor = 10.0
 
   kinesis {
     // Kinesis endpoint to use. Assumes EC2_REGION environment variable will be set

diff --git a/spectator-reg-tdigest/src/test/java/com/netflix/spectator/tdigest/StepDigestTest.java b/spectator-reg-tdigest/src/test/java/com/netflix/spectator/tdigest/StepDigestTest.java
@@ -19,12 +19,18 @@
 import com.netflix.spectator.api.Id;
 import com.netflix.spectator.api.ManualClock;
 import com.netflix.spectator.api.Registry;
+import com.tdunning.math.stats.TDigest;
+import com.tdunning.math.stats.TreeDigest;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Random;
+
 @RunWith(JUnit4.class)
 public class StepDigestTest {
 
@@ -60,4 +66,42 @@ public void addTwoValues() throws Exception {
     clock.setWallTime(10);
     Assert.assertEquals(50.5, digest.poll().quantile(0.5), 0.2);
   }
+
+  @Test
+  public void merge() throws Exception {
+    Random r = new Random();
+    double[] values = new double[1000];
+    for (int i = 0; i < 1000; ++i) {
+      values[i] = r.nextDouble();
+    }
+
+    TDigest d1 = TreeDigest.createDigest(10.0);
+    for (int i = 0; i < 1000; ++i) {
+      d1.add(values[i]);
+    }
+    double v = d1.quantile(0.99);
+    Assert.assertEquals(v, d1.quantile(0.99), 1e-12);
+
+    TDigest d2 = TreeDigest.createDigest(10.0);
+    TDigest d3 = TreeDigest.createDigest(10.0);
+    for (int i = 0; i < 1000; ++i) {
+      if (i % 2 == 0)
+        d2.add(values[i]);
+      else
+        d3.add(values[i]);
+    }
+    List<TDigest> vs = new ArrayList<>();
+    vs.add(d2);
+    vs.add(d3);
+
+    TDigest m1 = TreeDigest.merge(10.0, vs, r);
+    double v2 = m1.quantile(0.99);
+    Assert.assertEquals(v2, m1.quantile(0.99), 1e-12);
+
+    for (int i = 0; i < 10; ++i) {
+      Assert.assertEquals(v2, TreeDigest.merge(10.0, vs, r).quantile(0.99), v2 / 100);
+    }
+
+    Assert.assertEquals(v, v2, Math.abs(v) / 10);
+  }
 }