Remove TopDocsCollectorContext#shouldRescore method (elastic#94895)

QueryPhase#executeInternal returns a boolean that tells whether we
should rescore. That one is returned by searchWithCollector which in
turn is returned by TopDocsCollectorContext#shouldRescore which returns
false by default and the only subclass that overrides it returns whether
the search context had a rescorer configured. 

We can simplify this by pulling out the logic that we use to make the decision on whether to execute the rescore phase or not, and remove the shouldRescore method and return value that gets carried around which adds complexity.

Author: javanna

server/src/main/java/org/elasticsearch/search/query/QueryPhase.java

@@ -77,11 +77,9 @@ public static void execute(SearchContext searchContext) throws QueryPhaseExecuti
         // request, preProcess is called on the DFS phase, this is why we pre-process them
         // here to make sure it happens during the QUERY phase
         AggregationPhase.preProcess(searchContext);
-        boolean rescore = executeInternal(searchContext);
+        executeInternal(searchContext);
 
-        if (rescore) { // only if we do a regular search
-            RescorePhase.execute(searchContext);
-        }
+        RescorePhase.execute(searchContext);
         SuggestPhase.execute(searchContext);
         AggregationPhase.execute(searchContext);
 
@@ -93,9 +91,8 @@ public static void execute(SearchContext searchContext) throws QueryPhaseExecuti
     /**
      * In a package-private method so that it can be tested without having to
      * wire everything (mapperService, etc.)
-     * @return whether the rescoring phase should be executed
      */
-    static boolean executeInternal(SearchContext searchContext) throws QueryPhaseExecutionException {
+    static void executeInternal(SearchContext searchContext) throws QueryPhaseExecutionException {
         final ContextIndexSearcher searcher = searchContext.searcher();
         final IndexReader reader = searcher.getIndexReader();
         QuerySearchResult queryResult = searchContext.queryResult();
@@ -176,7 +173,7 @@ static boolean executeInternal(SearchContext searchContext) throws QueryPhaseExe
             }
 
             try {
-                boolean shouldRescore = searchWithCollector(searchContext, searcher, query, collectors, hasFilterCollector, timeoutSet);
+                searchWithCollector(searchContext, searcher, query, collectors, hasFilterCollector, timeoutSet);
                 ExecutorService executor = searchContext.indexShard().getThreadPool().executor(ThreadPool.Names.SEARCH);
                 assert executor instanceof EWMATrackingEsThreadPoolExecutor
                     || (executor instanceof EsThreadPoolExecutor == false /* in case thread pool is mocked out in tests */)
@@ -185,7 +182,6 @@ static boolean executeInternal(SearchContext searchContext) throws QueryPhaseExe
                     queryResult.nodeQueueSize(rExecutor.getCurrentQueueSize());
                     queryResult.serviceTimeEWMA((long) rExecutor.getTaskExecutionEWMA());
                 }
-                return shouldRescore;
             } finally {
                 // Search phase has finished, no longer need to check for timeout
                 // otherwise aggregation phase might get cancelled.
@@ -198,7 +194,7 @@ static boolean executeInternal(SearchContext searchContext) throws QueryPhaseExe
         }
     }
 
-    private static boolean searchWithCollector(
+    private static void searchWithCollector(
         SearchContext searchContext,
         ContextIndexSearcher searcher,
         Query query,
@@ -238,7 +234,6 @@ private static boolean searchWithCollector(
         for (QueryCollectorContext ctx : collectors) {
             ctx.postProcess(queryResult);
         }
-        return topDocsFactory.shouldRescore();
     }
 
     /**

server/src/main/java/org/elasticsearch/search/query/TopDocsCollectorContext.java

@@ -74,20 +74,6 @@ abstract class TopDocsCollectorContext extends QueryCollectorContext {
         this.numHits = numHits;
     }
 
-    /**
-     * Returns the number of top docs to retrieve
-     */
-    final int numHits() {
-        return numHits;
-    }
-
-    /**
-     * Returns true if the top docs should be re-scored after initial search
-     */
-    boolean shouldRescore() {
-        return false;
-    }
-
     static class EmptyTopDocsCollectorContext extends TopDocsCollectorContext {
         private final Sort sort;
         private final Collector collector;
@@ -188,7 +174,7 @@ void postProcess(QuerySearchResult result) throws IOException {
         }
     }
 
-    abstract static class SimpleTopDocsCollectorContext extends TopDocsCollectorContext {
+    static class SimpleTopDocsCollectorContext extends TopDocsCollectorContext {
 
         private static TopDocsCollector<?> createCollector(
             @Nullable SortAndFormats sortAndFormats,
@@ -479,12 +465,7 @@ static TopDocsCollectorContext createTopDocsCollectorContext(SearchContext searc
                 searchContext.trackScores(),
                 searchContext.trackTotalHitsUpTo(),
                 hasFilterCollector
-            ) {
-                @Override
-                boolean shouldRescore() {
-                    return rescore;
-                }
-            };
+            );
         }
     }
 

server/src/main/java/org/elasticsearch/search/rescore/RescorePhase.java

@@ -22,6 +22,10 @@
 public class RescorePhase {
 
     public static void execute(SearchContext context) {
+        if (context.size() == 0 || context.collapse() != null || context.rescore() == null || context.rescore().isEmpty()) {
+            return;
+        }
+
         TopDocs topDocs = context.queryResult().topDocs().topDocs;
         if (topDocs.scoreDocs.length == 0) {
             return;

server/src/test/java/org/elasticsearch/search/query/QueryPhaseTests.java

@@ -116,8 +116,7 @@ private void countTestCase(Query query, IndexReader reader, boolean shouldCollec
         context.parsedQuery(new ParsedQuery(query));
         context.setSize(0);
         context.setTask(new SearchShardTask(123L, "", "", "", null, Collections.emptyMap()));
-        final boolean rescore = QueryPhase.executeInternal(context);
-        assertFalse(rescore);
+        QueryPhase.executeInternal(context);
 
         ContextIndexSearcher countSearcher = shouldCollectCount
             ? newContextSearcher(reader)