-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Remove task to cleanup action_task_params of failed tasks #151873
Conversation
Pinging @elastic/response-ops (Team:ResponseOps) |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
…ana into actions/remove-cleanup-task
Moving PR back to draft, the previous PR (#152841) caused a lot of flaky tests that I will focus on first. |
This reverts commit cfcbd61.
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Unknown metric groupsESLint disabled line counts
References to deprecated APIs
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @mikecote |
Reviewed the code, looked good, so tried a migration from 8.2.3 to this PR. Weirdly, it looks like it migrated task manager in place - I know that's a new thing now, but since we did have migrations between then - adding the Here's
So, the odd-ball thing is that the old task that SHOULD HAVE been deleted, is still there. With an Thinking we should have someone else repro this, and if it's not me "holding it wrong", open an issue to get the "removed tasks" bit working again ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code LGTM; Kibana survived a migration from 8.2.3, but I left a comment in the PR because it turns out it didn't actually delete the old task (which I was expecting) - not a bug in this code though, so approving :-)
Reading through the "REMOVED_TYPES" processing in task manager, it looks like we don't actually delete the docs, we just don't search for them anymore. So I guess, working-as-designed? Seems weird to have enabled task docs in the index, that won't be read, based on a list of task types in the code. I wonder whether we should somehow make more explicit in the docs themselves, that they are no longer in use, if we're going to keep them around. |
Good observation, @pmuellr! I created #153964 to discuss the oddity now that we are using |
…1873) Part of elastic#79977 (step 2). Resolves elastic#79977. In this PR, I'm removing the recurring task defined by the actions plugin that removes unused `action_task_params` SOs. With the elastic#152841 PR, tasks will no longer get marked as failed and we have a migration script (`excludeOnUpgrade`) that removes all tasks and action_task_params that are leftover during the migration https://github.com/elastic/kibana/blob/main/x-pack/plugins/actions/server/saved_objects/index.ts#L81-L94. ~~NOTE: I will hold off merging this PR until elastic#152841 is merged.~~ (merged) ## To verify Not much to test here, but on a Kibana from `main` there will be this task type running in the background and moving to this PR will cause the task to get deleted because it is part of the `REMOVED_TYPES` array in Task Manager. --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Part of #79977 (step 2).
Resolves #79977.
In this PR, I'm removing the recurring task defined by the actions plugin that removes unused
action_task_params
SOs. With the #152841 PR, tasks will no longer get marked as failed and we have a migration script (excludeOnUpgrade
) that removes all tasks and action_task_params that are leftover during the migration https://github.com/elastic/kibana/blob/main/x-pack/plugins/actions/server/saved_objects/index.ts#L81-L94.NOTE: I will hold off merging this PR until #152841 is merged.(merged)To verify
Not much to test here, but on a Kibana from
main
there will be this task type running in the background and moving to this PR will cause the task to get deleted because it is part of theREMOVED_TYPES
array in Task Manager.