Skip to content

Commit 226d5c5

Browse files
Make AbstractSearchAsyncAction release resources earlier (#124275) (#124800)
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 bdd7b6a commit 226d5c5

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
@@ -28,7 +28,6 @@
2828
import org.elasticsearch.common.util.Maps;
2929
import org.elasticsearch.common.util.concurrent.AtomicArray;
3030
import org.elasticsearch.core.Releasable;
31-
import org.elasticsearch.core.Releasables;
3231
import org.elasticsearch.index.shard.ShardId;
3332
import org.elasticsearch.search.SearchContextMissingException;
3433
import org.elasticsearch.search.SearchPhaseResult;
@@ -103,7 +102,7 @@ abstract class AbstractSearchAsyncAction<Result extends SearchPhaseResult> exten
103102
private final AtomicBoolean requestCancelled = new AtomicBoolean();
104103

105104
// protected for tests
106-
protected final List<Releasable> releasables = new ArrayList<>();
105+
protected final SubscribableListener<Void> doneFuture = new SubscribableListener<>();
107106

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

189193
/**

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)