Skip to content

Conversation

@javanna
Copy link
Member

@javanna javanna commented Feb 15, 2025

We recently fixed timeout handling in the suggest phase. A test failure on SearchTimeoutIT surfaced an issue with the current approach. In case partial results are allowed, it may happen that some shards time out while executing the suggest phase and some don't.

SearchPhaseController assumes that if one shard has suggest results, all of the other shards will have suggest results too. We could address that assertion and check is the search timed out, instead this commit changes timeout handling in the suggest phase to return an empty suggestion instead of null. This seems appropriate in terms of providing some results and makes the assertion about non null suggestions in SearchPhaseController happy.

Relates to #122357

Closes #122548

We recently fixed timeout handling in the suggest phase. A test failure on SearchTimeoutIT surfaced an issue with the current approach. In case partial results are allowed, it may happen that some shards time out while executing the suggest phase and some don't.

SearchPhaseController assumes that if one shard has suggest results, all of the other shards will have suggest results too. We could address that assertion and check is the search timed out, instead this commit changes timeout handling in the suggest phase to return an empty suggestion instead of null. This seems appropriate in terms of providing some results and makes the assertion about non null suggestions in SearchPhaseController happy.

Relates to elastic#122357

Closes elastic#122548
@javanna javanna added :Search Relevance/Suggesters "Did you mean" and suggestions as you type backport auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) labels Feb 15, 2025
@javanna javanna changed the title Return an empty suggestion when suggest phase times out (#122575) [8.18] Return an empty suggestion when suggest phase times out (#122575) Feb 15, 2025
@elasticsearchmachine elasticsearchmachine merged commit b2e6c15 into elastic:8.18 Feb 15, 2025
16 checks passed
@javanna javanna deleted the backport/8.18/122575 branch February 15, 2025 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) backport :Search Relevance/Suggesters "Did you mean" and suggestions as you type v8.18.1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants