Re-enable parallel collection for field sorted top hits (#125916)

With #123610 we disabled parallel collection for field and script sorted top hits,
aligning its behaviour with that of top level search. This was mainly to work around
a bug in script sorting that did not support inter-segment concurrency.

The bug with script sort has been fixed with #123757 and concurrency re-enabled for it.

While sort by field is not optimized for search concurrency, top hits benefits from it
and disabling concurrency for sort by field in top hits has caused performance
regressions in our nightly benchmarks.

This commit re-enables concurrency for top hits with sort by field is used. This
introduces back a discrepancy between top level search and top hits, in that concurrency
is applied for top hits despite sort by field normally disables it. The key difference
is the context where sorting is applied, and the fact that concurrency is disabled
only for performance reasons on top level searches and not for functional reasons.

docs/changelog/125916.yaml

@@ -0,0 +1,5 @@
+pr: 125916
+summary: Re-enable parallel collection for field sorted top hits
+area: Search
+type: bug
+issues: []

server/src/main/java/org/elasticsearch/search/aggregations/metrics/TopHitsAggregationBuilder.java

@@ -49,7 +49,6 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
-import java.util.function.ToLongFunction;
 
 public class TopHitsAggregationBuilder extends AbstractAggregationBuilder<TopHitsAggregationBuilder> {
     public static final String NAME = "top_hits";
@@ -823,18 +822,4 @@ public class TopHitsAggregationBuilder extends AbstractAggregationBuilder<TopHit
     public TransportVersion getMinimalSupportedVersion() {
         return TransportVersions.ZERO;
     }
-
-    @Override
-    public boolean supportsParallelCollection(ToLongFunction<String> fieldCardinalityResolver) {
-        if (sorts != null) {
-            // the implicit sorting is by _score, which supports parallel collection
-            for (SortBuilder<?> sortBuilder : sorts) {
-                if (sortBuilder.supportsParallelCollection() == false) {
-                    return false;
-                }
-            }
-        }
-
-        return super.supportsParallelCollection(fieldCardinalityResolver);
-    }
 }

server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java

@@ -741,4 +741,11 @@ public final class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
         }
         return new FieldSortBuilder(this).setNestedSort(rewrite);
     }
+
+    @Override
+    public boolean supportsParallelCollection() {
+        // Disable parallel collection for sort by field.
+        // It is supported but not optimized on the Lucene side to share info across collectors, and can cause regressions.
+        return false;
+    }
 }

server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java

@@ -721,4 +721,11 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
         }
         return new GeoDistanceSortBuilder(this).setNestedSort(rewrite);
     }
+
+    @Override
+    public boolean supportsParallelCollection() {
+        // Disable parallel collection for sort by field.
+        // It is supported but not optimized on the Lucene side to share info across collectors, and can cause regressions.
+        return false;
+    }
 }

server/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.java

@@ -172,9 +172,4 @@ public final class ScoreSortBuilder extends SortBuilder<ScoreSortBuilder> {
     public ScoreSortBuilder rewrite(QueryRewriteContext ctx) {
         return this;
     }
-
-    @Override
-    public boolean supportsParallelCollection() {
-        return true;
-    }
 }

server/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java

@@ -502,9 +502,4 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> {
         }
         return new ScriptSortBuilder(this).setNestedSort(rewrite);
     }
-
-    @Override
-    public boolean supportsParallelCollection() {
-        return true;
-    }
 }

server/src/main/java/org/elasticsearch/search/sort/SortBuilder.java

@@ -285,6 +285,6 @@ public abstract class SortBuilder<T extends SortBuilder<T>>
     }
 
     public boolean supportsParallelCollection() {
-        return false;
+        return true;
     }
 }

server/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java

@@ -1007,7 +1007,7 @@ public class SearchSourceBuilderTests extends AbstractSearchTestCase {
         {
             SearchSourceBuilder searchSourceBuilder = newSearchSourceBuilder.get();
             searchSourceBuilder.aggregation(new TopHitsAggregationBuilder("tophits").sort(SortBuilders.fieldSort("field")));
-            assertFalse(searchSourceBuilder.supportsParallelCollection(fieldCardinality));
+            assertTrue(searchSourceBuilder.supportsParallelCollection(fieldCardinality));
         }
         {
             SearchSourceBuilder searchSourceBuilder = newSearchSourceBuilder.get();