Skip to content

Commit

Permalink
Revert "Remove shortcutTotalHitCount optimization (#89047)" (#94159)
Browse files Browse the repository at this point in the history
This reverts commit 283f8ac in the 8.7 branch (#89047).

We have found a performance regression around executing search requests with size greater than zero that hold queries that can shortcut their total hit count, like term and match_all. The previous shortcut total hit count optimization done in ES was able to shortcut those while the top score docs collector in Lucene does not support that. This can be improved further on main but for 8.7 we are going the safe path of reverting and leaving things how they were.
  • Loading branch information
javanna committed Feb 27, 2023
1 parent 74cd2b2 commit 75da304
Show file tree
Hide file tree
Showing 4 changed files with 304 additions and 79 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -129,15 +129,21 @@ static boolean executeInternal(SearchContext searchContext) throws QueryPhaseExe
}

final LinkedList<QueryCollectorContext> collectors = new LinkedList<>();
// whether the chain contains a collector that filters documents
boolean hasFilterCollector = false;
if (searchContext.terminateAfter() != SearchContext.DEFAULT_TERMINATE_AFTER) {
// add terminate_after before the filter collectors
// it will only be applied on documents accepted by these filter collectors
collectors.add(createEarlyTerminationCollectorContext(searchContext.terminateAfter()));
// this collector can filter documents during the collection
hasFilterCollector = true;
}
if (searchContext.parsedPostFilter() != null) {
// add post filters before aggregations
// it will only be applied to top hits
collectors.add(createFilteredCollectorContext(searcher, searchContext.parsedPostFilter().query()));
// this collector can filter documents during the collection
hasFilterCollector = true;
}
if (searchContext.queryCollectors().isEmpty() == false) {
// plug in additional collectors, like aggregations
Expand All @@ -146,6 +152,8 @@ static boolean executeInternal(SearchContext searchContext) throws QueryPhaseExe
if (searchContext.minimumScore() != null) {
// apply the minimum score after multi collector so we filter aggs as well
collectors.add(createMinScoreCollectorContext(searchContext.minimumScore()));
// this collector can filter documents during the collection
hasFilterCollector = true;
}

boolean timeoutSet = scrollContext == null
Expand All @@ -168,7 +176,7 @@ static boolean executeInternal(SearchContext searchContext) throws QueryPhaseExe
}

try {
boolean shouldRescore = searchWithCollector(searchContext, searcher, query, collectors, timeoutSet);
boolean shouldRescore = 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 */)
Expand All @@ -195,10 +203,11 @@ private static boolean searchWithCollector(
ContextIndexSearcher searcher,
Query query,
LinkedList<QueryCollectorContext> collectors,
boolean hasFilterCollector,
boolean timeoutSet
) throws IOException {
// create the top docs collector last when the other collectors are known
final TopDocsCollectorContext topDocsFactory = createTopDocsCollectorContext(searchContext);
final TopDocsCollectorContext topDocsFactory = createTopDocsCollectorContext(searchContext, hasFilterCollector);
// add the top docs collector, the first collector context in the chain
collectors.addFirst(topDocsFactory);

Expand Down

0 comments on commit 75da304

Please sign in to comment.