Browse Source

Don't create a new `DoubleHistogram` instance for empty buckets. (#92547)

Currently, the `percentile` and `percentile_ranks` aggregations create a new `DoubleHistogram` instance each time when creating empty aggregation. This is very wasteful. Especially when `numberOfSignificantValueDigits` is `5`. Then each instance costs ~5MB.

This change uses `null` instead. At reduce time `InternalHDRPercentileRanks` with a `null` state are skipped.
In case all `InternalHDRPercentileRanks` are null then reduce uses an empty `DoubleHistogram` instance.

Note that `buildEmptyAggregations()` could also return an empty `DoubleHistogram` instance. However, serializing an empty `DoubleHistogram` instance also adds significant overhead. (`ByteBuffer` instance is created based on `DoubleHistogram#getNeededByteBufferCapacity()`, and that can cause a very large ByteBuffer instance to be created)
Martijn van Groningen 2 years ago
parent
commit
a1ea6ea8b4

+ 5 - 0
docs/changelog/92547.yaml

@@ -0,0 +1,5 @@
+pr: 92547
+summary: Don't create a new `DoubleHistogram` instance for empty buckets
+area: Aggregations
+type: bug
+issues: []

+ 41 - 9
server/src/main/java/org/elasticsearch/search/aggregations/metrics/AbstractInternalHDRPercentiles.java

@@ -9,6 +9,7 @@
 package org.elasticsearch.search.aggregations.metrics;
 
 import org.HdrHistogram.DoubleHistogram;
+import org.elasticsearch.Version;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.search.DocValueFormat;
@@ -27,6 +28,8 @@ import java.util.zip.DataFormatException;
 
 abstract class AbstractInternalHDRPercentiles extends InternalNumericMetricsAggregation.MultiValue {
 
+    private static final DoubleHistogram EMPTY_HISTOGRAM = new DoubleHistogram(3);
+
     protected final double[] keys;
     protected final DoubleHistogram state;
     protected final boolean keyed;
@@ -51,29 +54,55 @@ abstract class AbstractInternalHDRPercentiles extends InternalNumericMetricsAggr
     protected AbstractInternalHDRPercentiles(StreamInput in) throws IOException {
         super(in);
         keys = in.readDoubleArray();
+        if (in.getVersion().onOrAfter(Version.V_8_7_0)) {
+            if (in.readBoolean()) {
+                state = decode(in);
+            } else {
+                state = null;
+            }
+        } else {
+            state = decode(in);
+        }
+        keyed = in.readBoolean();
+    }
+
+    private DoubleHistogram decode(StreamInput in) throws IOException {
         long minBarForHighestToLowestValueRatio = in.readLong();
         final int serializedLen = in.readVInt();
         byte[] bytes = new byte[serializedLen];
         in.readBytes(bytes, 0, serializedLen);
         ByteBuffer stateBuffer = ByteBuffer.wrap(bytes);
         try {
-            state = DoubleHistogram.decodeFromCompressedByteBuffer(stateBuffer, minBarForHighestToLowestValueRatio);
+            return DoubleHistogram.decodeFromCompressedByteBuffer(stateBuffer, minBarForHighestToLowestValueRatio);
         } catch (DataFormatException e) {
             throw new IOException("Failed to decode DoubleHistogram for aggregation [" + name + "]", e);
         }
-        keyed = in.readBoolean();
     }
 
     @Override
     protected void doWriteTo(StreamOutput out) throws IOException {
         out.writeNamedWriteable(format);
         out.writeDoubleArray(keys);
+        if (out.getVersion().onOrAfter(Version.V_8_7_0)) {
+            if (this.state != null) {
+                out.writeBoolean(true);
+                encode(this.state, out);
+            } else {
+                out.writeBoolean(false);
+            }
+        } else {
+            DoubleHistogram state = this.state != null ? this.state : EMPTY_HISTOGRAM;
+            encode(state, out);
+        }
+        out.writeBoolean(keyed);
+    }
+
+    private static void encode(DoubleHistogram state, StreamOutput out) throws IOException {
         out.writeLong(state.getHighestToLowestValueRatio());
         ByteBuffer stateBuffer = ByteBuffer.allocate(state.getNeededByteBufferCapacity());
         final int serializedLen = state.encodeIntoCompressedByteBuffer(stateBuffer);
         out.writeVInt(serializedLen);
         out.writeBytes(stateBuffer.array(), 0, serializedLen);
-        out.writeBoolean(keyed);
     }
 
     @Override
@@ -95,10 +124,6 @@ abstract class AbstractInternalHDRPercentiles extends InternalNumericMetricsAggr
 
     public abstract double value(double key);
 
-    public long getEstimatedMemoryFootprint() {
-        return state.getEstimatedFootprintInBytes();
-    }
-
     /**
      * Return the internal {@link DoubleHistogram} sketch for this metric.
      */
@@ -125,12 +150,18 @@ abstract class AbstractInternalHDRPercentiles extends InternalNumericMetricsAggr
         DoubleHistogram merged = null;
         for (InternalAggregation aggregation : aggregations) {
             final AbstractInternalHDRPercentiles percentiles = (AbstractInternalHDRPercentiles) aggregation;
+            if (percentiles.state == null) {
+                continue;
+            }
             if (merged == null) {
                 merged = new DoubleHistogram(percentiles.state);
                 merged.setAutoResize(true);
             }
             merged.add(percentiles.state);
         }
+        if (merged == null) {
+            merged = EMPTY_HISTOGRAM;
+        }
         return createReduced(getName(), keys, merged, keyed, getMetadata());
     }
 
@@ -149,6 +180,7 @@ abstract class AbstractInternalHDRPercentiles extends InternalNumericMetricsAggr
 
     @Override
     public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
+        DoubleHistogram state = this.state != null ? this.state : EMPTY_HISTOGRAM;
         if (keyed) {
             builder.startObject(CommonFields.VALUES.getPreferredName());
             for (int i = 0; i < keys.length; ++i) {
@@ -196,8 +228,8 @@ abstract class AbstractInternalHDRPercentiles extends InternalNumericMetricsAggr
             super.hashCode(),
             keyed,
             Arrays.hashCode(keys),
-            state.getIntegerToDoubleValueConversionRatio(),
-            state.getTotalCount()
+            state != null ? state.getIntegerToDoubleValueConversionRatio() : 0,
+            state != null ? state.getTotalCount() : 0
         );
     }
 }

+ 1 - 4
server/src/main/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksAggregator.java

@@ -45,10 +45,7 @@ class HDRPercentileRanksAggregator extends AbstractHDRPercentilesAggregator {
 
     @Override
     public InternalAggregation buildEmptyAggregation() {
-        DoubleHistogram state;
-        state = new DoubleHistogram(numberOfSignificantValueDigits);
-        state.setAutoResize(true);
-        return new InternalHDRPercentileRanks(name, keys, state, keyed, format, metadata());
+        return new InternalHDRPercentileRanks(name, keys, null, keyed, format, metadata());
     }
 
     @Override

+ 1 - 4
server/src/main/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesAggregator.java

@@ -55,9 +55,6 @@ class HDRPercentilesAggregator extends AbstractHDRPercentilesAggregator {
 
     @Override
     public InternalAggregation buildEmptyAggregation() {
-        DoubleHistogram state;
-        state = new DoubleHistogram(numberOfSignificantValueDigits);
-        state.setAutoResize(true);
-        return new InternalHDRPercentiles(name, keys, state, keyed, format, metadata());
+        return new InternalHDRPercentiles(name, keys, null, keyed, format, metadata());
     }
 }

+ 1 - 1
server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalHDRPercentileRanks.java

@@ -73,7 +73,7 @@ public class InternalHDRPercentileRanks extends AbstractInternalHDRPercentiles i
     }
 
     public static double percentileRank(DoubleHistogram state, double value) {
-        if (state.getTotalCount() == 0) {
+        if (state == null || state.getTotalCount() == 0) {
             return Double.NaN;
         }
         double percentileRank = state.getPercentileAtOrBelowValue(value);

+ 1 - 1
server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalHDRPercentiles.java

@@ -48,7 +48,7 @@ public class InternalHDRPercentiles extends AbstractInternalHDRPercentiles imple
 
     @Override
     public double percentile(double percent) {
-        if (state.getTotalCount() == 0) {
+        if (state == null || state.getTotalCount() == 0) {
             return Double.NaN;
         }
         return state.getValueAtPercentile(percent);