diff --git a/docs/changelog/125916.yaml b/docs/changelog/125916.yaml new file mode 100644 index 0000000000000..57741e4f0870a --- /dev/null +++ b/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: [] diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TopHitsAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TopHitsAggregationBuilder.java index b8bd95dcdf9e9..ac37b287736aa 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TopHitsAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TopHitsAggregationBuilder.java @@ -49,7 +49,6 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; -import java.util.function.ToLongFunction; public class TopHitsAggregationBuilder extends AbstractAggregationBuilder { public static final String NAME = "top_hits"; @@ -823,18 +822,4 @@ public String getType() { public TransportVersion getMinimalSupportedVersion() { return TransportVersions.ZERO; } - - @Override - public boolean supportsParallelCollection(ToLongFunction 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); - } } diff --git a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java index cd597f3328c0f..f1ca48b7d3dda 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java @@ -749,4 +749,11 @@ public FieldSortBuilder rewrite(QueryRewriteContext ctx) throws IOException { } 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; + } } diff --git a/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java index 9bd5d4b1a23fa..ab96638efe626 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java @@ -746,4 +746,11 @@ public GeoDistanceSortBuilder rewrite(QueryRewriteContext ctx) throws IOExceptio } 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; + } } diff --git a/server/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.java index a6fd4ef90693d..0c2fc68e7e856 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.java @@ -173,9 +173,4 @@ public TransportVersion getMinimalSupportedVersion() { public ScoreSortBuilder rewrite(QueryRewriteContext ctx) { return this; } - - @Override - public boolean supportsParallelCollection() { - return true; - } } diff --git a/server/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java index a1920d3ead600..c5ebf97183601 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java @@ -511,9 +511,4 @@ public ScriptSortBuilder rewrite(QueryRewriteContext ctx) throws IOException { } return new ScriptSortBuilder(this).setNestedSort(rewrite); } - - @Override - public boolean supportsParallelCollection() { - return true; - } } diff --git a/server/src/main/java/org/elasticsearch/search/sort/SortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/SortBuilder.java index 7ed3abe7daa1a..17da9abd3952b 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/SortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/SortBuilder.java @@ -290,6 +290,6 @@ public String toString() { } public boolean supportsParallelCollection() { - return false; + return true; } } diff --git a/server/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java b/server/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java index a2c9119d0d5cf..9fef56ba16396 100644 --- a/server/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java @@ -1030,7 +1030,7 @@ public void testSupportsParallelCollection() { { 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();