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.

Sign up for GitHub

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

Jump to bottom

[ML] Async creation of the .ml-annotations index can spill into subsequent tests #38952

Closed
droberts195 opened this issue Feb 15, 2019 · 3 comments · Fixed by #39049
Closed

[ML] Async creation of the .ml-annotations index can spill into subsequent tests #38952

droberts195 opened this issue Feb 15, 2019 · 3 comments · Fixed by #39049
Assignees
@droberts195
Labels
:ml Machine learning >test Issues or PRs that are addressing/adding tests

Comments

@droberts195
Copy link
Contributor

droberts195 commented Feb 15, 2019 •
edited
Loading

Following the change of #38903 the way we create the .ml-annotations-6 index will cause occasional assertion failures in tests that extend ESSingleNodeTestCase. One such failure is https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+7.0+internalClusterTest/589/consoleText

Currently the .ml-annotations-6 index is created by a cluster state listener if any other ML index exists. This is nice for production, because it avoids the clutter of unconditionally creating .ml-annotations-6 if ML is not in use, yet makes sure it is available for creating annotations as soon as any ML functionality is used. But it is nasty for tests, as creation of the index can spill from one test into another.

Some ideas for fixing this could be:

  • Switch all ML tests that extend ESSingleNodeTestCase to instead extend an intermediate ML test base class that has extra cleanup code. The extra cleanup would need to:
    1. Check if any ML indices existed
    2. If so, wait for the .ml-annotations-6 index to exist
    3. Delete the other ML indices found in step 1.
    4. Call the super class cleanup

or

  • Add a setting to disable creation of the ML annotations index and aliases. Set this in ESSingleNodeTestCase. Any ML tests that were testing ML annotations would have to explicitly create it.

There might be better ways. Any thoughts?
@droberts195 droberts195 added >test Issues or PRs that are addressing/adding tests :ml Machine learning team-discuss labels Feb 15, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core

@jasontedor
Copy link
Member

I favor the first approach. The second approach would require that the setting be in OSS to put it into ESSingleNodeTestCase, or that we have a single base class that all X-Pack ESSingleNodeTestCases would extend to set the setting there. Neither of those options are attractive to me. The first is not attractive because of the spilling into OSS, and the second is not attractive because of the overhead involved.

@iverase
Copy link
Contributor

iverase commented Feb 15, 2019

Another case:

https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+multijob-unix-compatibility/os=sles-12/245/console

@droberts195 droberts195 self-assigned this Feb 18, 2019
@droberts195 droberts195 mentioned this issue Feb 18, 2019
Closed
droberts195 added a commit to droberts195/elasticsearch that referenced this issue Feb 18, 2019
@droberts195
[ML] More advanced post-test cleanup of ML indices
0860941 
The .ml-annotations index is created asynchronously when
some other ML index exists.  This can interfere with the
post-test index deletion, as the .ml-annotations index
can be created after all other indices have been deleted.

This change adds an ML specific post-test cleanup step
that runs before the main cleanup and:

1. Checks if any ML indices exist
2. If so, waits for the .ml-annotations index to exist
3. Deletes the other ML indices found in step 1.
4. Calls the super class cleanup

This means that by the time the main post-test index
cleanup code runs:

1. The only ML index it has to delete will be the
   .ml-annotations index
2. No other ML indices will exist that could trigger
   recreation of the .ml-annotations index

Fixes elastic#38952
@droberts195 droberts195 mentioned this issue Feb 18, 2019
Merged
droberts195 added a commit that referenced this issue Feb 18, 2019
@droberts195
[ML] More advanced post-test cleanup of ML indices (#39049)
268a3ad 
The .ml-annotations index is created asynchronously when
some other ML index exists.  This can interfere with the
post-test index deletion, as the .ml-annotations index
can be created after all other indices have been deleted.

This change adds an ML specific post-test cleanup step
that runs before the main cleanup and:

1. Checks if any ML indices exist
2. If so, waits for the .ml-annotations index to exist
3. Deletes the other ML indices found in step 1.
4. Calls the super class cleanup

This means that by the time the main post-test index
cleanup code runs:

1. The only ML index it has to delete will be the
   .ml-annotations index
2. No other ML indices will exist that could trigger
   recreation of the .ml-annotations index

Fixes #38952
droberts195 added a commit that referenced this issue Feb 18, 2019
@droberts195
[ML] More advanced post-test cleanup of ML indices (#39049)
b660d2c 
The .ml-annotations index is created asynchronously when
some other ML index exists.  This can interfere with the
post-test index deletion, as the .ml-annotations index
can be created after all other indices have been deleted.

This change adds an ML specific post-test cleanup step
that runs before the main cleanup and:

1. Checks if any ML indices exist
2. If so, waits for the .ml-annotations index to exist
3. Deletes the other ML indices found in step 1.
4. Calls the super class cleanup

This means that by the time the main post-test index
cleanup code runs:

1. The only ML index it has to delete will be the
   .ml-annotations index
2. No other ML indices will exist that could trigger
   recreation of the .ml-annotations index

Fixes #38952
droberts195 added a commit that referenced this issue Feb 18, 2019
@droberts195
[ML] More advanced post-test cleanup of ML indices (#39049)
92e2ccf 
The .ml-annotations index is created asynchronously when
some other ML index exists.  This can interfere with the
post-test index deletion, as the .ml-annotations index
can be created after all other indices have been deleted.

This change adds an ML specific post-test cleanup step
that runs before the main cleanup and:

1. Checks if any ML indices exist
2. If so, waits for the .ml-annotations index to exist
3. Deletes the other ML indices found in step 1.
4. Calls the super class cleanup

This means that by the time the main post-test index
cleanup code runs:

1. The only ML index it has to delete will be the
   .ml-annotations index
2. No other ML indices will exist that could trigger
   recreation of the .ml-annotations index

Fixes #38952
droberts195 added a commit that referenced this issue Feb 18, 2019
@droberts195
[ML] More advanced post-test cleanup of ML indices (#39049)
2e641d9 
The .ml-annotations index is created asynchronously when
some other ML index exists.  This can interfere with the
post-test index deletion, as the .ml-annotations index
can be created after all other indices have been deleted.

This change adds an ML specific post-test cleanup step
that runs before the main cleanup and:

1. Checks if any ML indices exist
2. If so, waits for the .ml-annotations index to exist
3. Deletes the other ML indices found in step 1.
4. Calls the super class cleanup

This means that by the time the main post-test index
cleanup code runs:

1. The only ML index it has to delete will be the
   .ml-annotations index
2. No other ML indices will exist that could trigger
   recreation of the .ml-annotations index

Fixes #38952
droberts195 added a commit that referenced this issue Feb 18, 2019
@droberts195
[ML] More advanced post-test cleanup of ML indices (#39049)
9df015a 
The .ml-annotations index is created asynchronously when
some other ML index exists.  This can interfere with the
post-test index deletion, as the .ml-annotations index
can be created after all other indices have been deleted.

This change adds an ML specific post-test cleanup step
that runs before the main cleanup and:

1. Checks if any ML indices exist
2. If so, waits for the .ml-annotations index to exist
3. Deletes the other ML indices found in step 1.
4. Calls the super class cleanup

This means that by the time the main post-test index
cleanup code runs:

1. The only ML index it has to delete will be the
   .ml-annotations index
2. No other ML indices will exist that could trigger
   recreation of the .ml-annotations index

Fixes #38952
@droberts195 droberts195 mentioned this issue Sep 10, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@droberts195 droberts195

Labels
:ml Machine learning >test Issues or PRs that are addressing/adding tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[ML] More advanced post-test cleanup of ML indices droberts195/elasticsearch
4 participants
@jasontedor @droberts195 @elasticmachine @iverase