diff --git a/benchmarks/src/main/java/org/elasticsearch/benchmark/exponentialhistogram/ExponentialHistogramMergeBench.java b/benchmarks/src/main/java/org/elasticsearch/benchmark/exponentialhistogram/ExponentialHistogramMergeBench.java index 386d8a9c86943..259cf6c410112 100644 --- a/benchmarks/src/main/java/org/elasticsearch/benchmark/exponentialhistogram/ExponentialHistogramMergeBench.java +++ b/benchmarks/src/main/java/org/elasticsearch/benchmark/exponentialhistogram/ExponentialHistogramMergeBench.java @@ -130,7 +130,7 @@ private ExponentialHistogram asCompressedHistogram(ExponentialHistogram histogra CompressedExponentialHistogram.writeHistogramBytes(histoBytes, histogram.scale(), negativeBuckets, positiveBuckets); CompressedExponentialHistogram result = new CompressedExponentialHistogram(); BytesRef data = histoBytes.bytes().toBytesRef(); - result.reset(histogram.zeroBucket().zeroThreshold(), totalCount, histogram.sum(), histogram.min(), data); + result.reset(histogram.zeroBucket().zeroThreshold(), totalCount, histogram.sum(), histogram.min(), histogram.max(), data); return result; } catch (IOException e) { throw new RuntimeException(e); diff --git a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/EmptyExponentialHistogram.java b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/EmptyExponentialHistogram.java index a97e3498c2e5f..abc37b4c8246f 100644 --- a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/EmptyExponentialHistogram.java +++ b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/EmptyExponentialHistogram.java @@ -87,6 +87,11 @@ public double min() { return Double.NaN; } + @Override + public double max() { + return Double.NaN; + } + @Override public long ramBytesUsed() { return 0; diff --git a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogram.java b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogram.java index 8f0e6ea12464d..a170058101b8b 100644 --- a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogram.java +++ b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogram.java @@ -47,7 +47,6 @@ */ public interface ExponentialHistogram extends Accountable { - // TODO(b/128622): support min/max storage and merging. // TODO(b/128622): Add special positive and negative infinity buckets // to allow representation of explicit bucket histograms with open boundaries. @@ -117,6 +116,13 @@ public interface ExponentialHistogram extends Accountable { */ double min(); + /** + * Returns maximum of all values represented by this histogram. + * + * @return the maximum, NaN for empty histograms + */ + double max(); + /** * Represents a bucket range of an {@link ExponentialHistogram}, either the positive or the negative range. */ @@ -154,6 +160,7 @@ static boolean equals(ExponentialHistogram a, ExponentialHistogram b) { return a.scale() == b.scale() && a.sum() == b.sum() && equalsIncludingNaN(a.min(), b.min()) + && equalsIncludingNaN(a.max(), b.max()) && a.zeroBucket().equals(b.zeroBucket()) && bucketIteratorsEqual(a.negativeBuckets().iterator(), b.negativeBuckets().iterator()) && bucketIteratorsEqual(a.positiveBuckets().iterator(), b.positiveBuckets().iterator()); @@ -187,6 +194,7 @@ static int hashCode(ExponentialHistogram histogram) { hash = 31 * hash + Double.hashCode(histogram.sum()); hash = 31 * hash + Long.hashCode(histogram.valueCount()); hash = 31 * hash + Double.hashCode(histogram.min()); + hash = 31 * hash + Double.hashCode(histogram.max()); hash = 31 * hash + histogram.zeroBucket().hashCode(); // we intentionally don't include the hash of the buckets here, because that is likely expensive to compute // instead, we assume that the value count and sum are a good enough approximation in most cases to minimize collisions diff --git a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramGenerator.java b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramGenerator.java index 4d9716888cd95..a46cd14f22a4b 100644 --- a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramGenerator.java +++ b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramGenerator.java @@ -126,6 +126,7 @@ private void mergeValuesToHistogram() { Aggregates aggregates = rawValuesAggregates(); valueBuffer.setSum(aggregates.sum()); valueBuffer.setMin(aggregates.min()); + valueBuffer.setMax(aggregates.max()); int scale = valueBuffer.scale(); // Buckets must be provided with their indices in ascending order. @@ -166,15 +167,17 @@ private void mergeValuesToHistogram() { private Aggregates rawValuesAggregates() { if (valueCount == 0) { - return new Aggregates(0, Double.NaN); + return new Aggregates(0, Double.NaN, Double.NaN); } double sum = 0; double min = Double.MAX_VALUE; + double max = -Double.MAX_VALUE; for (int i = 0; i < valueCount; i++) { sum += rawValueBuffer[i]; min = Math.min(min, rawValueBuffer[i]); + max = Math.max(max, rawValueBuffer[i]); } - return new Aggregates(sum, min); + return new Aggregates(sum, min, max); } private static long estimateBaseSize(int numBuckets) { @@ -198,5 +201,5 @@ public void close() { } } - private record Aggregates(double sum, double min) {} + private record Aggregates(double sum, double min, double max) {} } diff --git a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramMerger.java b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramMerger.java index 8c7dd99dd909f..78e91b9e0e69f 100644 --- a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramMerger.java +++ b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramMerger.java @@ -27,6 +27,7 @@ import org.elasticsearch.core.Releasable; import java.util.OptionalLong; +import java.util.function.DoubleBinaryOperator; import static org.elasticsearch.exponentialhistogram.ExponentialScaleUtils.getMaximumScaleIncrease; @@ -151,7 +152,8 @@ public void add(ExponentialHistogram toAdd) { } buffer.setZeroBucket(zeroBucket); buffer.setSum(a.sum() + b.sum()); - buffer.setMin(nanAwareMin(a.min(), b.min())); + buffer.setMin(nanAwareAggregate(a.min(), b.min(), Math::min)); + buffer.setMax(nanAwareAggregate(a.max(), b.max(), Math::max)); // We attempt to bring everything to the scale of A. // This might involve increasing the scale for B, which would increase its indices. // We need to ensure that we do not exceed MAX_INDEX / MIN_INDEX in this case. @@ -231,14 +233,14 @@ private static int putBuckets( return overflowCount; } - private static double nanAwareMin(double a, double b) { + private static double nanAwareAggregate(double a, double b, DoubleBinaryOperator aggregator) { if (Double.isNaN(a)) { return b; } if (Double.isNaN(b)) { return a; } - return Math.min(a, b); + return aggregator.applyAsDouble(a, b); } } diff --git a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramUtils.java b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramUtils.java index c499cb11be7d7..84938ac1d4ea6 100644 --- a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramUtils.java +++ b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramUtils.java @@ -67,7 +67,7 @@ public static double estimateSum(BucketIterator negativeBuckets, BucketIterator * Estimates the minimum value of the histogram based on the populated buckets. * The returned value is guaranteed to be less than or equal to the exact minimum value of the histogram values. * If the histogram is empty, an empty Optional is returned. - * + *

* Note that this method can return +-Infinity if the histogram bucket boundaries are not representable in a double. * * @param zeroBucket the zero bucket of the histogram @@ -102,4 +102,40 @@ public static OptionalDouble estimateMin( } return OptionalDouble.empty(); } + + /** + * Estimates the maximum value of the histogram based on the populated buckets. + * The returned value is guaranteed to be greater than or equal to the exact maximum value of the histogram values. + * If the histogram is empty, an empty Optional is returned. + *

+ * Note that this method can return +-Infinity if the histogram bucket boundaries are not representable in a double. + * + * @param zeroBucket the zero bucket of the histogram + * @param negativeBuckets the negative buckets of the histogram + * @param positiveBuckets the positive buckets of the histogram + * @return the estimated minimum + */ + public static OptionalDouble estimateMax( + ZeroBucket zeroBucket, + ExponentialHistogram.Buckets negativeBuckets, + ExponentialHistogram.Buckets positiveBuckets + ) { + int scale = negativeBuckets.iterator().scale(); + assert scale == positiveBuckets.iterator().scale(); + + OptionalLong positiveMaxIndex = positiveBuckets.maxBucketIndex(); + if (positiveMaxIndex.isPresent()) { + return OptionalDouble.of(ExponentialScaleUtils.getUpperBucketBoundary(positiveMaxIndex.getAsLong(), scale)); + } + + if (zeroBucket.count() > 0) { + return OptionalDouble.of(zeroBucket.zeroThreshold()); + } + + BucketIterator negativeBucketsIt = negativeBuckets.iterator(); + if (negativeBucketsIt.hasNext()) { + return OptionalDouble.of(-ExponentialScaleUtils.getLowerBucketBoundary(negativeBucketsIt.peekIndex(), scale)); + } + return OptionalDouble.empty(); + } } diff --git a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramXContent.java b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramXContent.java index d6f840851a282..1f6b9c266a826 100644 --- a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramXContent.java +++ b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramXContent.java @@ -33,6 +33,7 @@ public class ExponentialHistogramXContent { public static final String SCALE_FIELD = "scale"; public static final String SUM_FIELD = "sum"; public static final String MIN_FIELD = "min"; + public static final String MAX_FIELD = "max"; public static final String ZERO_FIELD = "zero"; public static final String ZERO_COUNT_FIELD = "count"; public static final String ZERO_THRESHOLD_FIELD = "threshold"; @@ -55,6 +56,9 @@ public static void serialize(XContentBuilder builder, ExponentialHistogram histo if (Double.isNaN(histogram.min()) == false) { builder.field(MIN_FIELD, histogram.min()); } + if (Double.isNaN(histogram.max()) == false) { + builder.field(MAX_FIELD, histogram.max()); + } double zeroThreshold = histogram.zeroBucket().zeroThreshold(); long zeroCount = histogram.zeroBucket().count(); diff --git a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/FixedCapacityExponentialHistogram.java b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/FixedCapacityExponentialHistogram.java index 5db708302fc56..f8412f7f0237b 100644 --- a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/FixedCapacityExponentialHistogram.java +++ b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/FixedCapacityExponentialHistogram.java @@ -55,6 +55,7 @@ final class FixedCapacityExponentialHistogram extends AbstractExponentialHistogr private double sum; private double min; + private double max; private final ExponentialHistogramCircuitBreaker circuitBreaker; private boolean closed = false; @@ -83,6 +84,7 @@ private FixedCapacityExponentialHistogram(int bucketCapacity, ExponentialHistogr void reset() { sum = 0; min = Double.NaN; + max = Double.NaN; setZeroBucket(ZeroBucket.minimalEmpty()); resetBuckets(MAX_SCALE); } @@ -133,6 +135,15 @@ void setMin(double min) { this.min = min; } + @Override + public double max() { + return max; + } + + void setMax(double max) { + this.max = max; + } + /** * Attempts to add a bucket to the positive or negative range of this histogram. *
diff --git a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramEqualityTests.java b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramEqualityTests.java index fe2ec27cd65ca..3caba373cd2ab 100644 --- a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramEqualityTests.java +++ b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramEqualityTests.java @@ -41,6 +41,7 @@ public enum Modification { SCALE, SUM, MIN, + MAX, ZERO_THRESHOLD, ZERO_COUNT, POSITIVE_BUCKETS, @@ -102,6 +103,11 @@ private ExponentialHistogram copyWithModification(ExponentialHistogram toCopy, M } else { copy.setMin(toCopy.min()); } + if (modification == Modification.MAX) { + copy.setMax(randomDouble()); + } else { + copy.setMax(toCopy.max()); + } long zeroCount = toCopy.zeroBucket().count(); double zeroThreshold = toCopy.zeroBucket().zeroThreshold(); if (modification == Modification.ZERO_COUNT) { diff --git a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramMergerTests.java b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramMergerTests.java index 9b57bf5129a7a..5e2f8114f5997 100644 --- a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramMergerTests.java +++ b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramMergerTests.java @@ -112,6 +112,7 @@ public void testAggregatesCorrectness() { double[] secondValues = randomDoubles(50).map(val -> val * 2 - 1).toArray(); double correctSum = Arrays.stream(firstValues).sum() + Arrays.stream(secondValues).sum(); double correctMin = DoubleStream.concat(Arrays.stream(firstValues), Arrays.stream(secondValues)).min().getAsDouble(); + double correctMax = DoubleStream.concat(Arrays.stream(firstValues), Arrays.stream(secondValues)).max().getAsDouble(); try ( // Merge some empty histograms too to test that code path ReleasableExponentialHistogram merged = ExponentialHistogram.merge( @@ -125,6 +126,7 @@ public void testAggregatesCorrectness() { ) { assertThat(merged.sum(), closeTo(correctSum, 0.000001)); assertThat(merged.min(), equalTo(correctMin)); + assertThat(merged.max(), equalTo(correctMax)); } } diff --git a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramUtilsTests.java b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramUtilsTests.java index 87dfb3a12ad7b..59909f21333fb 100644 --- a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramUtilsTests.java +++ b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramUtilsTests.java @@ -86,7 +86,7 @@ public void testSumInfinityHandling() { assertThat(sum, equalTo(Double.NEGATIVE_INFINITY)); } - public void testMinimumEstimation() { + public void testMinMaxEstimation() { for (int i = 0; i < 100; i++) { int positiveValueCount = randomBoolean() ? 0 : randomIntBetween(10, 10_000); int negativeValueCount = randomBoolean() ? 0 : randomIntBetween(10, 10_000); @@ -94,6 +94,7 @@ public void testMinimumEstimation() { int bucketCount = randomIntBetween(4, 500); double correctMin = Double.MAX_VALUE; + double correctMax = -Double.MAX_VALUE; double zeroThreshold = Double.MAX_VALUE; double[] values = new double[positiveValueCount + negativeValueCount]; for (int j = 0; j < values.length; j++) { @@ -105,56 +106,84 @@ public void testMinimumEstimation() { } zeroThreshold = Math.min(zeroThreshold, absValue / 2); correctMin = Math.min(correctMin, values[j]); + correctMax = Math.max(correctMax, values[j]); } if (zeroValueCount > 0) { correctMin = Math.min(correctMin, -zeroThreshold); + correctMax = Math.max(correctMax, zeroThreshold); } ExponentialHistogram histo = createAutoReleasedHistogram(bucketCount, values); + ZeroBucket zeroBucket = ZeroBucket.create(zeroThreshold, zeroValueCount); OptionalDouble estimatedMin = ExponentialHistogramUtils.estimateMin( - ZeroBucket.create(zeroThreshold, zeroValueCount), + zeroBucket, + histo.negativeBuckets(), + histo.positiveBuckets() + ); + OptionalDouble estimatedMax = ExponentialHistogramUtils.estimateMax( + zeroBucket, histo.negativeBuckets(), histo.positiveBuckets() ); if (correctMin == Double.MAX_VALUE) { assertThat(estimatedMin.isPresent(), equalTo(false)); + assertThat(estimatedMax.isPresent(), equalTo(false)); } else { assertThat(estimatedMin.isPresent(), equalTo(true)); + assertThat(estimatedMax.isPresent(), equalTo(true)); // If the histogram does not contain mixed sign values, we have a guaranteed relative error bound of 2^(2^-scale) - 1 double histogramBase = Math.pow(2, Math.pow(2, -histo.scale())); - double allowedError = Math.abs(correctMin * (histogramBase - 1)); - assertThat(estimatedMin.getAsDouble(), closeTo(correctMin, allowedError)); + double allowedErrorMin = Math.abs(correctMin * (histogramBase - 1)); + assertThat(estimatedMin.getAsDouble(), closeTo(correctMin, allowedErrorMin)); + double allowedErrorMax = Math.abs(correctMax * (histogramBase - 1)); + assertThat(estimatedMax.getAsDouble(), closeTo(correctMax, allowedErrorMax)); } } } - public void testMinimumEstimationPositiveInfinityHandling() { + public void testMinMaxEstimationPositiveInfinityHandling() { FixedCapacityExponentialHistogram histo = createAutoReleasedHistogram(100); histo.resetBuckets(0); histo.tryAddBucket(2000, 1, true); - OptionalDouble estimate = ExponentialHistogramUtils.estimateMin( + OptionalDouble minEstimate = ExponentialHistogramUtils.estimateMin( ZeroBucket.minimalEmpty(), histo.negativeBuckets(), histo.positiveBuckets() ); - assertThat(estimate.isPresent(), equalTo(true)); - assertThat(estimate.getAsDouble(), equalTo(Double.POSITIVE_INFINITY)); + assertThat(minEstimate.isPresent(), equalTo(true)); + assertThat(minEstimate.getAsDouble(), equalTo(Double.POSITIVE_INFINITY)); + + OptionalDouble maxEstimate = ExponentialHistogramUtils.estimateMax( + ZeroBucket.minimalEmpty(), + histo.negativeBuckets(), + histo.positiveBuckets() + ); + assertThat(maxEstimate.isPresent(), equalTo(true)); + assertThat(maxEstimate.getAsDouble(), equalTo(Double.POSITIVE_INFINITY)); } - public void testMinimumEstimationNegativeInfinityHandling() { + public void testMinMaxEstimationNegativeInfinityHandling() { FixedCapacityExponentialHistogram histo = createAutoReleasedHistogram(100); histo.resetBuckets(0); histo.tryAddBucket(2000, 1, false); - OptionalDouble estimate = ExponentialHistogramUtils.estimateMin( + OptionalDouble minEstimate = ExponentialHistogramUtils.estimateMin( ZeroBucket.minimalEmpty(), histo.negativeBuckets(), histo.positiveBuckets() ); - assertThat(estimate.isPresent(), equalTo(true)); - assertThat(estimate.getAsDouble(), equalTo(Double.NEGATIVE_INFINITY)); + assertThat(minEstimate.isPresent(), equalTo(true)); + assertThat(minEstimate.getAsDouble(), equalTo(Double.NEGATIVE_INFINITY)); + + OptionalDouble maxEstimate = ExponentialHistogramUtils.estimateMax( + ZeroBucket.minimalEmpty(), + histo.negativeBuckets(), + histo.positiveBuckets() + ); + assertThat(maxEstimate.isPresent(), equalTo(true)); + assertThat(maxEstimate.getAsDouble(), equalTo(Double.NEGATIVE_INFINITY)); } public void testMinimumEstimationSanitizedNegativeZero() { diff --git a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramXContentTests.java b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramXContentTests.java index c1040566d0238..7ef623ac395d2 100644 --- a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramXContentTests.java +++ b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramXContentTests.java @@ -42,6 +42,7 @@ public void testFullHistogram() { histo.resetBuckets(7); histo.setSum(1234.56); histo.setMin(-321.123); + histo.setMax(123.123); histo.tryAddBucket(-10, 15, false); histo.tryAddBucket(10, 5, false); histo.tryAddBucket(-11, 10, true); @@ -53,6 +54,7 @@ public void testFullHistogram() { + "\"scale\":7," + "\"sum\":1234.56," + "\"min\":-321.123," + + "\"max\":123.123," + "\"zero\":{\"count\":42,\"threshold\":0.1234}," + "\"positive\":{\"indices\":[-11,11],\"counts\":[10,20]}," + "\"negative\":{\"indices\":[-10,10],\"counts\":[15,5]}" @@ -75,7 +77,8 @@ public void testOnlyZeroCount() { histo.resetBuckets(2); histo.setSum(1.1); histo.setMin(0); - assertThat(toJson(histo), equalTo("{\"scale\":2,\"sum\":1.1,\"min\":0.0,\"zero\":{\"count\":7}}")); + histo.setMax(0); + assertThat(toJson(histo), equalTo("{\"scale\":2,\"sum\":1.1,\"min\":0.0,\"max\":0.0,\"zero\":{\"count\":7}}")); } public void testOnlyPositiveBuckets() { @@ -83,9 +86,13 @@ public void testOnlyPositiveBuckets() { histo.resetBuckets(4); histo.setSum(1.1); histo.setMin(0.5); + histo.setMax(2.5); histo.tryAddBucket(-1, 3, true); histo.tryAddBucket(2, 5, true); - assertThat(toJson(histo), equalTo("{\"scale\":4,\"sum\":1.1,\"min\":0.5,\"positive\":{\"indices\":[-1,2],\"counts\":[3,5]}}")); + assertThat( + toJson(histo), + equalTo("{\"scale\":4,\"sum\":1.1,\"min\":0.5,\"max\":2.5,\"positive\":{\"indices\":[-1,2],\"counts\":[3,5]}}") + ); } public void testOnlyNegativeBuckets() { @@ -93,9 +100,13 @@ public void testOnlyNegativeBuckets() { histo.resetBuckets(5); histo.setSum(1.1); histo.setMin(-0.5); + histo.setMax(-0.25); histo.tryAddBucket(-1, 4, false); histo.tryAddBucket(2, 6, false); - assertThat(toJson(histo), equalTo("{\"scale\":5,\"sum\":1.1,\"min\":-0.5,\"negative\":{\"indices\":[-1,2],\"counts\":[4,6]}}")); + assertThat( + toJson(histo), + equalTo("{\"scale\":5,\"sum\":1.1,\"min\":-0.5,\"max\":-0.25,\"negative\":{\"indices\":[-1,2],\"counts\":[4,6]}}") + ); } private static String toJson(ExponentialHistogram histo) { diff --git a/x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/CompressedExponentialHistogram.java b/x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/CompressedExponentialHistogram.java index a516ca6820971..8db9bb93b29fb 100644 --- a/x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/CompressedExponentialHistogram.java +++ b/x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/CompressedExponentialHistogram.java @@ -36,6 +36,7 @@ public class CompressedExponentialHistogram extends AbstractExponentialHistogram private long valueCount; private double sum; private double min; + private double max; private ZeroBucket lazyZeroBucket; private final EncodedHistogramData encodedData = new EncodedHistogramData(); @@ -71,6 +72,11 @@ public double min() { return min; } + @Override + public double max() { + return max; + } + @Override public ExponentialHistogram.Buckets positiveBuckets() { return positiveBuckets; @@ -88,16 +94,20 @@ public ExponentialHistogram.Buckets negativeBuckets() { * @param valueCount the total number of values the histogram contains, needs to be stored externally * @param sum the total sum of the values the histogram contains, needs to be stored externally * @param min the minimum of the values the histogram contains, needs to be stored externally. - * Must be {@link Double#NaN} if the histogram is empty. + * Must be {@link Double#NaN} if the histogram is empty, non-Nan otherwise. + * @param max the maximum of the values the histogram contains, needs to be stored externally. + * Must be {@link Double#NaN} if the histogram is empty, non-Nan otherwise. * @param encodedHistogramData the encoded histogram bytes which previously where generated via * {@link #writeHistogramBytes(StreamOutput, int, List, List)}. */ - public void reset(double zeroThreshold, long valueCount, double sum, double min, BytesRef encodedHistogramData) throws IOException { + public void reset(double zeroThreshold, long valueCount, double sum, double min, double max, BytesRef encodedHistogramData) + throws IOException { lazyZeroBucket = null; this.zeroThreshold = zeroThreshold; this.valueCount = valueCount; this.sum = sum; this.min = min; + this.max = max; encodedData.decode(encodedHistogramData); negativeBuckets.resetCachedData(); positiveBuckets.resetCachedData(); @@ -105,7 +115,7 @@ public void reset(double zeroThreshold, long valueCount, double sum, double min, /** * Serializes the given histogram, so that exactly the same data can be reconstructed via - * {@link #reset(double, long, double, double, BytesRef)}. + * {@link #reset(double, long, double, double, double, BytesRef)}. * * @param output the output to write the serialized bytes to * @param scale the scale of the histogram diff --git a/x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapper.java b/x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapper.java index 0d5b21b08b2aa..3f8ee1127c181 100644 --- a/x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapper.java +++ b/x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapper.java @@ -76,6 +76,7 @@ * "scale": 12, * "sum": 1234, * "min": -123.456, + * "max": 456.456, * "zero": { * "threshold": 0.123456, * "count": 42 @@ -100,6 +101,7 @@ public class ExponentialHistogramFieldMapper extends FieldMapper { public static final ParseField SCALE_FIELD = new ParseField(ExponentialHistogramXContent.SCALE_FIELD); public static final ParseField SUM_FIELD = new ParseField(ExponentialHistogramXContent.SUM_FIELD); public static final ParseField MIN_FIELD = new ParseField(ExponentialHistogramXContent.MIN_FIELD); + public static final ParseField MAX_FIELD = new ParseField(ExponentialHistogramXContent.MAX_FIELD); public static final ParseField ZERO_FIELD = new ParseField(ExponentialHistogramXContent.ZERO_FIELD); public static final ParseField ZERO_COUNT_FIELD = new ParseField(ExponentialHistogramXContent.ZERO_COUNT_FIELD); public static final ParseField ZERO_THRESHOLD_FIELD = new ParseField(ExponentialHistogramXContent.ZERO_THRESHOLD_FIELD); @@ -149,6 +151,10 @@ private static String valuesMinSubFieldName(String fullPath) { return fullPath + "._values_min"; } + private static String valuesMaxSubFieldName(String fullPath) { + return fullPath + "._values_max"; + } + static class Builder extends FieldMapper.Builder { private final FieldMapper.Parameter> meta = FieldMapper.Parameter.metaParam(); @@ -276,6 +282,7 @@ public void parse(DocumentParserContext context) throws IOException { Double sum = null; Double min = null; + Double max = null; Integer scale = null; ParsedZeroBucket zeroBucket = ParsedZeroBucket.DEFAULT; List negativeBuckets = Collections.emptyList(); @@ -317,6 +324,8 @@ public void parse(DocumentParserContext context) throws IOException { sum = parseDoubleAllowingInfinity(subParser); } else if (fieldName.equals(MIN_FIELD.getPreferredName())) { min = parseDoubleAllowingInfinity(subParser); + } else if (fieldName.equals(MAX_FIELD.getPreferredName())) { + max = parseDoubleAllowingInfinity(subParser); } else if (fieldName.equals(ZERO_FIELD.getPreferredName())) { zeroBucket = parseZeroBucket(subParser); } else if (fieldName.equals(POSITIVE_FIELD.getPreferredName())) { @@ -357,35 +366,9 @@ public void parse(DocumentParserContext context) throws IOException { ); } - if (sum == null) { - sum = ExponentialHistogramUtils.estimateSum( - IndexWithCount.asBuckets(scale, negativeBuckets).iterator(), - IndexWithCount.asBuckets(scale, positiveBuckets).iterator() - ); - } else { - if (totalValueCount == 0 && sum != 0.0) { - throw new DocumentParsingException( - subParser.getTokenLocation(), - "error parsing field [" + fullPath() + "], sum field must be zero if the histogram is empty, but got " + sum - ); - } - } - - if (min == null) { - OptionalDouble estimatedMin = ExponentialHistogramUtils.estimateMin( - ZeroBucket.create(zeroBucket.threshold(), zeroBucket.count), - IndexWithCount.asBuckets(scale, negativeBuckets), - IndexWithCount.asBuckets(scale, positiveBuckets) - ); - if (estimatedMin.isPresent()) { - min = estimatedMin.getAsDouble(); - } - } else if (totalValueCount == 0) { - throw new DocumentParsingException( - subParser.getTokenLocation(), - "error parsing field [" + fullPath() + "], min field must be null if the histogram is empty, but got " + min - ); - } + sum = validateOrEstimateSum(sum, scale, negativeBuckets, positiveBuckets, totalValueCount, subParser); + min = validateOrEstimateMin(min, zeroBucket, scale, negativeBuckets, positiveBuckets, totalValueCount, subParser); + max = validateOrEstimateMax(max, zeroBucket, scale, negativeBuckets, positiveBuckets, totalValueCount, subParser); BytesStreamOutput histogramBytesOutput = new BytesStreamOutput(); CompressedExponentialHistogram.writeHistogramBytes(histogramBytesOutput, scale, negativeBuckets, positiveBuckets); @@ -411,6 +394,13 @@ public void parse(DocumentParserContext context) throws IOException { ); context.doc().add(minField); } + if (max != null) { + NumericDocValuesField maxField = new NumericDocValuesField( + valuesMaxSubFieldName(fullPath()), + NumericUtils.doubleToSortableLong(max) + ); + context.doc().add(maxField); + } } catch (Exception ex) { if (ignoreMalformed.value() == false) { @@ -440,6 +430,81 @@ public void parse(DocumentParserContext context) throws IOException { context.path().remove(); } + private Double validateOrEstimateSum( + Double sum, + Integer scale, + List negativeBuckets, + List positiveBuckets, + long totalValueCount, + XContentSubParser subParser + ) { + if (sum == null) { + return ExponentialHistogramUtils.estimateSum( + IndexWithCount.asBuckets(scale, negativeBuckets).iterator(), + IndexWithCount.asBuckets(scale, positiveBuckets).iterator() + ); + } + if (totalValueCount == 0 && sum != 0.0) { + throw new DocumentParsingException( + subParser.getTokenLocation(), + "error parsing field [" + fullPath() + "], sum field must be zero if the histogram is empty, but got " + sum + ); + } + return sum; + } + + private Double validateOrEstimateMin( + Double parsedMin, + ParsedZeroBucket zeroBucket, + Integer scale, + List negativeBuckets, + List positiveBuckets, + long totalValueCount, + XContentSubParser subParser + ) { + if (parsedMin == null) { + OptionalDouble estimatedMin = ExponentialHistogramUtils.estimateMin( + ZeroBucket.create(zeroBucket.threshold(), zeroBucket.count), + IndexWithCount.asBuckets(scale, negativeBuckets), + IndexWithCount.asBuckets(scale, positiveBuckets) + ); + return estimatedMin.isPresent() ? estimatedMin.getAsDouble() : null; + } + if (totalValueCount == 0) { + throw new DocumentParsingException( + subParser.getTokenLocation(), + "error parsing field [" + fullPath() + "], min field must be null if the histogram is empty, but got " + parsedMin + ); + } + return parsedMin; + } + + private Double validateOrEstimateMax( + Double parsedMax, + ParsedZeroBucket zeroBucket, + Integer scale, + List negativeBuckets, + List positiveBuckets, + long totalValueCount, + XContentSubParser subParser + ) { + if (parsedMax == null) { + OptionalDouble estimatedMax = ExponentialHistogramUtils.estimateMax( + ZeroBucket.create(zeroBucket.threshold(), zeroBucket.count), + IndexWithCount.asBuckets(scale, negativeBuckets), + IndexWithCount.asBuckets(scale, positiveBuckets) + ); + return estimatedMax.isPresent() ? estimatedMax.getAsDouble() : null; + } + if (totalValueCount == 0) { + throw new DocumentParsingException( + subParser.getTokenLocation(), + "error parsing field [" + fullPath() + "], max field must be null if the histogram is empty, but got " + parsedMax + ); + } + return parsedMax; + } + private double parseDoubleAllowingInfinity(XContentParser parser) throws IOException { XContentParser.Token token = parser.nextToken(); boolean isValidNumber = token == XContentParser.Token.VALUE_NUMBER; @@ -665,6 +730,7 @@ private class ExponentialHistogramSyntheticFieldLoader implements CompositeSynth private long valueCount; private double valueSum; private double valueMin; + private double valueMax; @Override public SourceLoader.SyntheticFieldLoader.DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf) @@ -678,7 +744,8 @@ public SourceLoader.SyntheticFieldLoader.DocValuesLoader docValuesLoader(LeafRea NumericDocValues zeroThresholds = leafReader.getNumericDocValues(zeroThresholdSubFieldName(fullPath())); NumericDocValues valueCounts = leafReader.getNumericDocValues(valuesCountSubFieldName(fullPath())); NumericDocValues valueSums = leafReader.getNumericDocValues(valuesSumSubFieldName(fullPath())); - NumericDocValues valueMins = leafReader.getNumericDocValues(valuesMinSubFieldName(fullPath())); + NumericDocValues valueMinima = leafReader.getNumericDocValues(valuesMinSubFieldName(fullPath())); + NumericDocValues valueMaxima = leafReader.getNumericDocValues(valuesMaxSubFieldName(fullPath())); assert zeroThresholds != null; assert valueCounts != null; assert valueSums != null; @@ -695,11 +762,16 @@ public SourceLoader.SyntheticFieldLoader.DocValuesLoader docValuesLoader(LeafRea valueCount = valueCounts.longValue(); valueSum = NumericUtils.sortableLongToDouble(valueSums.longValue()); - if (valueMins != null && valueMins.advanceExact(docId)) { - valueMin = NumericUtils.sortableLongToDouble(valueMins.longValue()); + if (valueMinima != null && valueMinima.advanceExact(docId)) { + valueMin = NumericUtils.sortableLongToDouble(valueMinima.longValue()); } else { valueMin = Double.NaN; } + if (valueMaxima != null && valueMaxima.advanceExact(docId)) { + valueMax = NumericUtils.sortableLongToDouble(valueMaxima.longValue()); + } else { + valueMax = Double.NaN; + } return true; } binaryValue = null; @@ -718,7 +790,7 @@ public void write(XContentBuilder b) throws IOException { return; } - histogram.reset(zeroThreshold, valueCount, valueSum, valueMin, binaryValue); + histogram.reset(zeroThreshold, valueCount, valueSum, valueMin, valueMax, binaryValue); ExponentialHistogramXContent.serialize(b, histogram); } diff --git a/x-pack/plugin/mapper-exponential-histogram/src/test/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapperTests.java b/x-pack/plugin/mapper-exponential-histogram/src/test/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapperTests.java index 9067c4da5aa52..82471573cc2e0 100644 --- a/x-pack/plugin/mapper-exponential-histogram/src/test/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapperTests.java +++ b/x-pack/plugin/mapper-exponential-histogram/src/test/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapperTests.java @@ -138,6 +138,9 @@ private static Map createRandomHistogramValue(int maxBucketCount if (randomBoolean()) { result.put("min", randomDoubleBetween(-1000, 1000, true)); } + if (randomBoolean()) { + result.put("max", randomDoubleBetween(-1000, 1000, true)); + } } return result; } @@ -406,6 +409,11 @@ protected List exampleMalformedValues() { // Min provided for empty histogram exampleMalformedValue(b -> b.startObject().field("scale", 0).field("min", 42.0).endObject()).errorMatches( "min field must be null if the histogram is empty, but got 42.0" + ), + + // Max provided for empty histogram + exampleMalformedValue(b -> b.startObject().field("scale", 0).field("max", 42.0).endObject()).errorMatches( + "max field must be null if the histogram is empty, but got 42.0" ) ); } @@ -480,6 +488,21 @@ private Map convertHistogramToCanonicalForm(Map result.put("min", min); } + Object max = histogram.get("max"); + if (max == null) { + OptionalDouble estimatedMax = ExponentialHistogramUtils.estimateMax( + mapToZeroBucket(zeroBucket), + IndexWithCount.asBuckets(scale, negative), + IndexWithCount.asBuckets(scale, positive) + ); + if (estimatedMax.isPresent()) { + max = estimatedMax.getAsDouble(); + } + } + if (max != null) { + result.put("max", max); + } + if (zeroBucket != null) { result.put("zero", zeroBucket); } diff --git a/x-pack/plugin/mapper-exponential-histogram/src/yamlRestTest/resources/rest-api-spec/test/10_synthetic_source.yml b/x-pack/plugin/mapper-exponential-histogram/src/yamlRestTest/resources/rest-api-spec/test/10_synthetic_source.yml index c786fa6c61d1d..cfa029e9fb78d 100644 --- a/x-pack/plugin/mapper-exponential-histogram/src/yamlRestTest/resources/rest-api-spec/test/10_synthetic_source.yml +++ b/x-pack/plugin/mapper-exponential-histogram/src/yamlRestTest/resources/rest-api-spec/test/10_synthetic_source.yml @@ -30,6 +30,7 @@ setup: scale: 12 sum: 1234.56 min: -345.67 + max: 123.67 zero: threshold: 0.123456 count: 42 @@ -49,6 +50,7 @@ setup: scale: 12 sum: 1234.56 min: -345.67 + max: 123.67 zero: threshold: 0.123456 count: 42 @@ -71,6 +73,7 @@ setup: scale: -10 sum: 1234.56 min: -345.67 + max: 123.67 positive: indices: [1,2,3,4,5] counts: [6,7,8,9,10] @@ -87,6 +90,7 @@ setup: scale: -10 sum: 1234.56 min: -345.67 + max: 123.67 positive: indices: [1,2,3,4,5] counts: [6,7,8,9,10] @@ -106,6 +110,7 @@ setup: scale: 0 sum: 1234.56 min: 345.67 + max: 4123.67 positive: indices: [-100, 10, 20] counts: [3, 2, 1] @@ -119,6 +124,7 @@ setup: scale: 0 sum: 1234.56 min: 345.67 + max: 4123.67 positive: indices: [-100, 10, 20] counts: [3, 2, 1] @@ -133,8 +139,9 @@ setup: body: my_histo: scale: 0 - sum: 1234.56 + sum: -1234.56 min: -345.67 + max: -123.67 negative: indices: [-100, 10, 20] counts: [3, 2, 1] @@ -146,8 +153,9 @@ setup: - match: _source.my_histo: scale: 0 - sum: 1234.56 + sum: -1234.56 min: -345.67 + max: -123.67 negative: indices: [-100, 10, 20] counts: [3, 2, 1] @@ -215,6 +223,7 @@ setup: scale: 0 sum: 0.0 min: 0.0 + max: 0.0 zero: count: 101 @@ -231,6 +240,7 @@ setup: scale: 38 sum: 1E300 min: -1E300 + max: 1E300 zero: count: 2305843009213693952 # 2^61 to not cause overflows for the total value count sum threshold: 1E-300 @@ -249,6 +259,7 @@ setup: scale: 38 sum: 1E300 min: -1E300 + max: 1E300 zero: count: 2305843009213693952 threshold: 1E-300 @@ -268,13 +279,13 @@ setup: refresh: true body: my_histo: - scale: 1 + scale: 0 positive: - indices: [8] - counts: [1] + indices: [0,1] + counts: [1,1] negative: - indices: [0, 1] - counts: [1, 5] + indices: [0] + counts: [4] - do: get: @@ -282,15 +293,16 @@ setup: id: "1" - match: _source.my_histo: - scale: 1 - sum: 9.289321881345247 + scale: 0 + sum: -1.3333333333333335 min: -2.0 + max: 4.0 positive: - indices: [8] - counts: [1] + indices: [0,1] + counts: [1,1] negative: - indices: [0, 1] - counts: [1, 5] + indices: [0] + counts: [4] --- "Positive infinity aggregates": @@ -315,6 +327,7 @@ setup: scale: 0 sum: Infinity min: Infinity + max: Infinity positive: indices: [2000] counts: [1] @@ -342,6 +355,7 @@ setup: scale: 0 sum: -Infinity min: -Infinity + max: -Infinity negative: indices: [2000] counts: [1]