diff --git a/docs/changelog/122575.yaml b/docs/changelog/122575.yaml new file mode 100644 index 0000000000000..af72c81b9da8c --- /dev/null +++ b/docs/changelog/122575.yaml @@ -0,0 +1,6 @@ +pr: 122575 +summary: Return an empty suggestion when suggest phase times out +area: Suggesters +type: bug +issues: + - 122548 diff --git a/muted-tests.yml b/muted-tests.yml index 311196d5b94fd..395c7771dc7e6 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -323,9 +323,6 @@ tests: - class: org.elasticsearch.compute.operator.exchange.ExchangeServiceTests method: testExchangeSourceContinueOnFailure issue: https://github.com/elastic/elasticsearch/issues/122408 -- class: org.elasticsearch.search.SearchTimeoutIT - method: testSuggestTimeoutWithPartialResults - issue: https://github.com/elastic/elasticsearch/issues/122548 - class: org.elasticsearch.repositories.blobstore.testkit.analyze.MinioRepositoryAnalysisRestIT method: testRepositoryAnalysis issue: https://github.com/elastic/elasticsearch/issues/122670 diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/SearchTimeoutIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/SearchTimeoutIT.java index f79321ef8d0de..8100e911f77cb 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/SearchTimeoutIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/SearchTimeoutIT.java @@ -484,8 +484,7 @@ protected TermSuggestion innerExecute( CharsRefBuilder spare ) { contextIndexSearcher.throwTimeExceededException(); - assert false; - return new TermSuggestion(name, suggestion.getSize(), SortBy.SCORE); + throw new AssertionError("should have thrown TimeExceededException"); } @Override 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 8ad52c4f9bb59..5fcfb2b9766cd 100644 --- a/server/src/main/java/org/elasticsearch/search/query/QueryPhase.java +++ b/server/src/main/java/org/elasticsearch/search/query/QueryPhase.java @@ -126,15 +126,7 @@ static void executeRank(SearchContext searchContext) throws QueryPhaseExecutionE static void executeQuery(SearchContext searchContext) throws QueryPhaseExecutionException { if (searchContext.hasOnlySuggest()) { - try { - SuggestPhase.execute(searchContext); - } catch (ContextIndexSearcher.TimeExceededException timeExceededException) { - SearchTimeoutException.handleTimeout( - searchContext.request().allowPartialSearchResults(), - searchContext.shardTarget(), - searchContext.queryResult() - ); - } + SuggestPhase.execute(searchContext); searchContext.queryResult().topDocs(new TopDocsAndMaxScore(Lucene.EMPTY_TOP_DOCS, Float.NaN), new DocValueFormat[0]); return; } @@ -150,18 +142,10 @@ static void executeQuery(SearchContext searchContext) throws QueryPhaseExecution addCollectorsAndSearch(searchContext); - try { - RescorePhase.execute(searchContext); - SuggestPhase.execute(searchContext); - if (searchContext.getProfilers() != null) { - searchContext.queryResult().profileResults(searchContext.getProfilers().buildQueryPhaseResults()); - } - } catch (ContextIndexSearcher.TimeExceededException timeExceededException) { - SearchTimeoutException.handleTimeout( - searchContext.request().allowPartialSearchResults(), - searchContext.shardTarget(), - searchContext.queryResult() - ); + RescorePhase.execute(searchContext); + SuggestPhase.execute(searchContext); + if (searchContext.getProfilers() != null) { + searchContext.queryResult().profileResults(searchContext.getProfilers().buildQueryPhaseResults()); } } diff --git a/server/src/main/java/org/elasticsearch/search/rescore/RescorePhase.java b/server/src/main/java/org/elasticsearch/search/rescore/RescorePhase.java index 7223da3c6101b..fdd5efceaae3c 100644 --- a/server/src/main/java/org/elasticsearch/search/rescore/RescorePhase.java +++ b/server/src/main/java/org/elasticsearch/search/rescore/RescorePhase.java @@ -18,7 +18,9 @@ import org.elasticsearch.common.lucene.search.TopDocsAndMaxScore; import org.elasticsearch.common.util.Maps; import org.elasticsearch.lucene.grouping.TopFieldGroups; +import org.elasticsearch.search.internal.ContextIndexSearcher; import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.search.query.SearchTimeoutException; import org.elasticsearch.search.sort.ShardDocSortField; import org.elasticsearch.search.sort.SortAndFormats; @@ -84,6 +86,13 @@ public static void execute(SearchContext context) { .topDocs(new TopDocsAndMaxScore(topDocs, topDocs.scoreDocs[0].score), context.queryResult().sortValueFormats()); } catch (IOException e) { throw new ElasticsearchException("Rescore Phase Failed", e); + } catch (ContextIndexSearcher.TimeExceededException timeExceededException) { + SearchTimeoutException.handleTimeout( + context.request().allowPartialSearchResults(), + context.shardTarget(), + context.queryResult() + ); + // if the rescore phase times out and partial results are allowed, the returned top docs from this shard won't be rescored } } diff --git a/server/src/main/java/org/elasticsearch/search/suggest/SuggestPhase.java b/server/src/main/java/org/elasticsearch/search/suggest/SuggestPhase.java index 272855bacd544..17ff07f167ff8 100644 --- a/server/src/main/java/org/elasticsearch/search/suggest/SuggestPhase.java +++ b/server/src/main/java/org/elasticsearch/search/suggest/SuggestPhase.java @@ -10,7 +10,9 @@ import org.apache.lucene.util.CharsRefBuilder; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.search.internal.ContextIndexSearcher; import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.search.query.SearchTimeoutException; import org.elasticsearch.search.suggest.Suggest.Suggestion; import org.elasticsearch.search.suggest.Suggest.Suggestion.Entry; import org.elasticsearch.search.suggest.Suggest.Suggestion.Entry.Option; @@ -40,12 +42,17 @@ public static void execute(SearchContext context) { for (Map.Entry entry : suggest.suggestions().entrySet()) { SuggestionSearchContext.SuggestionContext suggestion = entry.getValue(); Suggester suggester = suggestion.getSuggester(); - Suggestion> result = suggester.execute( - entry.getKey(), - suggestion, - context.searcher(), - spare - ); + Suggestion> result; + try { + result = suggester.execute(entry.getKey(), suggestion, context.searcher(), spare); + } catch (ContextIndexSearcher.TimeExceededException timeExceededException) { + SearchTimeoutException.handleTimeout( + context.request().allowPartialSearchResults(), + context.shardTarget(), + context.queryResult() + ); + result = suggester.emptySuggestion(entry.getKey(), suggestion, spare); + } if (result != null) { assert entry.getKey().equals(result.name); suggestions.add(result); diff --git a/server/src/test/java/org/elasticsearch/search/query/QueryPhaseTimeoutTests.java b/server/src/test/java/org/elasticsearch/search/query/QueryPhaseTimeoutTests.java index 6b38e05bdc4e3..aa9ca26974bad 100644 --- a/server/src/test/java/org/elasticsearch/search/query/QueryPhaseTimeoutTests.java +++ b/server/src/test/java/org/elasticsearch/search/query/QueryPhaseTimeoutTests.java @@ -297,7 +297,8 @@ public void testSuggestOnlyWithTimeout() throws Exception { assertTrue(context.hasOnlySuggest()); QueryPhase.execute(context); assertTrue(context.queryResult().searchTimedOut()); - assertNull(context.queryResult().suggest()); + assertEquals(1, context.queryResult().suggest().size()); + assertEquals(0, context.queryResult().suggest().getSuggestion("suggestion").getEntries().size()); assertNotNull(context.queryResult().topDocs()); assertEquals(0, context.queryResult().topDocs().topDocs.totalHits.value()); } @@ -311,7 +312,8 @@ public void testSuggestAndQueryWithSuggestTimeout() throws Exception { QueryPhase.execute(context); assertThat(context.queryResult().topDocs().topDocs.totalHits.value(), Matchers.greaterThan(0L)); assertTrue(context.queryResult().searchTimedOut()); - assertNull(context.queryResult().suggest()); + assertEquals(1, context.queryResult().suggest().size()); + assertEquals(0, context.queryResult().suggest().getSuggestion("suggestion").getEntries().size()); } }