Skip to content

Commit 8b0b1f2

Browse files
authored
Fix potential circuit breaker leak on InternalGeoGrid (#88273) (#88298)
During reduce time we are creating a LongObjectPagedHashMap that will not get released if there is an error when it is getting populated. This commit adds a finally clause so it is always release.
1 parent e01570c commit 8b0b1f2

File tree

2 files changed

+32
-20
lines changed

2 files changed

+32
-20
lines changed

docs/changelog/88273.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 88273
2+
summary: Fix potential circuit breaker leak on `InternalGeoGrid`
3+
area: Geo
4+
type: bug
5+
issues:
6+
- 88261

server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/InternalGeoGrid.java

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.elasticsearch.common.io.stream.StreamOutput;
1212
import org.elasticsearch.common.io.stream.Writeable;
1313
import org.elasticsearch.common.util.LongObjectPagedHashMap;
14+
import org.elasticsearch.core.Releasables;
1415
import org.elasticsearch.search.aggregations.AggregationReduceContext;
1516
import org.elasticsearch.search.aggregations.InternalAggregation;
1617
import org.elasticsearch.search.aggregations.InternalAggregations;
@@ -78,30 +79,35 @@ public List<InternalGeoGridBucket> getBuckets() {
7879
@Override
7980
public InternalGeoGrid<B> reduce(List<InternalAggregation> aggregations, AggregationReduceContext reduceContext) {
8081
LongObjectPagedHashMap<List<InternalGeoGridBucket>> buckets = null;
81-
for (InternalAggregation aggregation : aggregations) {
82-
@SuppressWarnings("unchecked")
83-
InternalGeoGrid<B> grid = (InternalGeoGrid<B>) aggregation;
84-
if (buckets == null) {
85-
buckets = new LongObjectPagedHashMap<>(grid.buckets.size(), reduceContext.bigArrays());
86-
}
87-
for (Object obj : grid.buckets) {
88-
InternalGeoGridBucket bucket = (InternalGeoGridBucket) obj;
89-
List<InternalGeoGridBucket> existingBuckets = buckets.get(bucket.hashAsLong());
90-
if (existingBuckets == null) {
91-
existingBuckets = new ArrayList<>(aggregations.size());
92-
buckets.put(bucket.hashAsLong(), existingBuckets);
82+
final BucketPriorityQueue<InternalGeoGridBucket> ordered;
83+
try {
84+
for (InternalAggregation aggregation : aggregations) {
85+
@SuppressWarnings("unchecked")
86+
InternalGeoGrid<B> grid = (InternalGeoGrid<B>) aggregation;
87+
if (buckets == null) {
88+
buckets = new LongObjectPagedHashMap<>(grid.buckets.size(), reduceContext.bigArrays());
89+
}
90+
for (InternalGeoGridBucket bucket : grid.buckets) {
91+
List<InternalGeoGridBucket> existingBuckets = buckets.get(bucket.hashAsLong());
92+
if (existingBuckets == null) {
93+
existingBuckets = new ArrayList<>(aggregations.size());
94+
buckets.put(bucket.hashAsLong(), existingBuckets);
95+
}
96+
existingBuckets.add(bucket);
9397
}
94-
existingBuckets.add(bucket);
9598
}
96-
}
9799

98-
final int size = Math.toIntExact(reduceContext.isFinalReduce() == false ? buckets.size() : Math.min(requiredSize, buckets.size()));
99-
BucketPriorityQueue<InternalGeoGridBucket> ordered = new BucketPriorityQueue<>(size);
100-
for (LongObjectPagedHashMap.Cursor<List<InternalGeoGridBucket>> cursor : buckets) {
101-
List<InternalGeoGridBucket> sameCellBuckets = cursor.value;
102-
ordered.insertWithOverflow(reduceBucket(sameCellBuckets, reduceContext));
100+
final int size = Math.toIntExact(
101+
reduceContext.isFinalReduce() == false ? buckets.size() : Math.min(requiredSize, buckets.size())
102+
);
103+
ordered = new BucketPriorityQueue<>(size);
104+
for (LongObjectPagedHashMap.Cursor<List<InternalGeoGridBucket>> cursor : buckets) {
105+
List<InternalGeoGridBucket> sameCellBuckets = cursor.value;
106+
ordered.insertWithOverflow(reduceBucket(sameCellBuckets, reduceContext));
107+
}
108+
} finally {
109+
Releasables.close(buckets);
103110
}
104-
buckets.close();
105111
InternalGeoGridBucket[] list = new InternalGeoGridBucket[ordered.size()];
106112
for (int i = ordered.size() - 1; i >= 0; i--) {
107113
list[i] = ordered.pop();

0 commit comments

Comments
 (0)