Skip to content

[Task Manager] Skip removed task types when claiming tasks #84273

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

Merged
merged 6 commits into from
Dec 2, 2020

Conversation

ymao1
Copy link
Contributor

@ymao1 ymao1 commented Nov 24, 2020

Resolves #84088

Summary

Adds a check in the update by query painless script to ensure the task type is in the list of currently registered task types to ensure we're only trying to claim (or mark as failed) current task types.

Checklist

Delete any items that are not applicable to this PR.

@ymao1 ymao1 changed the title Checking if task type is in registered list [Task Manager] Skip removed task types when claiming tasks Nov 24, 2020
@ymao1 ymao1 added Feature:Task Manager release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.11.0 v8.0.0 labels Nov 30, 2020
@ymao1 ymao1 self-assigned this Nov 30, 2020
@ymao1 ymao1 marked this pull request as ready for review November 30, 2020 18:22
@ymao1 ymao1 requested a review from a team as a code owner November 30, 2020 18:22
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@gmmorris gmmorris left a comment

Choose a reason for hiding this comment

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

LGTM, looks like it's working as expected. :)

My two notes though are:

  1. Can we reduce the size of mappings.json?
  2. Is it worth marking the removed tasks as failed (or even a new status?)? otherwise they'll remain in 'idle` forever. This isn't a must, but seems like a nice tweak while we're in here.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@ymao1 ymao1 merged commit cbc61af into elastic:master Dec 2, 2020
ymao1 added a commit to ymao1/kibana that referenced this pull request Dec 2, 2020
…4273)

* Checking if task type is in registered list

* Loading esArchiver data with removed task type for testing

* PR fixes
ymao1 added a commit that referenced this pull request Dec 2, 2020
…84794)

* Checking if task type is in registered list

* Loading esArchiver data with removed task type for testing

* PR fixes
@ymao1 ymao1 deleted the task-manager/removed-types branch February 4, 2021 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Task Manager release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tasks of removed types cause script errors
5 participants