Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,21 @@ public class ExponentialHistogramMerger implements Accountable, Releasable {
/**
* Creates a new instance with the specified bucket limit.
*
* @param bucketLimit the maximum number of buckets the result histogram is allowed to have
* @param bucketLimit the maximum number of buckets the result histogram is allowed to have, must be at least 4
* @param circuitBreaker the circuit breaker to use to limit memory allocations
*/
public static ExponentialHistogramMerger create(int bucketLimit, ExponentialHistogramCircuitBreaker circuitBreaker) {
circuitBreaker.adjustBreaker(BASE_SIZE);
return new ExponentialHistogramMerger(bucketLimit, circuitBreaker);
boolean success = false;
try {
ExponentialHistogramMerger result = new ExponentialHistogramMerger(bucketLimit, circuitBreaker);
success = true;
return result;
} finally {
if (success == false) {
circuitBreaker.adjustBreaker(-BASE_SIZE);
}
}
}

private ExponentialHistogramMerger(int bucketLimit, ExponentialHistogramCircuitBreaker circuitBreaker) {
Expand All @@ -69,6 +78,10 @@ private ExponentialHistogramMerger(int bucketLimit, ExponentialHistogramCircuitB

// Only intended for testing, using this in production means an unnecessary reduction of precision
private ExponentialHistogramMerger(int bucketLimit, int maxScale, ExponentialHistogramCircuitBreaker circuitBreaker) {
// We need at least four buckets to represent any possible distribution
if (bucketLimit < 4) {
throw new IllegalArgumentException("The bucket limit must be at least 4");
}
this.bucketLimit = bucketLimit;
this.maxScale = maxScale;
this.circuitBreaker = circuitBreaker;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public class ExponentialHistogramGeneratorTests extends ExponentialHistogramTest

public void testVeryLargeValue() {
double value = Double.MAX_VALUE / 10;
ExponentialHistogram histo = createAutoReleasedHistogram(1, value);
ExponentialHistogram histo = createAutoReleasedHistogram(4, value);
long index = histo.positiveBuckets().iterator().peekIndex();
int scale = histo.scale();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import static org.elasticsearch.exponentialhistogram.ExponentialHistogram.MIN_INDEX;
import static org.elasticsearch.exponentialhistogram.ExponentialScaleUtils.adjustScale;
import static org.hamcrest.Matchers.closeTo;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;

Expand Down Expand Up @@ -115,7 +116,7 @@ public void testAggregatesCorrectness() {
try (
// Merge some empty histograms too to test that code path
ReleasableExponentialHistogram merged = ExponentialHistogram.merge(
2,
4,
breaker(),
ExponentialHistogram.empty(),
createAutoReleasedHistogram(10, firstValues),
Expand Down Expand Up @@ -152,6 +153,15 @@ public void testUpscalingDoesNotExceedIndexLimits() {
}
}

public void testMinimumBucketCountBounded() {
try {
ExponentialHistogram.merge(3, breaker(), ExponentialHistogram.empty(), ExponentialHistogram.empty());
fail("Expected exception");
} catch (IllegalArgumentException e) {
assertThat(e.getMessage(), containsString("limit must be at least 4"));
}
}

/**
* Verify that the resulting histogram is independent of the order of elements and therefore merges performed.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class ExponentialHistogramUtilsTests extends ExponentialHistogramTestCase
public void testRandomDataSumEstimation() {
for (int i = 0; i < 100; i++) {
int valueCount = randomIntBetween(100, 10_000);
int bucketCount = randomIntBetween(2, 500);
int bucketCount = randomIntBetween(4, 500);

double correctSum = 0;
double sign = randomBoolean() ? 1 : -1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public void testEmptyHistogram() {
}

public void testSingleValueHistogram() {
ExponentialHistogram histo = createAutoReleasedHistogram(1, 42.0);
ExponentialHistogram histo = createAutoReleasedHistogram(4, 42.0);
for (double q : QUANTILES_TO_TEST) {
assertThat(ExponentialHistogramQuantile.getQuantile(histo, q), closeTo(42, 0.0000001));
}
Expand Down