Skip to content

Commit 910d2e9

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 b1c3772 commit 910d2e9

File tree

2 files changed

+9
-7
lines changed

2 files changed

+9
-7
lines changed

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import org.elasticsearch.common.util.Maps;
2828
import org.elasticsearch.common.util.concurrent.AtomicArray;
2929
import org.elasticsearch.core.Releasable;
30-
import org.elasticsearch.core.Releasables;
3130
import org.elasticsearch.index.shard.ShardId;
3231
import org.elasticsearch.search.SearchContextMissingException;
3332
import org.elasticsearch.search.SearchPhaseResult;
@@ -102,7 +101,7 @@ abstract class AbstractSearchAsyncAction<Result extends SearchPhaseResult> exten
102101
private final AtomicBoolean requestCancelled = new AtomicBoolean();
103102

104103
// protected for tests
105-
protected final List<Releasable> releasables = new ArrayList<>();
104+
protected final SubscribableListener<Void> doneFuture = new SubscribableListener<>();
106105

107106
AbstractSearchAsyncAction(
108107
String name,
@@ -151,7 +150,7 @@ abstract class AbstractSearchAsyncAction<Result extends SearchPhaseResult> exten
151150
this.executor = executor;
152151
this.request = request;
153152
this.task = task;
154-
this.listener = ActionListener.runAfter(listener, () -> Releasables.close(releasables));
153+
this.listener = ActionListener.runBefore(listener, () -> doneFuture.onResponse(null));
155154
this.nodeIdToConnection = nodeIdToConnection;
156155
this.concreteIndexBoosts = concreteIndexBoosts;
157156
this.clusterStateVersion = clusterState.version();
@@ -182,7 +181,12 @@ protected void notifyListShards(
182181
* Registers a {@link Releasable} that will be closed when the search request finishes or fails.
183182
*/
184183
public void addReleasable(Releasable releasable) {
185-
releasables.add(releasable);
184+
var doneFuture = this.doneFuture;
185+
if (doneFuture.isDone()) {
186+
releasable.close();
187+
} else {
188+
doneFuture.addListener(ActionListener.releasing((releasable)));
189+
}
186190
}
187191

188192
/**

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

Lines changed: 1 addition & 3 deletions
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)