Skip to content

Commit ffa76e5

Browse files
authored
Add option to merge histogram without upscaling (#137674)
1 parent 4c38246 commit ffa76e5

File tree

2 files changed

+32
-6
lines changed

2 files changed

+32
-6
lines changed

libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramMerger.java

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,25 @@ public ExponentialHistogram get() {
154154
return (result == null) ? ExponentialHistogram.empty() : result;
155155
}
156156

157+
/**
158+
* Merges the given histogram into the current result, not upscaling it.
159+
* This should be used when merging intermediate results to prevent accumulating errors.
160+
*
161+
* @param toAdd the histogram to merge
162+
*/
163+
public void addWithoutUpscaling(ExponentialHistogram toAdd) {
164+
add(toAdd, false);
165+
}
166+
167+
/**
168+
* Merges the given histogram into the current result. The histogram might be upscaled if needed.
169+
*
170+
* @param toAdd the histogram to merge
171+
*/
172+
public void add(ExponentialHistogram toAdd) {
173+
add(toAdd, true);
174+
}
175+
157176
// This algorithm is very efficient if B has roughly as many buckets as A.
158177
// However, if B is much smaller we still have to iterate over all buckets of A.
159178
// This can be optimized by buffering the buckets of small histograms and only merging them when we have enough buckets.
@@ -165,12 +184,7 @@ public ExponentialHistogram get() {
165184
// It would be possible to only enable the buffering for small histograms,
166185
// but the optimization seems not worth the added complexity at this point.
167186

168-
/**
169-
* Merges the given histogram into the current result.
170-
*
171-
* @param toAdd the histogram to merge
172-
*/
173-
public void add(ExponentialHistogram toAdd) {
187+
private void add(ExponentialHistogram toAdd, boolean allowUpscaling) {
174188
ExponentialHistogram a = result == null ? ExponentialHistogram.empty() : result;
175189
ExponentialHistogram b = toAdd;
176190

@@ -193,6 +207,10 @@ public void add(ExponentialHistogram toAdd) {
193207
// This might involve increasing the scale for B, which would increase its indices.
194208
// We need to ensure that we do not exceed MAX_INDEX / MIN_INDEX in this case.
195209
int targetScale = Math.min(maxScale, a.scale());
210+
if (allowUpscaling == false) {
211+
targetScale = Math.min(targetScale, b.scale());
212+
}
213+
196214
if (targetScale > b.scale()) {
197215
if (negBucketsB.hasNext()) {
198216
long smallestIndex = negBucketsB.peekIndex();

libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramMergerTests.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,14 @@ public void testEmptyZeroBucketIgnored() {
105105
assertThat(posBuckets.hasNext(), equalTo(false));
106106
}
107107

108+
public void testMergeWithoutUpscaling() {
109+
ExponentialHistogram histo = createAutoReleasedHistogram(b -> b.scale(0).setPositiveBucket(2, 42));
110+
try (ExponentialHistogramMerger merger = ExponentialHistogramMerger.create(100, breaker())) {
111+
merger.addWithoutUpscaling(histo);
112+
assertThat(merger.get(), equalTo(histo));
113+
}
114+
}
115+
108116
public void testAggregatesCorrectness() {
109117
double[] firstValues = randomDoubles(100).map(val -> val * 2 - 1).toArray();
110118
double[] secondValues = randomDoubles(50).map(val -> val * 2 - 1).toArray();

0 commit comments

Comments
 (0)