From ca3d8817165d5b980f3dbb72647ce77cfab936f1 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Wed, 8 May 2019 12:25:58 +0200 Subject: [PATCH] Always set terminated_early if terminate_after is set in the search request (#40839) * terminated_early should always be set in the response with terminate_after Today we set `terminated_early` to true in the response if the query terminated early due to `terminate_after`. However if `terminate_after` is smaller than the number of documents in a shard we don't set the flag in the response indicating that the query was exhaustive. This change fixes this disprepancy by setting terminated_early to false in the response if the number of documents that match the query is smaller than the provided `terminate_after` value. Closes #33949 --- .../elasticsearch/search/query/QueryPhase.java | 4 ++++ .../search/query/QueryPhaseTests.java | 15 +++++++++++++-- .../search/simple/SimpleSearchIT.java | 2 +- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/query/QueryPhase.java b/server/src/main/java/org/elasticsearch/search/query/QueryPhase.java index 8f933b28d94..82d3528695c 100644 --- a/server/src/main/java/org/elasticsearch/search/query/QueryPhase.java +++ b/server/src/main/java/org/elasticsearch/search/query/QueryPhase.java @@ -282,6 +282,10 @@ public class QueryPhase implements SearchPhase { } finally { searchContext.clearReleasables(SearchContext.Lifetime.COLLECTION); } + if (searchContext.terminateAfter() != SearchContext.DEFAULT_TERMINATE_AFTER + && queryResult.terminatedEarly() == null) { + queryResult.terminatedEarly(false); + } final QuerySearchResult result = searchContext.queryResult(); for (QueryCollectorContext ctx : collectors) { diff --git a/server/src/test/java/org/elasticsearch/search/query/QueryPhaseTests.java b/server/src/test/java/org/elasticsearch/search/query/QueryPhaseTests.java index 16b18efe623..69d5f095948 100644 --- a/server/src/test/java/org/elasticsearch/search/query/QueryPhaseTests.java +++ b/server/src/test/java/org/elasticsearch/search/query/QueryPhaseTests.java @@ -345,11 +345,23 @@ public class QueryPhaseTests extends IndexShardTestCase { TestSearchContext context = new TestSearchContext(null, indexShard); context.setTask(new SearchTask(123L, "", "", "", null, Collections.emptyMap())); context.parsedQuery(new ParsedQuery(new MatchAllDocsQuery())); - context.terminateAfter(1); final IndexReader reader = DirectoryReader.open(dir); IndexSearcher contextSearcher = new IndexSearcher(reader); + context.terminateAfter(numDocs); + { + context.setSize(10); + TotalHitCountCollector collector = new TotalHitCountCollector(); + context.queryCollectors().put(TotalHitCountCollector.class, collector); + QueryPhase.execute(context, contextSearcher, checkCancelled -> {}); + assertFalse(context.queryResult().terminatedEarly()); + assertThat(context.queryResult().topDocs().topDocs.totalHits.value, equalTo((long) numDocs)); + assertThat(context.queryResult().topDocs().topDocs.scoreDocs.length, equalTo(10)); + assertThat(collector.getTotalHits(), equalTo(numDocs)); + } + + context.terminateAfter(1); { context.setSize(1); QueryPhase.execute(context, contextSearcher, checkCancelled -> {}); @@ -411,7 +423,6 @@ public class QueryPhaseTests extends IndexShardTestCase { assertThat(context.queryResult().topDocs().topDocs.scoreDocs.length, equalTo(0)); assertThat(collector.getTotalHits(), equalTo(1)); } - reader.close(); dir.close(); } diff --git a/server/src/test/java/org/elasticsearch/search/simple/SimpleSearchIT.java b/server/src/test/java/org/elasticsearch/search/simple/SimpleSearchIT.java index 3e546d17202..74b0408636f 100644 --- a/server/src/test/java/org/elasticsearch/search/simple/SimpleSearchIT.java +++ b/server/src/test/java/org/elasticsearch/search/simple/SimpleSearchIT.java @@ -250,7 +250,7 @@ public class SimpleSearchIT extends ESIntegTestCase { .setTerminateAfter(2 * max).get(); assertHitCount(searchResponse, max); - assertNull(searchResponse.isTerminatedEarly()); + assertFalse(searchResponse.isTerminatedEarly()); } public void testSimpleIndexSortEarlyTerminate() throws Exception {