Skip to content

Commit

Permalink
Aggregations: Fix infinite loop in the histogram reduce logic.
Browse files Browse the repository at this point in the history
The histogram reduce method can run into an infinite loop if the
Rounding.nextRoundingValue value is buggy, which happened to be the case for
DayTimeZoneRoundingFloor.

DayTimeZoneRoundingFloor is fixed, and the histogram reduce method has been
changed to fail instead of running into an infinite loop in case of a buffy
nextRoundingValue impl.

Close elastic#6965
  • Loading branch information
jpountz committed Jul 24, 2014
1 parent f5d1e0a commit 9f2cc0c
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 4 deletions.
Expand Up @@ -162,7 +162,7 @@ public long valueForKey(long time) {

@Override
public long nextRoundingValue(long value) {
return unit.field().roundCeiling(value + 1);
return unit.field().getDurationField().add(value, 1);
}

@Override
Expand Down Expand Up @@ -210,7 +210,7 @@ public long valueForKey(long key) {

@Override
public long nextRoundingValue(long value) {
return unit.field().roundCeiling(value + 1);
return unit.field().getDurationField().add(value, 1);
}

@Override
Expand Down Expand Up @@ -262,7 +262,7 @@ public long valueForKey(long time) {

@Override
public long nextRoundingValue(long value) {
return unit.field().getDurationField().getUnitMillis() + value;
return unit.field().getDurationField().add(value, 1);
}

@Override
Expand Down
Expand Up @@ -320,10 +320,11 @@ public InternalAggregation reduce(ReduceContext reduceContext) {
B nextBucket = list.get(iter.nextIndex());
if (lastBucket != null) {
long key = emptyBucketInfo.rounding.nextRoundingValue(lastBucket.key);
while (key != nextBucket.key) {
while (key < nextBucket.key) {
iter.add(createBucket(key, 0, emptyBucketInfo.subAggregations, formatter));
key = emptyBucketInfo.rounding.nextRoundingValue(key);
}
assert key == nextBucket.key;
}
lastBucket = iter.next();
}
Expand Down
Expand Up @@ -1299,4 +1299,36 @@ public void singleValue_WithMultipleDateFormatsFromMapping() throws Exception {
assertThat(bucket, Matchers.notNullValue());
assertThat(bucket.getDocCount(), equalTo(5l));
}

public void testIssue6965() {
SearchResponse response = client().prepareSearch("idx")
.addAggregation(dateHistogram("histo").field("date").preZone("+01:00").interval(DateHistogram.Interval.MONTH).minDocCount(0))
.execute().actionGet();

assertSearchResponse(response);


DateHistogram histo = response.getAggregations().get("histo");
assertThat(histo, notNullValue());
assertThat(histo.getName(), equalTo("histo"));
assertThat(histo.getBuckets().size(), equalTo(3));

DateTime key = new DateTime(2012, 1, 1, 0, 0, DateTimeZone.UTC);
DateHistogram.Bucket bucket = getBucket(histo, key);
assertThat(bucket, notNullValue());
assertThat(bucket.getKeyAsNumber().longValue(), equalTo(key.getMillis()));
assertThat(bucket.getDocCount(), equalTo(1l));

key = new DateTime(2012, 2, 1, 0, 0, DateTimeZone.UTC);
bucket = getBucket(histo, key);
assertThat(bucket, notNullValue());
assertThat(bucket.getKeyAsNumber().longValue(), equalTo(key.getMillis()));
assertThat(bucket.getDocCount(), equalTo(2l));

key = new DateTime(2012, 3, 1, 0, 0, DateTimeZone.UTC);
bucket = getBucket(histo, key);
assertThat(bucket, notNullValue());
assertThat(bucket.getKeyAsNumber().longValue(), equalTo(key.getMillis()));
assertThat(bucket.getDocCount(), equalTo(3l));
}
}

0 comments on commit 9f2cc0c

Please sign in to comment.