From 40d7ebc515978fa80d7a2bde3e72df0648f3b89a Mon Sep 17 00:00:00 2001 From: Areek Zillur Date: Fri, 12 Aug 2016 17:05:35 -0400 Subject: [PATCH] Fix bug in single shard optimization when sorting documents in search request This commit adds a function to shard-level query result to determine whether there are any hits that needs fetching. Currently, a shard-level query result can have hits when there are search hits and/or completion suggestion hits. The newly added function encapsulates the checks to determine if a shard-level query result has any fetchable hits, which is used in optimizing for sorting documents and releasing search request contexts. --- .../action/search/AbstractSearchAsyncAction.java | 7 ++----- .../java/org/elasticsearch/search/SearchService.java | 9 ++------- .../search/controller/SearchPhaseController.java | 2 +- .../elasticsearch/search/query/QuerySearchResult.java | 6 ++++++ 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java b/core/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java index f9103f0cddc..a61384f9595 100644 --- a/core/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java +++ b/core/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java @@ -322,11 +322,8 @@ abstract class AbstractSearchAsyncAction // we only release search context that we did not fetch from if we are not scrolling if (request.scroll() == null) { for (AtomicArray.Entry entry : queryResults.asList()) { - QuerySearchResult queryResult = entry.value.queryResult().queryResult(); - final TopDocs topDocs = queryResult.topDocs(); - final Suggest suggest = queryResult.suggest(); - if (((topDocs != null && topDocs.scoreDocs.length > 0) // the shard had matches - ||suggest != null && suggest.hasScoreDocs()) // or had suggest docs + QuerySearchResult queryResult = entry.value.queryResult(); + if (queryResult.hasHits() && docIdsToLoad.get(entry.index) == null) { // but none of them made it to the global top docs try { DiscoveryNode node = nodes.get(entry.value.queryResult().shardTarget().nodeId()); diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index 4d618eb057a..dce1048a724 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -269,7 +269,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv loadOrExecuteQueryPhase(request, context); - if (hasHits(context.queryResult()) == false && context.scrollContext() == null) { + if (context.queryResult().hasHits() == false && context.scrollContext() == null) { freeContext(context.id()); } else { contextProcessedSuccessfully(context); @@ -324,7 +324,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv operationListener.onPreQueryPhase(context); long time = System.nanoTime(); queryPhase.execute(context); - if (hasHits(context.queryResult()) == false && context.scrollContext() == null) { + if (context.queryResult().hasHits() == false && context.scrollContext() == null) { // no hits, we can release the context since there will be no fetch phase freeContext(context.id()); } else { @@ -861,11 +861,6 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv context.docIdsToLoad(docIdsToLoad, 0, docIdsToLoad.length); } - private static boolean hasHits(final QuerySearchResult searchResult) { - return searchResult.topDocs().scoreDocs.length > 0 || - (searchResult.suggest() != null && searchResult.suggest().hasScoreDocs()); - } - private void processScroll(InternalScrollSearchRequest request, SearchContext context) { // process scroll context.from(context.from() + context.size()); diff --git a/core/src/main/java/org/elasticsearch/search/controller/SearchPhaseController.java b/core/src/main/java/org/elasticsearch/search/controller/SearchPhaseController.java index 97f3b191aa9..411e5593229 100644 --- a/core/src/main/java/org/elasticsearch/search/controller/SearchPhaseController.java +++ b/core/src/main/java/org/elasticsearch/search/controller/SearchPhaseController.java @@ -181,7 +181,7 @@ public class SearchPhaseController extends AbstractComponent { } else { // lets see if we only got hits from a single shard, if so, we can optimize... for (AtomicArray.Entry entry : results) { - if (entry.value.queryResult().topDocs().scoreDocs.length > 0) { + if (entry.value.queryResult().hasHits()) { if (result != null) { // we already have one, can't really optimize canOptimize = false; break; diff --git a/core/src/main/java/org/elasticsearch/search/query/QuerySearchResult.java b/core/src/main/java/org/elasticsearch/search/query/QuerySearchResult.java index 5125950a412..e583cfbf13e 100644 --- a/core/src/main/java/org/elasticsearch/search/query/QuerySearchResult.java +++ b/core/src/main/java/org/elasticsearch/search/query/QuerySearchResult.java @@ -188,6 +188,12 @@ public class QuerySearchResult extends QuerySearchResultProvider { return this; } + /** Returns true iff the result has hits */ + public boolean hasHits() { + return (topDocs != null && topDocs.scoreDocs.length > 0) || + (suggest != null && suggest.hasScoreDocs()); + } + public static QuerySearchResult readQuerySearchResult(StreamInput in) throws IOException { QuerySearchResult result = new QuerySearchResult(); result.readFrom(in);