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] More advanced post-test cleanup of ML indices #39049

Merged
merged 1 commit into from
Feb 18, 2019
Merged

[ML] More advanced post-test cleanup of ML indices #39049

merged 1 commit into from
Feb 18, 2019

Conversation

droberts195
Copy link
Contributor

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
[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 added >test Issues or PRs that are addressing/adding tests v7.0.0 :ml Machine learning v6.7.0 v8.0.0 v7.2.0 v6.6.2 labels Feb 18, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core

dimitris-athanasiou
dimitris-athanasiou approved these changes Feb 18, 2019
View reviewed changes
Copy link
Contributor

@dimitris-athanasiou dimitris-athanasiou 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 268a3ad into elastic:master Feb 18, 2019
@droberts195 droberts195 deleted the improved-ml-index-cleanup branch February 18, 2019 14:05
jasontedor
jasontedor reviewed Feb 18, 2019
View reviewed changes
x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlSingleNodeTestCase.java
@Override
public void tearDown() throws Exception {
try {
logger.info("[{}#{}]: ML-specific after test cleanup", getTestClass().getSimpleName(), getTestName());
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason for making this info level? I would like to see our test logs have less lines in them, not more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's info in the same place in ESSingleNodeTestCase, so I copied that. I'm happy to downgrade this one to trace though.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I did not realize that we had an info line there in ESSingleNodeTestCase too. Thanks for pointing that out. Yes, I think that not only this one but both can be downgraded to trace. 🙏

droberts195 added a commit that referenced this pull request 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 pull request 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 pull request 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 pull request 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 added a commit to droberts195/elasticsearch that referenced this pull request Feb 18, 2019
@droberts195
Reduce single node test cleanup logging
3f96d84 
As per elastic#39049 (comment)
@droberts195 droberts195 mentioned this pull request Feb 18, 2019
Merged
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Feb 18, 2019
@jasontedor
Merge remote-tracking branch 'elastic/master' into retention-lease-st…
8433480 
…ate-file

* elastic/master:
  Remove tests and branches that will never execute (elastic#38772)
  also check ccr stats api return empty response in ensureNoCcrTasks()
  Add overlapping, before, after filters to intervals query (elastic#38999)
  Mute test elastic#38949
  Add remote recovery to ShardFollowTaskReplicationTests (elastic#39007)
  [ML] More advanced post-test cleanup of ML indices (elastic#39049)
  wait for shard to be allocated before executing a resume follow api
  Update track-total-hits.asciidoc
  Force kill testcluster nodes (elastic#37353)
  Make pullFixture a task dependency of resolveAllDependencies (elastic#38956)
  set minimum supported version (elastic#39043)
  Enforce Completion Context Limit (elastic#38675)
  Mute test
  Don't close caches while there might still be in-flight requests. (elastic#38958)
  Fix elastic#38623 remove xpack namespace REST API (elastic#38625)
  Add data frame feature (elastic#38934)
  Test bi-directional index following during a rolling upgrade. (elastic#38962)
  Generate mvn pom for ssl-config library (elastic#39019)
  Mute testRetentionLeaseIsRenewedDuringRecovery
droberts195 added a commit that referenced this pull request Feb 18, 2019
@droberts195
Reduce single node test cleanup logging (#39060)
de8ba77 
As per #39049 (comment)
droberts195 added a commit that referenced this pull request Feb 18, 2019
@droberts195
Reduce single node test cleanup logging (#39060)
ae9243a 
As per #39049 (comment)
@droberts195 droberts195 mentioned this pull request Sep 10, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasontedor jasontedor jasontedor left review comments

@dimitris-athanasiou dimitris-athanasiou dimitris-athanasiou approved these changes

Assignees
No one assigned
Labels
:ml Machine learning >test Issues or PRs that are addressing/adding tests v6.6.2 v6.7.0 v7.0.0-rc2 v7.2.0 v8.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ML] Async creation of the .ml-annotations index can spill into subsequent tests
5 participants
@droberts195 @elasticmachine @jasontedor @dimitris-athanasiou @jakelandis