Skip to content

Commit a21f19a

Browse files
Make AbstractSearchAsyncAction release resources earlier
We can release resources earlier by releasing before responding to the listener (everything that needs retaining is retained via the search response already) as well as make the GCs life a little easier and get obvious correctness by using the listener that nulls out resources in a thread-safe manner intead of a non-thread-safe and mutable list shared across all kinds of places in the code.
1 parent a5e0423 commit a21f19a

File tree

2 files changed

+10
-7
lines changed

2 files changed

+10
-7
lines changed

server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java

+9-4
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@
2626
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
2727
import org.elasticsearch.common.util.Maps;
2828
import org.elasticsearch.common.util.concurrent.AtomicArray;
29+
import org.elasticsearch.common.util.concurrent.ListenableFuture;
2930
import org.elasticsearch.core.Releasable;
30-
import org.elasticsearch.core.Releasables;
3131
import org.elasticsearch.index.shard.ShardId;
3232
import org.elasticsearch.search.SearchContextMissingException;
3333
import org.elasticsearch.search.SearchPhaseResult;
@@ -102,7 +102,7 @@ abstract class AbstractSearchAsyncAction<Result extends SearchPhaseResult> exten
102102
private final AtomicBoolean requestCancelled = new AtomicBoolean();
103103

104104
// protected for tests
105-
protected final List<Releasable> releasables = new ArrayList<>();
105+
protected final ListenableFuture<Void> doneFuture = new ListenableFuture<>();
106106

107107
AbstractSearchAsyncAction(
108108
String name,
@@ -151,7 +151,7 @@ abstract class AbstractSearchAsyncAction<Result extends SearchPhaseResult> exten
151151
this.executor = executor;
152152
this.request = request;
153153
this.task = task;
154-
this.listener = ActionListener.runAfter(listener, () -> Releasables.close(releasables));
154+
this.listener = ActionListener.runBefore(listener, () -> doneFuture.onResponse(null));
155155
this.nodeIdToConnection = nodeIdToConnection;
156156
this.concreteIndexBoosts = concreteIndexBoosts;
157157
this.clusterStateVersion = clusterState.version();
@@ -182,7 +182,12 @@ protected void notifyListShards(
182182
* Registers a {@link Releasable} that will be closed when the search request finishes or fails.
183183
*/
184184
public void addReleasable(Releasable releasable) {
185-
releasables.add(releasable);
185+
var doneFuture = this.doneFuture;
186+
if (doneFuture.isDone()) {
187+
releasable.close();
188+
} else {
189+
doneFuture.addListener(ActionListener.releasing((releasable)));
190+
}
186191
}
187192

188193
/**

server/src/test/java/org/elasticsearch/action/search/MockSearchPhaseContext.java

+1-3
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1919
import org.elasticsearch.common.util.concurrent.AtomicArray;
2020
import org.elasticsearch.core.Nullable;
21-
import org.elasticsearch.core.Releasables;
2221
import org.elasticsearch.search.SearchPhaseResult;
2322
import org.elasticsearch.search.SearchShardTarget;
2423
import org.elasticsearch.search.internal.ShardSearchContextId;
@@ -104,8 +103,7 @@ public void sendSearchResponse(SearchResponseSections internalSearchResponse, At
104103
searchContextId
105104
)
106105
);
107-
Releasables.close(releasables);
108-
releasables.clear();
106+
doneFuture.onResponse(null);
109107
if (existing != null) {
110108
existing.decRef();
111109
}

0 commit comments

Comments
 (0)