Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SearchTimeoutIT #120390

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

javanna
Copy link
Member

@javanna javanna commented Jan 17, 2025

Two of the timeout tests have been muted for several months. The reason is that we tightened the assertions to cover for partial results being returned, but there were edge cases in which partial results were not actually returned.

The edge case is a typical timing issue where the initial check for timeout in CancellableBulkScorer already triggers the timeout, before any document has been collected.

I made several adjustments to the test to make it more robust:

  • use index random to index documents, that speeds it up
  • share indexing across test methods, so that it happens once at the suite level
  • raise a single timeout, rather than one per visited document which causes an unnecessary slowdown
  • make sure that one document is always visited before a timeout, so that partial results are always guaranteed to be returned

Closes #98369
Closes #98053

Two of the timeout tests have been muted for several months. The reason is that we
tightened the assertions to cover for partial results being returned, but there were
edge cases in which partial results were not actually returned.

The edge case is a typical timing issue where the initial check for timeout in
CancellableBulkScorer already triggers the timeout, before any document has been collected.

I made several adjustments to the test to make it more robust:
- use index random to index documents, that speeds it up
- share indexing across test methods, so that it happens once at the suite level
- raise a single timeout, rather than one per visited document which causes an unnecessary slowdown
- make sure that one document is always visited before a timeout, so that partial results are
always guaranteed to be returned

Closes elastic#98369
Closes elastic#98053
@elasticsearchmachine elasticsearchmachine added the Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch label Jan 17, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-foundations (Team:Search Foundations)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Search Foundations/Search Catch all for Search Foundations Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch v8.16.4 v8.17.2 v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] SearchTimeoutIT testTopHitsTimeout failing [CI] SearchTimeoutIT testAggsTimeout failing
2 participants