Skip to content

[Advanced Settings] Add retry for 409 conflicts in API integration tests#189813

Merged
ElenaStoeva merged 7 commits intoelastic:mainfrom
ElenaStoeva:advanced-settings/fix-api-integration-tests
Aug 14, 2024
Merged

[Advanced Settings] Add retry for 409 conflicts in API integration tests#189813
ElenaStoeva merged 7 commits intoelastic:mainfrom
ElenaStoeva:advanced-settings/fix-api-integration-tests

Conversation

@ElenaStoeva
Copy link
Copy Markdown
Contributor

@ElenaStoeva ElenaStoeva commented Aug 2, 2024

Fixes #176445

Summary

Flaky test runner: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6679

This PR fixes the advanced settings API integration tests that seem to be flaky. The reason for the occasional failures is most likely a version_conflict_engine_exception which is thrown when another node indexes the same documents. This can happen when we save an advanced setting since the settings API uses saved objects under the hood, and in CI, multiple nodes can try to save an advanced setting simultaneously.

The solution in this PR is to retry the request if we encounter a 409 error. This is adapted from the solution in #174185 which resolves a similar failure.

@ElenaStoeva ElenaStoeva self-assigned this Aug 2, 2024
@ElenaStoeva ElenaStoeva added Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// release_note:skip Skip the PR/issue when compiling release notes labels Aug 5, 2024
@kibanamachine
Copy link
Copy Markdown
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#6679

[✅] x-pack/test/api_integration/apis/management/config.ts: 200/200 tests passed.

see run history

@ElenaStoeva ElenaStoeva marked this pull request as ready for review August 5, 2024 11:42
@ElenaStoeva ElenaStoeva requested a review from a team as a code owner August 5, 2024 11:42
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-management (Team:Kibana Management)

Copy link
Copy Markdown
Member

@sabarasaba sabarasaba left a comment

Choose a reason for hiding this comment

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

Thanks a lot for patching this up @ElenaStoeva! Code changes lgtm, the only thing I wanted to get your opinion on is if you think the retry_if_conflicts util might be useful somewhere else at some point? Otherwise might be worth to move to a common folder

@ElenaStoeva
Copy link
Copy Markdown
Contributor Author

Thanks for the review @sabarasaba!

the only thing I wanted to get your opinion on is if you think the retry_if_conflicts util might be useful somewhere else at some point? Otherwise might be worth to move to a common folder

Good call! This util may be useful somewhere else but I'm not sure where exactly since this is for a bit specific case where the called API uses saved objects under the hood. I looked into the test directory structure but I didn't find a place where it made sense to me to move this file to. Do you have any specific location in mind?

@ElenaStoeva ElenaStoeva enabled auto-merge (squash) August 14, 2024 09:34
@kibana-ci
Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @ElenaStoeva

@ElenaStoeva ElenaStoeva merged commit cc29eea into elastic:main Aug 14, 2024
@kibanamachine
Copy link
Copy Markdown
Contributor

💔 All backports failed

Status Branch Result
8.14 Backport failed because of merge conflicts
8.15 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 189813

Questions ?

Please refer to the Backport tool documentation

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 16, 2024
@kibanamachine
Copy link
Copy Markdown
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 189813 locally

ElenaStoeva added a commit to ElenaStoeva/kibana that referenced this pull request Aug 19, 2024
…sts (elastic#189813)

Fixes elastic#176445

Flaky test runner:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6679

This PR fixes the advanced settings API integration tests that seem to
be flaky. The reason for the occasional failures is most likely a
`version_conflict_engine_exception` which is thrown when another node
indexes the same documents. This can happen when we save an advanced
setting since the settings API uses saved objects under the hood, and in
CI, multiple nodes can try to save an advanced setting simultaneously.

The solution in this PR is to retry the request if we encounter a 409
error. This is adapted from the solution in
elastic#174185 which resolves a similar
failure.

(cherry picked from commit cc29eea)
ElenaStoeva added a commit to ElenaStoeva/kibana that referenced this pull request Aug 19, 2024
…sts (elastic#189813)

Fixes elastic#176445

Flaky test runner:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6679

This PR fixes the advanced settings API integration tests that seem to
be flaky. The reason for the occasional failures is most likely a
`version_conflict_engine_exception` which is thrown when another node
indexes the same documents. This can happen when we save an advanced
setting since the settings API uses saved objects under the hood, and in
CI, multiple nodes can try to save an advanced setting simultaneously.

The solution in this PR is to retry the request if we encounter a 409
error. This is adapted from the solution in
elastic#174185 which resolves a similar
failure.

(cherry picked from commit cc29eea)
@kibanamachine
Copy link
Copy Markdown
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 189813 locally

@ElenaStoeva ElenaStoeva removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 20, 2024
@ElenaStoeva ElenaStoeva added the backport:skip This PR does not require backporting label Aug 20, 2024
@ElenaStoeva ElenaStoeva deleted the advanced-settings/fix-api-integration-tests branch January 31, 2026 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v8.14.0 v8.15.0 v8.16.0

Projects

None yet

5 participants