-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[saved objects] Add migrations v2 integration test for scenario with multiple Kibana instances. #100171
Conversation
…multiple Kibana nodes.
Pinging @elastic/kibana-core (Team:Core) |
src/core/server/saved_objects/migrationsv2/integration_tests/multiple_kibana_nodes.test.ts
Show resolved
Hide resolved
src/core/server/saved_objects/migrationsv2/integration_tests/multiple_kibana_nodes.test.ts
Show resolved
Hide resolved
src/core/server/saved_objects/migrationsv2/integration_tests/multiple_kibana_nodes.test.ts
Outdated
Show resolved
Hide resolved
src/core/server/saved_objects/migrationsv2/integration_tests/multiple_kibana_nodes.test.ts
Outdated
Show resolved
Hide resolved
src/core/server/saved_objects/migrationsv2/integration_tests/multiple_kibana_nodes.test.ts
Outdated
Show resolved
Hide resolved
src/core/server/saved_objects/migrationsv2/integration_tests/multiple_kibana_nodes.test.ts
Outdated
Show resolved
Hide resolved
src/core/server/saved_objects/migrationsv2/integration_tests/multiple_kibana_nodes.test.ts
Outdated
Show resolved
Hide resolved
src/core/server/saved_objects/migrationsv2/integration_tests/multiple_kibana_nodes.test.ts
Outdated
Show resolved
Hide resolved
src/core/server/saved_objects/migrationsv2/integration_tests/multiple_kibana_nodes.test.ts
Outdated
Show resolved
Hide resolved
src/core/server/saved_objects/migrationsv2/integration_tests/multiple_kibana_nodes.test.ts
Show resolved
Hide resolved
src/core/server/saved_objects/migrationsv2/integration_tests/multiple_kibana_nodes.test.ts
Show resolved
Hide resolved
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💔 Build Failed
Failed CI StepsTest FailuresKibana Pipeline / general / Jest Integration Tests.src/core/server/saved_objects/migrationsv2/integration_tests.migration v2 migrates saved objects normally when multiple Kibana instances are runningStandard Out
Stack Trace
Metrics [docs]Unknown metric groupsReferences to deprecated APIs
History
To update your PR or re-run it, just comment with: cc @lukeelmers |
I hadn't seen these failures until this week (perhaps just a coincidence), so I ran it through the flaky test runner overnight, and it failed 30 out of 100 times 🙁 As a comparison, I ran this same branch through the flaky test runner with the v1 migrations enabled, and all 100 runs passed (makes sense I guess, as we weren't using write blocks in the old algorithm). So I did some digging. AFAICT v2 migrations aren't handling this particular type of kibana/src/core/server/saved_objects/migrationsv2/actions/index.ts Lines 743 to 745 in d6828a2
The error in the stack trace comes from kibana/src/core/server/saved_objects/migrationsv2/actions/index.ts Lines 1229 to 1233 in d6828a2
I'm not super familiar with this API, but seems it's coming from here in ES: Perhaps it's a scenario we didn't consider (or run into), where multiple Kibanas migrating in parallel could also hit this error, if one starts creating the write block before the others? When I stuck in some code here to ignore any Would appreciate any thoughts @elastic/kibana-core -- should we be handling this error in v2 migrations, or am I doing something unusual in the test that would trigger this error in a way would not realistically happen for users? |
This is happening because we're setting So, yea, in a situation where This is surprising you didn't encounter this issue sooner while implementing these tests btw, as it seems a relatively 'easy' race condition to reproduce if both instances are not started at the exact same time. Now, for the resolution, it kinda depends on what we will choose to do regarding #101052 If we decide to keep the existing behavior and have the migration fails if the instance encounters unknown SO types, I think it would be acceptable to have Now, if we decide to copy the unknown documents instead (and allow instances to have different set of plugins enabled), this can be way trickier, as I thinkwe can't safely perform the same step jump? (also see #101052 (comment)) |
Putting this PR on hold pending the outcome of #101351 Once that's resolved we should update/merge this, or incorporate it into that PR. For the time being, I'll move this back into draft. |
Closing in favor of #104516 |
Closes #99211
This adds integration tests for two scenarios:
(Confirms v2 algorithm operations are idempotent)
(Confirms there are no possible race conditions between instances that have & don't have certain plugins)
Multiple Kibana instances are simulated by creating multiple
Root
s. To allow ample time for the possibility of races, the data set is 5k objects, and I'm using a batch size of 100.This was my first time really diving into the v2 migrations code, maybe others who have worked with it more will have ideas of other ways to try to simulate race conditions 🙂