Skip to content

HLREST: Machine learning tests leave ML state after they've finished #32993

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

Closed
nik9000 opened this issue Aug 20, 2018 · 5 comments
Closed

HLREST: Machine learning tests leave ML state after they've finished #32993

nik9000 opened this issue Aug 20, 2018 · 5 comments
Assignees
Labels
:ml Machine learning >test-failure Triaged test failures from CI

Comments

@nik9000
Copy link
Member

nik9000 commented Aug 20, 2018

This build failed because the ML tests leave the cluster in a state where it recreates the .ml-notifcations. This interferes with other tests that expect an empty cluster. I think the right thing to do to fix this is to open source the ml state cleaner and move it into the test framework and use it in the rest client tests.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

nik9000 added a commit that referenced this issue Aug 20, 2018
It leaks state into other tests causing them to fail sometimes.

Relates to #32993
@dimitris-athanasiou
Copy link
Contributor

dimitris-athanasiou commented Aug 20, 2018

We've been aware we'd have to clean up. We were waiting to have the basic APIs to do so using the HLRC itself. We need #32960 first. Once that's in, I'll add the cleaner and re-enable the tests.

nik9000 added a commit that referenced this issue Aug 20, 2018
It leaks state into other tests causing them to fail sometimes.

Relates to #32993
@nik9000
Copy link
Member Author

nik9000 commented Aug 20, 2018

Awesome!

jasontedor pushed a commit that referenced this issue Aug 21, 2018
It leaks state into other tests causing them to fail sometimes.

Relates to #32993
@dimitris-athanasiou dimitris-athanasiou added the :ml Machine learning label Aug 21, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core

@dimitris-athanasiou dimitris-athanasiou self-assigned this Aug 21, 2018
@dimitris-athanasiou
Copy link
Contributor

dimitris-athanasiou commented Aug 21, 2018

I've been thinking about which is a better option: reuse MlRestTestStateCleaner.java from server or write a new cleaner in the client library. While it is nice to reuse the server cleaner, it will only be a temporary solution. We are heading towards a hard split between the client and the server side, so we will not be able to keep reusing it. However, it might be a good idea to do that at first as it allows us to implement the datafeed APIs while adding tests for them. Then, we can move back the MlRestTestStateCleaner and write a client one using the HLRC.

Actually, to avoid the churn in the server side, we can simply duplicate the MlRestTestStateCleaner temporarily, then remove it and introduce the HLRC cleaner. That way we don't churn at all the server side.

I'll go for this approach unless there is a strong downside I missed. Any thoughts @benwtrent , @davidkyle ?

dimitris-athanasiou added a commit to dimitris-athanasiou/elasticsearch that referenced this issue Aug 21, 2018
This commit duplicates the `MlRestTestStateCleaner` to make sure
all ML data is removed after each test. After implementing
the job and datafeed APIs in the HLRC, we shall replace this
implementation with one using the HLRC itself.

Closes elastic#32993
dimitris-athanasiou added a commit that referenced this issue Aug 21, 2018
This commit duplicates the `MlRestTestStateCleaner` to make sure
all ML data is removed after each test. After implementing
the job and datafeed APIs in the HLRC, we shall replace this
implementation with one using the HLRC itself.

Closes #32993
dimitris-athanasiou added a commit that referenced this issue Aug 21, 2018
This commit duplicates the `MlRestTestStateCleaner` to make sure
all ML data is removed after each test. After implementing
the job and datafeed APIs in the HLRC, we shall replace this
implementation with one using the HLRC itself.

Closes #32993
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:ml Machine learning >test-failure Triaged test failures from CI
Projects
None yet
Development

No branches or pull requests

3 participants