Make use of constants and utility methods to build empty SearchHits i…

…nstances (#103983) This are to be made ref-counted shortly. There's no point in having any pooling/leak-tracking for empty instances though. To prepare for that, lets add some short-cuts for dealing with empty instances to make the overall change smaller and cleanup code already.
elastic · Jan 5, 2024 · 36f08ea · 36f08ea
1 parent 91ca22c
commit 36f08ea
Show file tree

Hide file tree

Showing 39 changed files with 92 additions and 127 deletions.
diff --git a/.../src/main/java/org/elasticsearch/plugin/noop/action/search/TransportNoopSearchAction.java b/.../src/main/java/org/elasticsearch/plugin/noop/action/search/TransportNoopSearchAction.java
@@ -7,7 +7,6 @@
  */
 package org.elasticsearch.plugin.noop.action.search;
 
-import org.apache.lucene.search.TotalHits;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.search.SearchRequest;
 import org.elasticsearch.action.search.SearchResponse;
@@ -18,7 +17,6 @@
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.util.concurrent.EsExecutors;
 import org.elasticsearch.plugin.noop.NoopPlugin;
-import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.search.SearchHits;
 import org.elasticsearch.search.aggregations.InternalAggregations;
 import org.elasticsearch.search.profile.SearchProfileResults;
@@ -44,7 +42,7 @@ public TransportNoopSearchAction(TransportService transportService, ActionFilter
     protected void doExecute(Task task, SearchRequest request, ActionListener<SearchResponse> listener) {
         listener.onResponse(
             new SearchResponse(
-                new SearchHits(new SearchHit[0], new TotalHits(0L, TotalHits.Relation.EQUAL_TO), 0.0f),
+                SearchHits.EMPTY_WITH_TOTAL_HITS,
                 InternalAggregations.EMPTY,
                 new Suggest(Collections.emptyList()),
                 false,

diff --git a/...nk-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainTests.java b/...nk-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainTests.java
@@ -13,6 +13,7 @@
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.index.shard.ShardId;
 import org.elasticsearch.search.SearchHit;
+import org.elasticsearch.search.SearchHits;
 import org.elasticsearch.search.SearchShardTarget;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xcontent.ToXContent;
@@ -208,15 +209,14 @@ public void testNoResults() throws Exception {
                 }
             }
         }
-        SearchHit[] hits = new SearchHit[0];
         DiscountedCumulativeGain dcg = new DiscountedCumulativeGain();
-        EvalQueryQuality result = dcg.evaluate("id", hits, ratedDocs);
+        EvalQueryQuality result = dcg.evaluate("id", SearchHits.EMPTY, ratedDocs);
         assertEquals(0.0d, result.metricScore(), DELTA);
         assertEquals(0, filterUnratedDocuments(result.getHitsAndRatings()).size());
 
         // also check normalized
         dcg = new DiscountedCumulativeGain(true, null, 10);
-        result = dcg.evaluate("id", hits, ratedDocs);
+        result = dcg.evaluate("id", SearchHits.EMPTY, ratedDocs);
         assertEquals(0.0d, result.metricScore(), DELTA);
         assertEquals(0, filterUnratedDocuments(result.getHitsAndRatings()).size());
     }

diff --git a/...rank-eval/src/test/java/org/elasticsearch/index/rankeval/ExpectedReciprocalRankTests.java b/...rank-eval/src/test/java/org/elasticsearch/index/rankeval/ExpectedReciprocalRankTests.java
@@ -12,6 +12,7 @@
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.index.shard.ShardId;
 import org.elasticsearch.search.SearchHit;
+import org.elasticsearch.search.SearchHits;
 import org.elasticsearch.search.SearchShardTarget;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xcontent.ToXContent;
@@ -114,7 +115,7 @@ private SearchHit[] createSearchHits(List<RatedDocument> rated, Integer[] releva
      */
     public void testNoResults() throws Exception {
         ExpectedReciprocalRank err = new ExpectedReciprocalRank(5, 0, 10);
-        assertEquals(0.0, err.evaluate("id", new SearchHit[0], Collections.emptyList()).metricScore(), DELTA);
+        assertEquals(0.0, err.evaluate("id", SearchHits.EMPTY, Collections.emptyList()).metricScore(), DELTA);
     }
 
     public void testParseFromXContent() throws IOException {

diff --git a/...les/rank-eval/src/test/java/org/elasticsearch/index/rankeval/MeanReciprocalRankTests.java b/...les/rank-eval/src/test/java/org/elasticsearch/index/rankeval/MeanReciprocalRankTests.java
@@ -12,6 +12,7 @@
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.index.shard.ShardId;
 import org.elasticsearch.search.SearchHit;
+import org.elasticsearch.search.SearchHits;
 import org.elasticsearch.search.SearchShardTarget;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xcontent.ToXContent;
@@ -150,8 +151,7 @@ public void testEvaluationNoRelevantInResults() {
     }
 
     public void testNoResults() throws Exception {
-        SearchHit[] hits = new SearchHit[0];
-        EvalQueryQuality evaluated = (new MeanReciprocalRank()).evaluate("id", hits, Collections.emptyList());
+        EvalQueryQuality evaluated = (new MeanReciprocalRank()).evaluate("id", SearchHits.EMPTY, Collections.emptyList());
         assertEquals(0.0d, evaluated.metricScore(), 0.00001);
         assertEquals(-1, ((MeanReciprocalRank.Detail) evaluated.getMetricDetails()).getFirstRelevantRank());
     }

diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtKTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtKTests.java
@@ -12,6 +12,7 @@
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.index.shard.ShardId;
 import org.elasticsearch.search.SearchHit;
+import org.elasticsearch.search.SearchHits;
 import org.elasticsearch.search.SearchShardTarget;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xcontent.ToXContent;
@@ -136,8 +137,7 @@ public void testNoRatedDocs() throws Exception {
     }
 
     public void testNoResults() throws Exception {
-        SearchHit[] hits = new SearchHit[0];
-        EvalQueryQuality evaluated = (new PrecisionAtK()).evaluate("id", hits, Collections.emptyList());
+        EvalQueryQuality evaluated = (new PrecisionAtK()).evaluate("id", SearchHits.EMPTY, Collections.emptyList());
         assertEquals(0.0d, evaluated.metricScore(), 0.00001);
         assertEquals(0, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRelevantRetrieved());
         assertEquals(0, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRetrieved());

diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RecallAtKTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RecallAtKTests.java
@@ -12,6 +12,7 @@
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.index.shard.ShardId;
 import org.elasticsearch.search.SearchHit;
+import org.elasticsearch.search.SearchHits;
 import org.elasticsearch.search.SearchShardTarget;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xcontent.ToXContent;
@@ -113,7 +114,7 @@ public void testNoRatedDocs() throws Exception {
     }
 
     public void testNoResults() throws Exception {
-        EvalQueryQuality evaluated = (new RecallAtK()).evaluate("id", new SearchHit[0], Collections.emptyList());
+        EvalQueryQuality evaluated = (new RecallAtK()).evaluate("id", SearchHits.EMPTY, Collections.emptyList());
         assertEquals(0.0d, evaluated.metricScore(), 0.00001);
         assertEquals(0, ((RecallAtK.Detail) evaluated.getMetricDetails()).getRelevantRetrieved());
         assertEquals(0, ((RecallAtK.Detail) evaluated.getMetricDetails()).getRelevant());
@@ -123,7 +124,7 @@ public void testNoResultsWithRatedDocs() throws Exception {
         List<RatedDocument> rated = new ArrayList<>();
         rated.add(createRatedDoc("test", "0", RELEVANT_RATING));
 
-        EvalQueryQuality evaluated = (new RecallAtK()).evaluate("id", new SearchHit[0], rated);
+        EvalQueryQuality evaluated = (new RecallAtK()).evaluate("id", SearchHits.EMPTY, rated);
         assertEquals(0.0d, evaluated.metricScore(), 0.00001);
         assertEquals(0, ((RecallAtK.Detail) evaluated.getMetricDetails()).getRelevantRetrieved());
         assertEquals(1, ((RecallAtK.Detail) evaluated.getMetricDetails()).getRelevant());

diff --git a/...rc/javaRestTest/java/org/elasticsearch/search/CrossClusterSearchUnavailableClusterIT.java b/...rc/javaRestTest/java/org/elasticsearch/search/CrossClusterSearchUnavailableClusterIT.java
@@ -93,7 +93,7 @@ private static MockTransportService startTransport(
                 SearchRequest::new,
                 (request, channel, task) -> channel.sendResponse(
                     new SearchResponse(
-                        new SearchHits(new SearchHit[0], new TotalHits(0, TotalHits.Relation.EQUAL_TO), Float.NaN),
+                        SearchHits.empty(new TotalHits(0, TotalHits.Relation.EQUAL_TO), Float.NaN),
                         InternalAggregations.EMPTY,
                         null,
                         false,

diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchResponse.java b/server/src/main/java/org/elasticsearch/action/search/SearchResponse.java
@@ -24,7 +24,6 @@
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.rest.action.RestActions;
-import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.search.SearchHits;
 import org.elasticsearch.search.aggregations.Aggregations;
 import org.elasticsearch.search.aggregations.InternalAggregations;
@@ -1368,9 +1367,8 @@ public String toString() {
 
     // public for tests
     public static SearchResponse empty(Supplier<Long> tookInMillisSupplier, Clusters clusters) {
-        SearchHits searchHits = new SearchHits(new SearchHit[0], new TotalHits(0L, TotalHits.Relation.EQUAL_TO), Float.NaN);
         return new SearchResponse(
-            searchHits,
+            SearchHits.empty(new TotalHits(0L, TotalHits.Relation.EQUAL_TO), Float.NaN),
             InternalAggregations.EMPTY,
             null,
             false,

diff --git a/server/src/main/java/org/elasticsearch/search/SearchHits.java b/server/src/main/java/org/elasticsearch/search/SearchHits.java
@@ -35,8 +35,8 @@
 public final class SearchHits implements Writeable, ChunkedToXContent, Iterable<SearchHit> {
 
     public static final SearchHit[] EMPTY = new SearchHit[0];
-    public static final SearchHits EMPTY_WITH_TOTAL_HITS = new SearchHits(EMPTY, new TotalHits(0, Relation.EQUAL_TO), 0);
-    public static final SearchHits EMPTY_WITHOUT_TOTAL_HITS = new SearchHits(EMPTY, null, 0);
+    public static final SearchHits EMPTY_WITH_TOTAL_HITS = SearchHits.empty(new TotalHits(0, Relation.EQUAL_TO), 0);
+    public static final SearchHits EMPTY_WITHOUT_TOTAL_HITS = SearchHits.empty(null, 0);
 
     private final SearchHit[] hits;
     private final TotalHits totalHits;
@@ -48,6 +48,10 @@ public final class SearchHits implements Writeable, ChunkedToXContent, Iterable<
     @Nullable
     private final Object[] collapseValues;
 
+    public static SearchHits empty(@Nullable TotalHits totalHits, float maxScore) {
+        return new SearchHits(EMPTY, totalHits, maxScore);
+    }
+
     public SearchHits(SearchHit[] hits, @Nullable TotalHits totalHits, float maxScore) {
         this(hits, totalHits, maxScore, null, null, null);
     }
@@ -235,7 +239,7 @@ public static SearchHits fromXContent(XContentParser parser) throws IOException
                 }
             }
         }
-        return new SearchHits(hits.toArray(new SearchHit[0]), totalHits, maxScore);
+        return new SearchHits(hits.toArray(SearchHits.EMPTY), totalHits, maxScore);
     }
 
     @Override

diff --git a/server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java b/server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java
@@ -67,8 +67,8 @@ public void execute(SearchContext context, int[] docIdsToLoad) {
 
         if (docIdsToLoad == null || docIdsToLoad.length == 0) {
             // no individual hits to process, so we shortcut
-            SearchHits hits = new SearchHits(new SearchHit[0], context.queryResult().getTotalHits(), context.queryResult().getMaxScore());
-            context.fetchResult().shardResult(hits, null);
+            context.fetchResult()
+                .shardResult(SearchHits.empty(context.queryResult().getTotalHits(), context.queryResult().getMaxScore()), null);
             return;
         }
 

diff --git a/server/src/test/java/org/elasticsearch/action/search/ExpandSearchPhaseTests.java b/server/src/test/java/org/elasticsearch/action/search/ExpandSearchPhaseTests.java
@@ -232,7 +232,7 @@ void sendExecuteMultiSearch(MultiSearchRequest request, SearchTask task, ActionL
                 )
             );
 
-        SearchHits hits = new SearchHits(new SearchHit[0], new TotalHits(1, TotalHits.Relation.EQUAL_TO), 1.0f);
+        SearchHits hits = SearchHits.empty(new TotalHits(1, TotalHits.Relation.EQUAL_TO), 1.0f);
         ExpandSearchPhase phase = new ExpandSearchPhase(mockSearchPhaseContext, hits, () -> new SearchPhase("test") {
             @Override
             public void run() {
@@ -274,7 +274,7 @@ void sendExecuteMultiSearch(MultiSearchRequest request, SearchTask task, ActionL
             .preference("foobar")
             .routing("baz");
 
-        SearchHits hits = new SearchHits(new SearchHit[0], new TotalHits(1, TotalHits.Relation.EQUAL_TO), 1.0f);
+        SearchHits hits = SearchHits.empty(new TotalHits(1, TotalHits.Relation.EQUAL_TO), 1.0f);
         ExpandSearchPhase phase = new ExpandSearchPhase(mockSearchPhaseContext, hits, () -> new SearchPhase("test") {
             @Override
             public void run() {

diff --git a/server/src/test/java/org/elasticsearch/action/search/FetchLookupFieldsPhaseTests.java b/server/src/test/java/org/elasticsearch/action/search/FetchLookupFieldsPhaseTests.java
@@ -95,7 +95,7 @@ void sendExecuteMultiSearch(
                         fields.forEach((f, values) -> hit.setDocumentField(f, new DocumentField(f, values, List.of())));
                         searchHits = new SearchHits(new SearchHit[] { hit }, new TotalHits(1, TotalHits.Relation.EQUAL_TO), 1.0f);
                     } else {
-                        searchHits = new SearchHits(new SearchHit[0], new TotalHits(0, TotalHits.Relation.EQUAL_TO), 1.0f);
+                        searchHits = SearchHits.empty(new TotalHits(0, TotalHits.Relation.EQUAL_TO), 1.0f);
                     }
                     responses[i] = new MultiSearchResponse.Item(
                         new SearchResponse(

diff --git a/server/src/test/java/org/elasticsearch/action/search/SearchPhaseControllerTests.java b/server/src/test/java/org/elasticsearch/action/search/SearchPhaseControllerTests.java
@@ -577,7 +577,7 @@ private static AtomicArray<SearchPhaseResult> generateFetchResults(
                     }
                 }
             }
-            SearchHit[] hits = searchHits.toArray(new SearchHit[0]);
+            SearchHit[] hits = searchHits.toArray(SearchHits.EMPTY);
             ProfileResult profileResult = profile && searchHits.size() > 0
                 ? new ProfileResult("fetch", "fetch", Map.of(), Map.of(), randomNonNegativeLong(), List.of())
                 : null;

diff --git a/server/src/test/java/org/elasticsearch/action/search/SearchResponseMergerTests.java b/server/src/test/java/org/elasticsearch/action/search/SearchResponseMergerTests.java
@@ -330,9 +330,8 @@ public void testMergeProfileResults() throws InterruptedException {
             for (int i = 0; i < numResponses; i++) {
                 SearchProfileResults profile = SearchProfileResultsTests.createTestItem();
                 expectedProfile.putAll(profile.getShardResults());
-                SearchHits searchHits = new SearchHits(new SearchHit[0], new TotalHits(0, TotalHits.Relation.EQUAL_TO), Float.NaN);
                 SearchResponse searchResponse = new SearchResponse(
-                    searchHits,
+                    SearchHits.empty(new TotalHits(0, TotalHits.Relation.EQUAL_TO), Float.NaN),
                     null,
                     null,
                     false,
@@ -408,7 +407,7 @@ public void testMergeCompletionSuggestions() throws InterruptedException {
                 completionSuggestion.addTerm(options);
                 suggestions.add(completionSuggestion);
                 Suggest suggest = new Suggest(suggestions);
-                SearchHits searchHits = new SearchHits(new SearchHit[0], null, Float.NaN);
+                SearchHits searchHits = SearchHits.empty(null, Float.NaN);
                 SearchResponse searchResponse = new SearchResponse(
                     searchHits,
                     null,
@@ -494,9 +493,8 @@ public void testMergeCompletionSuggestionsTieBreak() throws InterruptedException
                 completionSuggestion.addTerm(options);
                 suggestions.add(completionSuggestion);
                 Suggest suggest = new Suggest(suggestions);
-                SearchHits searchHits = new SearchHits(new SearchHit[0], null, Float.NaN);
                 SearchResponse searchResponse = new SearchResponse(
-                    searchHits,
+                    SearchHits.empty(null, Float.NaN),
                     null,
                     suggest,
                     false,
@@ -565,7 +563,6 @@ public void testMergeEmptyFormat() throws InterruptedException {
             Collections.emptyMap()
         );
 
-        SearchHits searchHits = new SearchHits(new SearchHit[0], null, Float.NaN);
         try (
             SearchResponseMerger searchResponseMerger = new SearchResponseMerger(
                 0,
@@ -578,7 +575,7 @@ public void testMergeEmptyFormat() throws InterruptedException {
             for (Max max : Arrays.asList(max1, max2)) {
                 InternalAggregations aggs = InternalAggregations.from(Arrays.asList(max));
                 SearchResponse searchResponse = new SearchResponse(
-                    searchHits,
+                    SearchHits.empty(null, Float.NaN),
                     aggs,
                     null,
                     false,
@@ -645,9 +642,8 @@ public void testMergeAggs() throws InterruptedException {
                 );
                 InternalDateRange range = factory.create(rangeAggName, singletonList(bucket), DocValueFormat.RAW, false, emptyMap());
                 InternalAggregations aggs = InternalAggregations.from(Arrays.asList(range, max));
-                SearchHits searchHits = new SearchHits(new SearchHit[0], null, Float.NaN);
                 SearchResponse searchResponse = new SearchResponse(
-                    searchHits,
+                    SearchHits.empty(null, Float.NaN),
                     aggs,
                     null,
                     false,
@@ -977,16 +973,8 @@ public void testMergeEmptySearchHitsWithNonEmpty() {
                 }
             }
             {
-                SearchHits empty = new SearchHits(
-                    new SearchHit[0],
-                    new TotalHits(0, TotalHits.Relation.EQUAL_TO),
-                    Float.NaN,
-                    null,
-                    null,
-                    null
-                );
                 SearchResponse searchResponse = new SearchResponse(
-                    empty,
+                    SearchHits.empty(new TotalHits(0, TotalHits.Relation.EQUAL_TO), Float.NaN),
                     null,
                     null,
                     false,
@@ -1041,9 +1029,8 @@ public void testMergeOnlyEmptyHits() {
                     long previousValue = expectedTotalHits == null ? 0 : expectedTotalHits.value;
                     expectedTotalHits = new TotalHits(Math.min(previousValue + totalHits.value, trackTotalHitsUpTo), totalHitsRelation);
                 }
-                SearchHits empty = new SearchHits(new SearchHit[0], totalHits, Float.NaN, null, null, null);
                 SearchResponse searchResponse = new SearchResponse(
-                    empty,
+                    SearchHits.empty(totalHits, Float.NaN),
                     null,
                     null,
                     false,

diff --git a/server/src/test/java/org/elasticsearch/action/search/TransportSearchActionTests.java b/server/src/test/java/org/elasticsearch/action/search/TransportSearchActionTests.java
@@ -61,7 +61,6 @@
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.search.DummyQueryBuilder;
 import org.elasticsearch.search.Scroll;
-import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.search.SearchHits;
 import org.elasticsearch.search.SearchService;
 import org.elasticsearch.search.SearchShardTarget;
@@ -479,26 +478,6 @@ private MockTransportService[] startTransport(
         return mockTransportServices;
     }
 
-    private static SearchResponse emptySearchResponse() {
-        return new SearchResponse(
-            new SearchHits(new SearchHit[0], new TotalHits(0, TotalHits.Relation.EQUAL_TO), Float.NaN),
-            InternalAggregations.EMPTY,
-            null,
-            false,
-            null,
-            null,
-            1,
-            null,
-            1,
-            1,
-            0,
-            100,
-            ShardSearchFailure.EMPTY_ARRAY,
-            SearchResponse.Clusters.EMPTY,
-            null
-        );
-    }
-
     public void testCCSRemoteReduceMergeFails() throws Exception {
         int numClusters = randomIntBetween(2, 10);
         DiscoveryNode[] nodes = new DiscoveryNode[numClusters];
@@ -876,12 +855,26 @@ public void onNodeDisconnected(DiscoveryNode node, Transport.Connection connecti
     }
 
     private static void resolveWithEmptySearchResponse(Tuple<SearchRequest, ActionListener<SearchResponse>> tuple) {
-        var resp = emptySearchResponse();
-        try {
-            tuple.v2().onResponse(resp);
-        } finally {
-            resp.decRef();
-        }
+        ActionListener.respondAndRelease(
+            tuple.v2(),
+            new SearchResponse(
+                SearchHits.empty(new TotalHits(0, TotalHits.Relation.EQUAL_TO), Float.NaN),
+                InternalAggregations.EMPTY,
+                null,
+                false,
+                null,
+                null,
+                1,
+                null,
+                1,
+                1,
+                0,
+                100,
+                ShardSearchFailure.EMPTY_ARRAY,
+                SearchResponse.Clusters.EMPTY,
+                null
+            )
+        );
     }
 
     public void testCollectSearchShards() throws Exception {