Skip to content

Commit 9de1215

Browse files
Release AggregationContext more eagerly
No need to drag a reference to this thing around until the query result is fully release. Track it via a future (so it's unlinked on release) and release it as soon as we set the aggregations. Only release on aggs release and ref-counting to zero as a backup.
1 parent 677ab35 commit 9de1215

File tree

3 files changed

+22
-14
lines changed

3 files changed

+22
-14
lines changed

server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.elasticsearch.common.breaker.CircuitBreaker;
2727
import org.elasticsearch.common.lucene.search.Queries;
2828
import org.elasticsearch.core.Nullable;
29-
import org.elasticsearch.core.Releasable;
3029
import org.elasticsearch.core.TimeValue;
3130
import org.elasticsearch.index.IndexMode;
3231
import org.elasticsearch.index.IndexService;
@@ -51,6 +50,7 @@
5150
import org.elasticsearch.index.search.NestedHelper;
5251
import org.elasticsearch.index.shard.IndexShard;
5352
import org.elasticsearch.search.aggregations.SearchContextAggregations;
53+
import org.elasticsearch.search.aggregations.support.AggregationContext;
5454
import org.elasticsearch.search.builder.SearchSourceBuilder;
5555
import org.elasticsearch.search.collapse.CollapseContext;
5656
import org.elasticsearch.search.dfs.DfsSearchResult;
@@ -861,8 +861,8 @@ public QuerySearchResult queryResult() {
861861
return queryResult;
862862
}
863863

864-
public void addQuerySearchResultReleasable(Releasable releasable) {
865-
queryResult.addReleasable(releasable);
864+
public void addAggregationContext(AggregationContext aggregationContext) {
865+
queryResult.addAggregationContext(aggregationContext);
866866
}
867867

868868
@Override

server/src/main/java/org/elasticsearch/search/SearchService.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -1613,7 +1613,7 @@ private void parseSource(DefaultSearchContext context, SearchSourceBuilder sourc
16131613
enableRewriteAggsToFilterByFilter,
16141614
source.aggregations().isInSortOrderExecutionRequired()
16151615
);
1616-
context.addQuerySearchResultReleasable(aggContext);
1616+
context.addAggregationContext(aggContext);
16171617
try {
16181618
final AggregatorFactories factories = source.aggregations().build(aggContext, null);
16191619
context.aggregations(

server/src/main/java/org/elasticsearch/search/query/QuerySearchResult.java

+18-10
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,14 @@
1313
import org.apache.lucene.search.TotalHits;
1414
import org.elasticsearch.TransportVersion;
1515
import org.elasticsearch.TransportVersions;
16+
import org.elasticsearch.action.ActionListener;
17+
import org.elasticsearch.action.support.SubscribableListener;
1618
import org.elasticsearch.common.io.stream.DelayableWriteable;
1719
import org.elasticsearch.common.io.stream.StreamInput;
1820
import org.elasticsearch.common.io.stream.StreamOutput;
1921
import org.elasticsearch.common.lucene.search.TopDocsAndMaxScore;
2022
import org.elasticsearch.core.Nullable;
2123
import org.elasticsearch.core.RefCounted;
22-
import org.elasticsearch.core.Releasable;
2324
import org.elasticsearch.core.Releasables;
2425
import org.elasticsearch.core.SimpleRefCounted;
2526
import org.elasticsearch.search.DocValueFormat;
@@ -28,6 +29,7 @@
2829
import org.elasticsearch.search.SearchShardTarget;
2930
import org.elasticsearch.search.aggregations.InternalAggregation;
3031
import org.elasticsearch.search.aggregations.InternalAggregations;
32+
import org.elasticsearch.search.aggregations.support.AggregationContext;
3133
import org.elasticsearch.search.internal.ShardSearchContextId;
3234
import org.elasticsearch.search.internal.ShardSearchRequest;
3335
import org.elasticsearch.search.profile.SearchProfileDfsPhaseResult;
@@ -37,8 +39,6 @@
3739
import org.elasticsearch.transport.LeakTracker;
3840

3941
import java.io.IOException;
40-
import java.util.ArrayList;
41-
import java.util.List;
4242

4343
import static org.elasticsearch.common.lucene.Lucene.readTopDocs;
4444
import static org.elasticsearch.common.lucene.Lucene.writeTopDocs;
@@ -75,7 +75,7 @@ public final class QuerySearchResult extends SearchPhaseResult {
7575

7676
private final RefCounted refCounted;
7777

78-
private final List<Releasable> toRelease;
78+
private final SubscribableListener<Void> aggsContextReleased;
7979

8080
public QuerySearchResult() {
8181
this(false);
@@ -99,22 +99,22 @@ public QuerySearchResult(StreamInput in, boolean delayedAggregations) throws IOE
9999
readFromWithId(id, in, delayedAggregations);
100100
}
101101
refCounted = null;
102-
toRelease = null;
102+
aggsContextReleased = null;
103103
}
104104

105105
public QuerySearchResult(ShardSearchContextId contextId, SearchShardTarget shardTarget, ShardSearchRequest shardSearchRequest) {
106106
this.contextId = contextId;
107107
setSearchShardTarget(shardTarget);
108108
isNull = false;
109109
setShardSearchRequest(shardSearchRequest);
110-
this.toRelease = new ArrayList<>();
111110
this.refCounted = LeakTracker.wrap(new SimpleRefCounted());
111+
this.aggsContextReleased = new SubscribableListener<>();
112112
}
113113

114114
private QuerySearchResult(boolean isNull) {
115115
this.isNull = isNull;
116116
this.refCounted = null;
117-
toRelease = null;
117+
aggsContextReleased = null;
118118
}
119119

120120
/**
@@ -275,16 +275,24 @@ public void releaseAggs() {
275275
aggregations.close();
276276
aggregations = null;
277277
}
278+
releaseAggsContext();
278279
}
279280

280-
public void addReleasable(Releasable releasable) {
281-
toRelease.add(releasable);
281+
public void addAggregationContext(AggregationContext aggsContext) {
282+
aggsContextReleased.addListener(ActionListener.releasing(aggsContext));
282283
}
283284

284285
public void aggregations(InternalAggregations aggregations) {
285286
assert this.aggregations == null : "aggregations already set to [" + this.aggregations + "]";
286287
this.aggregations = aggregations == null ? null : DelayableWriteable.referencing(aggregations);
287288
hasAggs = aggregations != null;
289+
releaseAggsContext();
290+
}
291+
292+
private void releaseAggsContext() {
293+
if (aggsContextReleased != null) {
294+
aggsContextReleased.onResponse(null);
295+
}
288296
}
289297

290298
@Nullable
@@ -547,7 +555,7 @@ public boolean tryIncRef() {
547555
public boolean decRef() {
548556
if (refCounted != null) {
549557
if (refCounted.decRef()) {
550-
Releasables.close(toRelease);
558+
aggsContextReleased.onResponse(null);
551559
return true;
552560
}
553561
return false;

0 commit comments

Comments
 (0)