Skip to content

Fix serverless test #121180

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

Merged

Conversation

carlosdelest
Copy link
Member

This test fails on serverless, as the wait for index to be green happened after the second insertion into the synonyms index. It should happen after the first to ensure the index has been created and search shards are available in order to perform the second insert.

@carlosdelest carlosdelest added >test Issues or PRs that are addressing/adding tests :Search Relevance/Analysis How text is split into tokens Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v9.0.0 labels Jan 29, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-relevance (Team:Search Relevance)

@elasticsearchmachine elasticsearchmachine added serverless-linked Added by automation, don't add manually v9.1.0 and removed v9.0.0 labels Jan 29, 2025
@carlosdelest
Copy link
Member Author

@benwtrent @mayya-sharipova may I get your review in this test change?

Thanks!

Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is OK. But I would expect users to want the ability to ensure that the synonyms are fully reloaded (e.g. the API waits until some timeout for it to occur).

Could we open an issue to add a flag to the API that allows it to spin wait until the synonyms are fully reloaded and available? Of course, there is always the case where it remains unavailable due to bad state, etc. So, there will have to be some timeout associated with it.

@carlosdelest
Copy link
Member Author

But I would expect users to want the ability to ensure that the synonyms are fully reloaded (e.g. the API waits until some timeout for it to occur).

The API does not return until all analyzers have been reloaded. But that doesn't mean that it succeeds on the operation, which is what happened on the test.

We could add a final check on the API requests that updates the synonym(s) to perform a request for them afterwards, and deal there with the potential 503 returned in serverless - waiting for a timeout if needed. I'd put that as part of the default behaviour as the operation is expected to return when analyzers have been reloaded and we're all set. I've opened #121441 to deal with that.

I'm not sure how other system indices deal with this situation, if they do it at all. 🤔

@carlosdelest carlosdelest merged commit 67c2f41 into elastic:main Jan 31, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Search Relevance/Analysis How text is split into tokens serverless-linked Added by automation, don't add manually Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch >test Issues or PRs that are addressing/adding tests v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants