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

[ML] Wait for shards to initialize after creating ML internal indices #59027

Merged

Conversation

droberts195
Copy link
Contributor

There have been a few test failures that are likely caused by tests
performing actions that use ML indices immediately after the actions
that create those ML indices. Currently this can result in attempts
to search the newly created index before its shards have initialized.

This change makes the method that creates the internal ML indices
that have been affected by this problem (state and stats) wait for
the shards to be initialized before returning.

Fixes #54887
Fixes #55221
Fixes #55807
Fixes #57102
Fixes #58841
Fixes #59011

There have been a few test failures that are likely caused by tests
performing actions that use ML indices immediately after the actions
that create those ML indices.  Currently this can result in attempts
to search the newly created index before its shards have initialized.

This change makes the method that creates the internal ML indices
that have been affected by this problem (state and stats) wait for
the shards to be initialized before returning.

Fixes elastic#54887
Fixes elastic#55221
Fixes elastic#55807
Fixes elastic#57102
Fixes elastic#58841
Fixes elastic#59011
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (:ml)

Copy link
Member

@davidkyle davidkyle left a comment

Choose a reason for hiding this comment

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

LGTM

The test failures are mocking issues in MlIndexAndAliasTests

@przemekwitek przemekwitek self-requested a review July 6, 2020 07:57
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

Thanks for fixing this, just 2 minor comments.

ActionListener.<ClusterHealthResponse>wrap(
response -> finalListener.onResponse(response.isTimedOut() == false),
finalListener::onFailure),
(request, listener) -> client.admin().cluster().health(request, listener)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you could replace this call with a more concise client.admin().cluster()::health.

ActionListener<Boolean> finalListener) {

// If the index and alias were successfully created then wait for the shards of the index that the alias points to be ready
ActionListener<Boolean> waitForShardsListener = ActionListener.wrap(
Copy link
Contributor

Choose a reason for hiding this comment

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

TBH it is always unclear to me how should I name the listener variable: after the action that takes place inside the listener or after the action that invokes the listener in the end?
Is there any codebase-wide guideline for that?

@droberts195
Copy link
Contributor Author

Jenkins run elasticsearch-ci/bwc

@droberts195
Copy link
Contributor Author

@elasticmachine update branch

@droberts195 droberts195 merged commit 0804b8c into elastic:master Jul 6, 2020
@droberts195 droberts195 deleted the wait_for_shards_on_index_create branch July 6, 2020 15:57
droberts195 added a commit to droberts195/elasticsearch that referenced this pull request Jul 6, 2020
There have been a few test failures that are likely caused by tests
performing actions that use ML indices immediately after the actions
that create those ML indices.  Currently this can result in attempts
to search the newly created index before its shards have initialized.

This change makes the method that creates the internal ML indices
that have been affected by this problem (state and stats) wait for
the shards to be initialized before returning.

Backport of elastic#59027
droberts195 added a commit that referenced this pull request Jul 7, 2020
…#59087)

There have been a few test failures that are likely caused by tests
performing actions that use ML indices immediately after the actions
that create those ML indices.  Currently this can result in attempts
to search the newly created index before its shards have initialized.

This change makes the method that creates the internal ML indices
that have been affected by this problem (state and stats) wait for
the shards to be initialized before returning.

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