From f6c3bf3c4d84606795a8b9995ea9fb741d289533 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Fri, 30 Nov 2018 10:27:39 -0500 Subject: [PATCH 1/2] Unit test for very large percentile aggs --- .../metrics/TDigestStateTests.java | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestStateTests.java diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestStateTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestStateTests.java new file mode 100644 index 0000000000000..ef505b109701b --- /dev/null +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestStateTests.java @@ -0,0 +1,35 @@ +package org.elasticsearch.search.aggregations.metrics; + +import org.elasticsearch.test.ESTestCase; +import org.junit.Test; + +import java.util.Arrays; + +public class TDigestStateTests extends ESTestCase { + @Test + public void testMoreThan4BValues() { + // Regression test for #19528 + // See https://github.com/tdunning/t-digest/pull/70/files#diff-4487072cee29b939694825647928f742R439 + TDigestState digest = new TDigestState(100); + for (int i = 0; i < 1000; ++i) { + digest.add(randomDouble()); + } + final int count = 1 << 29; + for (int i = 0; i < 10; ++i) { + digest.add(randomDouble(), count); + } + assertEquals(1000 + 10L * (1 << 29), digest.size()); + assertTrue(digest.size() > 2 * Integer.MAX_VALUE); + final double[] quantiles = new double[] { 0, 0.1, 0.5, 0.9, 1, randomDouble()}; + Arrays.sort(quantiles); + double prev = Double.NEGATIVE_INFINITY; + for (double q : quantiles) { + final double v = digest.quantile(q); + System.out.println("q=" + q + ", v=" + v); + assertTrue(v >= prev); + assertTrue("Unexpectedly low value: " + v, v >= 0.0); + assertTrue("Unexpectedly high value: " + v, v <= 1.0); + prev = v; + } + } +} From e4d608b76f5299ed417ad8f6005a34d6aaa276a5 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Mon, 3 Dec 2018 14:22:53 -0500 Subject: [PATCH 2/2] Response to PR feedback --- .../metrics/TDigestStateTests.java | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestStateTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestStateTests.java index ef505b109701b..5862577721088 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestStateTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestStateTests.java @@ -1,12 +1,30 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + package org.elasticsearch.search.aggregations.metrics; import org.elasticsearch.test.ESTestCase; -import org.junit.Test; import java.util.Arrays; public class TDigestStateTests extends ESTestCase { - @Test + public void testMoreThan4BValues() { // Regression test for #19528 // See https://github.com/tdunning/t-digest/pull/70/files#diff-4487072cee29b939694825647928f742R439 @@ -25,7 +43,7 @@ public void testMoreThan4BValues() { double prev = Double.NEGATIVE_INFINITY; for (double q : quantiles) { final double v = digest.quantile(q); - System.out.println("q=" + q + ", v=" + v); + logger.trace("q=" + q + ", v=" + v); assertTrue(v >= prev); assertTrue("Unexpectedly low value: " + v, v >= 0.0); assertTrue("Unexpectedly high value: " + v, v <= 1.0);