[Transform] Make force-stopping the transform always remove persistent task from cluster state #106989
Conversation
This file contains 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
przemekwitek
force-pushed
the
fix_transform_force_stop
branch
6 times, most recently
from
770d234 to
7af527f
Compare
…t task from cluster state
przemekwitek
force-pushed
the
fix_transform_force_stop
branch
from
7af527f to
db7397f
Compare
|
Pinging @elastic/ml-core (Team:ML) |
|
Hi @przemekwitek, I've created a changelog YAML for you. |
prwhelan
approved these changes
Apr 9, 2024
Comment on lines
-251
to
-253
| transformTask.shutdown(); | ||
| // Here the indexer is aborted so that its thread finishes work ASAP. | ||
| transformTask.onCancelled(); |
There was a problem hiding this comment.
These are no longer needed because persistentTasksService.sendRemoveRequest will invoke them, is that correct?
There was a problem hiding this comment.
Yes, the sequence of events is as follows:
persistentTasksService.sendRemoveRequestremoves the locally running task and notifiesPersistentTasksClusterServiceto remove the task from cluster state.- when the cluster state changes,
PersistentTaskNodeServiceis notified viaclusterChangedmethod PersistentTaskNodeServicecancels the taskTransformTask.onCancelledmethod is called and this method callsshutdown
3 tasks
walterra
added a commit
to elastic/kibana
that referenced
this pull request
Apr 11, 2024
## Summary Fixes #180503 Fixes #180499 Fixes #180495 Fixes #180496 Fixes #180497 Fixes #180504 The tests themselves were passing, but the cleanup in the `after` blocks turned out problematic after a [recent update in ES related to stopping transforms](elastic/elasticsearch#106989) using `force`. Originally we called `stop` on all test transforms and then called the helper function `.cleanTransformIndices();`. For some time now this helper was updated to first both force stop and delete transforms before deleting related indices. So we ended up calling stop twice in some tests! The second stop using force could then return an error if the transform was already stopped. This PR fixes the tests by removing the now unnecessary stop commands. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Currently, if, for any reason, persistent transform task removal cannot commit to cluster state, the leftover persistent task entry will stay forever in the cluster state.
This PR fixes the
_stop/forceAPI so that persistent transform task removal is possible in such a case.Rather than relying on
taskOperation()method (which might not be called if the task is in this problematic state), this PR makes the force-stop action remove the persistent task as soon as possible in thedoExecutemethod (before the delegation totaskOperation()method).Fixes #106811