diff --git a/server/src/main/java/org/elasticsearch/action/search/FetchSearchPhase.java b/server/src/main/java/org/elasticsearch/action/search/FetchSearchPhase.java index 0fbface3793a8..8568b60916761 100644 --- a/server/src/main/java/org/elasticsearch/action/search/FetchSearchPhase.java +++ b/server/src/main/java/org/elasticsearch/action/search/FetchSearchPhase.java @@ -271,7 +271,7 @@ private void moveToNextPhase( ) { context.executeNextPhase(this, () -> { var resp = SearchPhaseController.merge(context.getRequest().scroll() != null, reducedQueryPhase, fetchResultsArr); - context.addReleasable(resp::decRef); + context.addReleasable(resp); return nextPhaseFactory.apply(resp, searchPhaseShardResults); }); } diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchResponseSections.java b/server/src/main/java/org/elasticsearch/action/search/SearchResponseSections.java index 8c9a42a61e33e..9d85348b80d62 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchResponseSections.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchResponseSections.java @@ -9,14 +9,12 @@ package org.elasticsearch.action.search; -import org.elasticsearch.core.RefCounted; -import org.elasticsearch.core.SimpleRefCounted; +import org.elasticsearch.core.Releasable; import org.elasticsearch.search.SearchHits; import org.elasticsearch.search.aggregations.InternalAggregations; import org.elasticsearch.search.profile.SearchProfileResults; import org.elasticsearch.search.profile.SearchProfileShardResult; import org.elasticsearch.search.suggest.Suggest; -import org.elasticsearch.transport.LeakTracker; import java.util.Collections; import java.util.Map; @@ -25,7 +23,7 @@ * Holds some sections that a search response is composed of (hits, aggs, suggestions etc.) during some steps of the search response * building. */ -public class SearchResponseSections implements RefCounted { +public class SearchResponseSections implements Releasable { public static final SearchResponseSections EMPTY_WITH_TOTAL_HITS = new SearchResponseSections( SearchHits.EMPTY_WITH_TOTAL_HITS, @@ -53,8 +51,6 @@ public class SearchResponseSections implements RefCounted { protected final Boolean terminatedEarly; protected final int numReducePhases; - private final RefCounted refCounted; - public SearchResponseSections( SearchHits hits, InternalAggregations aggregations, @@ -72,7 +68,6 @@ public SearchResponseSections( this.timedOut = timedOut; this.terminatedEarly = terminatedEarly; this.numReducePhases = numReducePhases; - refCounted = hits.getHits().length > 0 ? LeakTracker.wrap(new SimpleRefCounted()) : ALWAYS_REFERENCED; } public final SearchHits hits() { @@ -97,26 +92,7 @@ public final Map profile() { } @Override - public void incRef() { - refCounted.incRef(); - } - - @Override - public boolean tryIncRef() { - return refCounted.tryIncRef(); - } - - @Override - public boolean decRef() { - if (refCounted.decRef()) { - hits.decRef(); - return true; - } - return false; - } - - @Override - public boolean hasReferences() { - return refCounted.hasReferences(); + public void close() { + hits.decRef(); } } diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchScrollAsyncAction.java b/server/src/main/java/org/elasticsearch/action/search/SearchScrollAsyncAction.java index 60e96a8cce8ab..2231f791384fa 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchScrollAsyncAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchScrollAsyncAction.java @@ -246,8 +246,7 @@ protected final void sendResponse( if (request.scroll() != null) { scrollId = request.scrollId(); } - var sections = SearchPhaseController.merge(true, queryPhase, fetchResults); - try { + try (var sections = SearchPhaseController.merge(true, queryPhase, fetchResults)) { ActionListener.respondAndRelease( listener, new SearchResponse( @@ -262,8 +261,6 @@ protected final void sendResponse( null ) ); - } finally { - sections.decRef(); } } catch (Exception e) { listener.onFailure(new ReduceSearchPhaseException("fetch", "inner finish failed", e, buildShardFailures())); diff --git a/server/src/test/java/org/elasticsearch/action/search/ExpandSearchPhaseTests.java b/server/src/test/java/org/elasticsearch/action/search/ExpandSearchPhaseTests.java index 23184be02f9c3..5fb70500d515f 100644 --- a/server/src/test/java/org/elasticsearch/action/search/ExpandSearchPhaseTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/ExpandSearchPhaseTests.java @@ -96,11 +96,10 @@ void sendExecuteMultiSearch(MultiSearchRequest request, SearchTask task, ActionL List mSearchResponses = new ArrayList<>(numInnerHits); for (int innerHitNum = 0; innerHitNum < numInnerHits; innerHitNum++) { - var sections = new SearchResponseSections(collapsedHits.get(innerHitNum), null, null, false, null, null, 1); - try { + try ( + var sections = new SearchResponseSections(collapsedHits.get(innerHitNum), null, null, false, null, null, 1) + ) { mockSearchPhaseContext.sendSearchResponse(sections, null); - } finally { - sections.decRef(); } mSearchResponses.add(new MultiSearchResponse.Item(mockSearchPhaseContext.searchResponse.get(), null)); // transferring ownership to the multi-search response so no need to release here @@ -121,11 +120,8 @@ void sendExecuteMultiSearch(MultiSearchRequest request, SearchTask task, ActionL ExpandSearchPhase phase = new ExpandSearchPhase(mockSearchPhaseContext, hits, () -> new SearchPhase("test") { @Override public void run() { - var sections = new SearchResponseSections(hits, null, null, false, null, null, 1); - try { + try (var sections = new SearchResponseSections(hits, null, null, false, null, null, 1)) { mockSearchPhaseContext.sendSearchResponse(sections, null); - } finally { - sections.decRef(); } } }); @@ -215,11 +211,8 @@ void sendExecuteMultiSearch(MultiSearchRequest request, SearchTask task, ActionL ExpandSearchPhase phase = new ExpandSearchPhase(mockSearchPhaseContext, hits, () -> new SearchPhase("test") { @Override public void run() { - var sections = new SearchResponseSections(hits, null, null, false, null, null, 1); - try { + try (var sections = new SearchResponseSections(hits, null, null, false, null, null, 1)) { mockSearchPhaseContext.sendSearchResponse(sections, null); - } finally { - sections.decRef(); } } }); @@ -254,11 +247,8 @@ void sendExecuteMultiSearch(MultiSearchRequest request, SearchTask task, ActionL ExpandSearchPhase phase = new ExpandSearchPhase(mockSearchPhaseContext, hits, () -> new SearchPhase("test") { @Override public void run() { - var sections = new SearchResponseSections(hits, null, null, false, null, null, 1); - try { + try (var sections = new SearchResponseSections(hits, null, null, false, null, null, 1)) { mockSearchPhaseContext.sendSearchResponse(sections, null); - } finally { - sections.decRef(); } } }); diff --git a/server/src/test/java/org/elasticsearch/action/search/FetchLookupFieldsPhaseTests.java b/server/src/test/java/org/elasticsearch/action/search/FetchLookupFieldsPhaseTests.java index 1d2daf0cd660e..5c508dce61fc3 100644 --- a/server/src/test/java/org/elasticsearch/action/search/FetchLookupFieldsPhaseTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/FetchLookupFieldsPhaseTests.java @@ -47,12 +47,10 @@ void sendExecuteMultiSearch(MultiSearchRequest request, SearchTask task, ActionL searchHits[i] = SearchHitTests.createTestItem(randomBoolean(), randomBoolean()); } SearchHits hits = new SearchHits(searchHits, new TotalHits(numHits, TotalHits.Relation.EQUAL_TO), 1.0f); - var sections = new SearchResponseSections(hits, null, null, false, null, null, 1); - try { + try (var sections = new SearchResponseSections(hits, null, null, false, null, null, 1)) { FetchLookupFieldsPhase phase = new FetchLookupFieldsPhase(searchPhaseContext, sections, null); phase.run(); } finally { - sections.decRef(); hits.decRef(); } searchPhaseContext.assertNoFailure(); @@ -189,12 +187,10 @@ void sendExecuteMultiSearch( new TotalHits(2, TotalHits.Relation.EQUAL_TO), 1.0f ); - var sections = new SearchResponseSections(searchHits, null, null, false, null, null, 1); - try { + try (var sections = new SearchResponseSections(searchHits, null, null, false, null, null, 1)) { FetchLookupFieldsPhase phase = new FetchLookupFieldsPhase(searchPhaseContext, sections, null); phase.run(); } finally { - sections.decRef(); searchHits.decRef(); } assertTrue(requestSent.get()); diff --git a/server/src/test/java/org/elasticsearch/action/search/SearchPhaseControllerTests.java b/server/src/test/java/org/elasticsearch/action/search/SearchPhaseControllerTests.java index 857402d1baaac..c95a155a790fd 100644 --- a/server/src/test/java/org/elasticsearch/action/search/SearchPhaseControllerTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/SearchPhaseControllerTests.java @@ -292,8 +292,7 @@ public void testMerge() { reducedQueryPhase.suggest(), profile ); - final SearchResponseSections mergedResponse = SearchPhaseController.merge(false, reducedQueryPhase, fetchResults); - try { + try (SearchResponseSections mergedResponse = SearchPhaseController.merge(false, reducedQueryPhase, fetchResults)) { if (trackTotalHits == SearchContext.TRACK_TOTAL_HITS_DISABLED) { assertNull(mergedResponse.hits.getTotalHits()); } else { @@ -346,7 +345,6 @@ public void testMerge() { assertThat(mergedResponse.profile(), is(anEmptyMap())); } } finally { - mergedResponse.decRef(); fetchResults.asList().forEach(TransportMessage::decRef); } } finally { @@ -410,8 +408,7 @@ protected boolean lessThan(RankDoc a, RankDoc b) { reducedQueryPhase.suggest(), false ); - SearchResponseSections mergedResponse = SearchPhaseController.merge(false, reducedQueryPhase, fetchResults); - try { + try (SearchResponseSections mergedResponse = SearchPhaseController.merge(false, reducedQueryPhase, fetchResults)) { if (trackTotalHits == SearchContext.TRACK_TOTAL_HITS_DISABLED) { assertNull(mergedResponse.hits.getTotalHits()); } else { @@ -427,7 +424,6 @@ protected boolean lessThan(RankDoc a, RankDoc b) { assertThat(mergedResponse.hits().getHits().length, equalTo(reducedQueryPhase.sortedTopDocs().scoreDocs().length)); assertThat(mergedResponse.profile(), is(anEmptyMap())); } finally { - mergedResponse.decRef(); fetchResults.asList().forEach(TransportMessage::decRef); } } finally {