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 and unmute synonyms tests #117486

Merged

Conversation

carlosdelest
Copy link
Member

Closes #116777

This PR modifies waiting condition for synonyms index:

  • Wait for green status on the index, as waiting for wait_for_no_initializing_shards didn't cover all cases in serverless.
  • Add a timeout, as stateful tests won't be able to get green index due to auto-expand replicas being set to 0-all in synonyms index. BwC tests upgrade nodes and make 2 shards unavailable that can't be recovered on the tests, but doesn't interfere with the testing.
  • Ignoring the timeout - as it provides enough time on serverless for the search shards to become available, and BwC test.

This test can't be properly fixed until auto-expand replicas are set to 0-1 on synonyms index. Once that is done, ignoring the timeout should be removed.

Related PRs:

@carlosdelest carlosdelest added >test Issues or PRs that are addressing/adding tests :Search Relevance/Analysis How text is split into tokens auto-backport Automatically create backport pull requests when merged Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v9.0.0 v8.18.0 labels Nov 25, 2024
@elasticsearchmachine elasticsearchmachine added the serverless-linked Added by automation, don't add manually label Nov 25, 2024
@carlosdelest carlosdelest marked this pull request as ready for review November 25, 2024 17:27
@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Contributor

@bcully bcully left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@carlosdelest carlosdelest merged commit 930a99c into elastic:main Nov 25, 2024
17 checks passed
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.17 Commit could not be cherrypicked due to conflicts
8.x Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 117486

carlosdelest added a commit to carlosdelest/elasticsearch that referenced this pull request Nov 25, 2024
(cherry picked from commit 930a99c)

# Conflicts:
#	muted-tests.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/110_synonyms_invalid.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/20_synonyms_get.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/30_synonyms_delete.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/40_synonyms_sets_get.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/60_synonym_rule_get.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/80_synonyms_from_index.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/90_synonyms_reloading_for_synset.yml
carlosdelest added a commit to carlosdelest/elasticsearch that referenced this pull request Nov 25, 2024
(cherry picked from commit 930a99c)

# Conflicts:
#	muted-tests.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/110_synonyms_invalid.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/20_synonyms_get.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/30_synonyms_delete.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/40_synonyms_sets_get.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/60_synonym_rule_get.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/80_synonyms_from_index.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/90_synonyms_reloading_for_synset.yml
@carlosdelest
Copy link
Member Author

💚 All backports created successfully

Status Branch Result
8.x
8.17

Questions ?

Please refer to the Backport tool documentation

alexey-ivanov-es pushed a commit to alexey-ivanov-es/elasticsearch that referenced this pull request Nov 28, 2024
carlosdelest added a commit to carlosdelest/elasticsearch that referenced this pull request Dec 9, 2024
(cherry picked from commit 930a99c)

# Conflicts:
#	muted-tests.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/110_synonyms_invalid.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/20_synonyms_get.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/30_synonyms_delete.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/40_synonyms_sets_get.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/60_synonym_rule_get.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/80_synonyms_from_index.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/90_synonyms_reloading_for_synset.yml
elasticsearchmachine pushed a commit that referenced this pull request Dec 10, 2024
…#117555) (#118297)

* Fix and unmute synonyms tests using timeout (#117486)

(cherry picked from commit 930a99c)

# Conflicts:
#	muted-tests.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/110_synonyms_invalid.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/20_synonyms_get.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/30_synonyms_delete.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/40_synonyms_sets_get.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/60_synonym_rule_get.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/80_synonyms_from_index.yml
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/90_synonyms_reloading_for_synset.yml

* Fix merge
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged backport pending :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 v8.17.0 v8.18.0 v9.0.0
Projects
None yet
3 participants