Skip to content

Conversation

@droberts195
Copy link

When a cluster is upgraded from a version that didn't have hidden indices/aliases to one that does, we make the relevant ML indices and aliases hidden. However, the steps to do this are split over 5 actions that run sequentially, so the work might take a second or two after the upgraded cluster starts up. This can cause the test that asserts on the final state to fail if it runs very quickly after the upgraded cluster startup. The solution is to retry the test for a few seconds to give a chance for all the upgrade actions to complete.

Relates #93062

When a cluster is upgraded from a version that didn't have hidden
indices/aliases to one that does, we make the relevant ML indices
and aliases hidden. However, the steps to do this are split over
5 actions that run sequentially, so the work might take a second or
two after the upgraded cluster starts up. This can cause the test
that asserts on the final state to fail if it runs very quickly
after the upgraded cluster startup. The solution is to retry the
test for a few seconds to give a chance for all the upgrade actions
to complete.

Relates elastic#93062
@droberts195 droberts195 added >test Issues or PRs that are addressing/adding tests :ml Machine learning auto-backport-and-merge v8.6.1 v8.7.0 v7.17.9 labels Jan 23, 2023
@elasticsearchmachine elasticsearchmachine added the Team:ML Meta label for the ML team label Jan 23, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

Comment on lines +106 to +109
// The 5 operations in MlInitializationService.makeMlInternalIndicesHidden() run sequentially, so might
// not all be finished when this test runs. The desired state should exist within a few seconds of startup,
// hence the assertBusy().
assertBusy(() -> {
Copy link
Author

Choose a reason for hiding this comment

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

All the changes apart from the comment and the assertBusy() are just auto-formatting changes.

Copy link
Contributor

@przemekwitek przemekwitek left a comment

Choose a reason for hiding this comment

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

LGTM

@droberts195 droberts195 merged commit 5ddb87c into elastic:main Jan 23, 2023
@droberts195 droberts195 deleted the assert_busy_waiting_for_hidden_indices branch January 23, 2023 12:00
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.6
7.17 Commit could not be cherrypicked due to conflicts

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

droberts195 pushed a commit to droberts195/elasticsearch that referenced this pull request Jan 23, 2023
elastic#93148)

When a cluster is upgraded from a version that didn't have hidden
indices/aliases to one that does, we make the relevant ML indices
and aliases hidden. However, the steps to do this are split over
5 actions that run sequentially, so the work might take a second or
two after the upgraded cluster starts up. This can cause the test
that asserts on the final state to fail if it runs very quickly
after the upgraded cluster startup. The solution is to retry the
test for a few seconds to give a chance for all the upgrade actions
to complete.

Relates elastic#93062
elasticsearchmachine pushed a commit that referenced this pull request Jan 23, 2023
#93148) (#93149)

When a cluster is upgraded from a version that didn't have hidden
indices/aliases to one that does, we make the relevant ML indices
and aliases hidden. However, the steps to do this are split over
5 actions that run sequentially, so the work might take a second or
two after the upgraded cluster starts up. This can cause the test
that asserts on the final state to fail if it runs very quickly
after the upgraded cluster startup. The solution is to retry the
test for a few seconds to give a chance for all the upgrade actions
to complete.

Relates #93062
@droberts195
Copy link
Author

It turns out this test and the related code isn't in 7.17, due to #53674 (comment). So this doesn't need backporting to 7.17 after all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:ml Machine learning Team:ML Meta label for the ML team >test Issues or PRs that are addressing/adding tests v8.6.1 v8.7.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants