Skip to content

Commit d66b5ae

Browse files
Cleanup some redundancies around DfsQueryPhase (#116057)
No need to add the result consumer to teh context another time, it's already added to it in the constructor of `SearchDfsQueryThenFetchAsyncAction`. Also, no need to feed `this` and `this.results` to `getNextPhase` explicitly, there's only a single call to this method so we can safely clean up the redundant arguments.
1 parent 9ebe95a commit d66b5ae

File tree

8 files changed

+15
-22
lines changed

8 files changed

+15
-22
lines changed

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,7 @@ private void raisePhaseFailure(SearchPhaseExecutionException exception) {
689689
* @see #onShardResult(SearchPhaseResult, SearchShardIterator)
690690
*/
691691
final void onPhaseDone() { // as a tribute to @kimchy aka. finishHim()
692-
executeNextPhase(this, () -> getNextPhase(results, this));
692+
executeNextPhase(this, this::getNextPhase);
693693
}
694694

695695
@Override
@@ -746,11 +746,8 @@ protected final ShardSearchRequest buildShardSearchRequest(SearchShardIterator s
746746

747747
/**
748748
* Returns the next phase based on the results of the initial search phase
749-
* @param results the results of the initial search phase. Each non null element in the result array represent a successfully
750-
* executed shard request
751-
* @param context the search context for the next phase
752749
*/
753-
protected abstract SearchPhase getNextPhase(SearchPhaseResults<Result> results, SearchPhaseContext context);
750+
protected abstract SearchPhase getNextPhase();
754751

755752
private static final class PendingExecutions {
756753
private final Semaphore semaphore;

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,6 @@ final class DfsQueryPhase extends SearchPhase {
6565
this.nextPhaseFactory = nextPhaseFactory;
6666
this.context = context;
6767
this.searchTransportService = context.getSearchTransport();
68-
69-
// register the release of the query consumer to free up the circuit breaker memory
70-
// at the end of the search
71-
context.addReleasable(queryResult);
7268
}
7369

7470
@Override

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ protected void executePhaseOnShard(
9898
}
9999

100100
@Override
101-
protected SearchPhase getNextPhase(final SearchPhaseResults<DfsSearchResult> results, SearchPhaseContext context) {
101+
protected SearchPhase getNextPhase() {
102102
final List<DfsSearchResult> dfsSearchResults = results.getAtomicArray().asList();
103103
final AggregatedDfs aggregatedDfs = SearchPhaseController.aggregateDfs(dfsSearchResults);
104104
final List<DfsKnnResults> mergedKnnResults = SearchPhaseController.mergeKnnResults(getRequest(), dfsSearchResults);
@@ -107,8 +107,8 @@ protected SearchPhase getNextPhase(final SearchPhaseResults<DfsSearchResult> res
107107
aggregatedDfs,
108108
mergedKnnResults,
109109
queryPhaseResultConsumer,
110-
(queryResults) -> SearchQueryThenFetchAsyncAction.nextPhase(client, context, queryResults, aggregatedDfs),
111-
context
110+
(queryResults) -> SearchQueryThenFetchAsyncAction.nextPhase(client, this, queryResults, aggregatedDfs),
111+
this
112112
);
113113
}
114114

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ static SearchPhase nextPhase(
147147
}
148148

149149
@Override
150-
protected SearchPhase getNextPhase(final SearchPhaseResults<SearchPhaseResult> results, SearchPhaseContext context) {
150+
protected SearchPhase getNextPhase() {
151151
return nextPhase(client, this, results, null);
152152
}
153153

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ protected void executePhaseOnShard(
277277
}
278278

279279
@Override
280-
protected SearchPhase getNextPhase(SearchPhaseResults<SearchPhaseResult> results, SearchPhaseContext context) {
280+
protected SearchPhase getNextPhase() {
281281
return new SearchPhase(getName()) {
282282

283283
private void onExecuteFailure(Exception e) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ private AbstractSearchAsyncAction<SearchPhaseResult> createAction(
9494
SearchResponse.Clusters.EMPTY
9595
) {
9696
@Override
97-
protected SearchPhase getNextPhase(final SearchPhaseResults<SearchPhaseResult> results, SearchPhaseContext context) {
97+
protected SearchPhase getNextPhase() {
9898
return null;
9999
}
100100

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ protected void executePhaseOnShard(
139139
}
140140

141141
@Override
142-
protected SearchPhase getNextPhase(SearchPhaseResults<TestSearchPhaseResult> results, SearchPhaseContext context) {
142+
protected SearchPhase getNextPhase() {
143143
return new SearchPhase("test") {
144144
@Override
145145
public void run() {
@@ -255,7 +255,7 @@ protected void executePhaseOnShard(
255255
}
256256

257257
@Override
258-
protected SearchPhase getNextPhase(SearchPhaseResults<TestSearchPhaseResult> results, SearchPhaseContext context) {
258+
protected SearchPhase getNextPhase() {
259259
return new SearchPhase("test") {
260260
@Override
261261
public void run() {
@@ -359,7 +359,7 @@ protected void executePhaseOnShard(
359359
}
360360

361361
@Override
362-
protected SearchPhase getNextPhase(SearchPhaseResults<TestSearchPhaseResult> results, SearchPhaseContext context) {
362+
protected SearchPhase getNextPhase() {
363363
return new SearchPhase("test") {
364364
@Override
365365
public void run() {
@@ -488,7 +488,7 @@ protected void executePhaseOnShard(
488488
}
489489

490490
@Override
491-
protected SearchPhase getNextPhase(SearchPhaseResults<TestSearchPhaseResult> results, SearchPhaseContext context) {
491+
protected SearchPhase getNextPhase() {
492492
return new SearchPhase("test") {
493493
@Override
494494
public void run() {
@@ -600,7 +600,7 @@ protected void executePhaseOnShard(
600600
}
601601

602602
@Override
603-
protected SearchPhase getNextPhase(SearchPhaseResults<TestSearchPhaseResult> results, SearchPhaseContext context) {
603+
protected SearchPhase getNextPhase() {
604604
return new SearchPhase("test") {
605605
@Override
606606
public void run() {
@@ -680,7 +680,7 @@ protected void executePhaseOnShard(
680680
}
681681

682682
@Override
683-
protected SearchPhase getNextPhase(SearchPhaseResults<TestSearchPhaseResult> results, SearchPhaseContext context) {
683+
protected SearchPhase getNextPhase() {
684684
return new SearchPhase("test") {
685685
@Override
686686
public void run() {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ public void sendExecuteQuery(
204204
null
205205
) {
206206
@Override
207-
protected SearchPhase getNextPhase(SearchPhaseResults<SearchPhaseResult> results, SearchPhaseContext context) {
207+
protected SearchPhase getNextPhase() {
208208
return new SearchPhase("test") {
209209
@Override
210210
public void run() {

0 commit comments

Comments
 (0)