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 9f6f98a4c8fb0..2ec30b411928a 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 5691435c83ecb..1bf6a47808041 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java @@ -741,4 +741,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 0bd8cc44edb30..61d20fa72e262 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java @@ -721,4 +721,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 e60e534d6acaa..0977b38585052 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.java @@ -172,9 +172,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 b3c88be60c179..bd353d0af6c2c 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java @@ -502,9 +502,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 4a8cdbcdffa55..8fb2e102113c2 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/SortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/SortBuilder.java @@ -285,6 +285,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 95217b6a005c4..83f19924ec3d6 100644 --- a/server/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java @@ -1007,7 +1007,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();