[ML] Fix simultaneous stop and force stop datafeed#49367
Merged
droberts195 merged 1 commit intoelastic:masterfrom Nov 20, 2019
Merged
[ML] Fix simultaneous stop and force stop datafeed#49367droberts195 merged 1 commit intoelastic:masterfrom
droberts195 merged 1 commit intoelastic:masterfrom
Conversation
If a datafeed is stopped normally and force stopped at the same time then it is possible that the force stop removes the persistent task while the normal stop is performing actions. Currently this causes the normal stop to error, but since stopping a stopped datafeed is not an error this doesn't make sense. Instead the force stop should just take precedence. This is a followup to elastic#49191 and should really have been included in the changes in that PR.
Collaborator
|
Pinging @elastic/ml-core (:ml) |
Author
|
Existing tests already cover the scenario that is fixed here. It was one of the backport PRs of #49191 that found the problem - the relevant Gradle scan is https://gradle-enterprise.elastic.co/s/lrofxnw6rotco and that came from a PR build of #49290. For 7.5 and 6.8 I will incorporate the changes of this PR into the (still open) backport PRs of #49191. |
droberts195
added a commit
that referenced
this pull request
Nov 20, 2019
If a datafeed is stopped normally and force stopped at the same time then it is possible that the force stop removes the persistent task while the normal stop is performing actions. Currently this causes the normal stop to error, but since stopping a stopped datafeed is not an error this doesn't make sense. Instead the force stop should just take precedence. This is a followup to #49191 and should really have been included in the changes in that PR.
Author
droberts195
added a commit
to droberts195/elasticsearch
that referenced
this pull request
Nov 22, 2019
The problem reported in elastic#44566 should be fixed by the change that was made in elastic#49367, so the muted test can be unmuted. Closes elastic#44566
droberts195
added a commit
that referenced
this pull request
Nov 22, 2019
droberts195
added a commit
to droberts195/elasticsearch
that referenced
this pull request
Jul 19, 2021
A check for this situation when closing multiple jobs or stopping multiple datafeeds was added in elastic#38113. Later the wildcard requirement was removed for jobs in elastic#49367. But the wildcard requirement really should have been removed for both jobs and datafeeds, as they both having the same semantics that closing or stopping one that's already closed/stopped is not an error. This commit removes the wildcard condition for jobs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If a datafeed is stopped normally and force stopped at the same
time then it is possible that the force stop removes the
persistent task while the normal stop is performing actions.
Currently this causes the normal stop to error, but since
stopping a stopped datafeed is not an error this doesn't make
sense. Instead the force stop should just take precedence.
This is a followup to #49191 and should really have been
included in the changes in that PR.