Skip to content

Conversation

@mikecote
Copy link
Contributor

@mikecote mikecote commented Apr 13, 2020

This PR fixes task manager's InactiveTasks query to AND the filters in the must_not instead of OR which caused tasks with status running or claiming to never return.

Steps to reproduce issue:

  • Stop kibana while an alert executor is running (use timeouts and logs to tell you when to stop Kibana)
  • Restart kibana and lookup the alert task's retryAt
  • Wait for retryAt window to pass
  • Notice alert isn't executing anymore

@mikecote mikecote added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Task Manager v7.7.0 Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v7.8.0 labels Apr 13, 2020
@mikecote mikecote requested a review from a team as a code owner April 13, 2020 15:20
@mikecote mikecote self-assigned this Apr 13, 2020
@elasticmachine
Copy link
Contributor

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


export const InactiveTasks: MustNotCondition<TermFilter | RangeFilter> = {
// TODO: Fix query clauses to support this
export const InactiveTasks: BoolClauseWithAnyCondition<any> = {
Copy link
Contributor Author

@mikecote mikecote Apr 13, 2020

Choose a reason for hiding this comment

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

I did not manage to get query clause TS types working within a timebox, deferring to later.

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
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@mikecote mikecote merged commit 0666dbd into elastic:master Apr 13, 2020
mikecote added a commit to mikecote/kibana that referenced this pull request Apr 13, 2020
* Fix task manager query to also return tasks to retry

* Fix failing jest tests
mikecote added a commit to mikecote/kibana that referenced this pull request Apr 13, 2020
* Fix task manager query to also return tasks to retry

* Fix failing jest tests
mikecote added a commit that referenced this pull request Apr 13, 2020
* Fix task manager query to also return tasks to retry

* Fix failing jest tests
mikecote added a commit that referenced this pull request Apr 13, 2020
* Fix task manager query to also return tasks to retry

* Fix failing jest tests
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 14, 2020
* master: (132 commits)
  document code splitting for client code (elastic#62593)
  Escape single quotes surrounded by double quotes (elastic#63229)
  [Endpoint] Update cli mapping to match endpoint package (elastic#63372)
  update in-app links to metricbeat configuration docs (elastic#63295)
  investigation notes field (documentation / metadata) (elastic#63386)
  [Maps] fix bug where toggling Scaling type does not re-fetch data (elastic#63326)
  [Alerting] set correct parameter for unauthented email action (elastic#63086)
  [Telemetry] force staging urls in tests (elastic#63356)
  Migrate legacy maps service to NP & update refs (elastic#60942)
  Fix task manager query to return tasks to retry (elastic#63360)
  [Endpoint] Policy list support for URL pagination state (elastic#63291)
  [Canvas] Migrate saved object mappings and migrations to Kibana Platform (elastic#58891)
  [DOCS] Add ILM tutorial (elastic#59502)
  [Maps] Add SOURCE_TYPES enumeration (elastic#62975)
  [Maps] update geospatial filters to use geo_shape query for geo_point fields (elastic#62966)
  Move away from npStart for embeddables in canvas (elastic#62680)
  Use MapInput type from Maps plugin (elastic#61539)
  Update to pagination for workpad and templates (elastic#62050)
  [SIEM] Fix AlertsTable id (elastic#63368)
  Consistent terminology around cypress test data (elastic#63279)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 14, 2020
* master:
  document code splitting for client code (elastic#62593)
  Escape single quotes surrounded by double quotes (elastic#63229)
  [Endpoint] Update cli mapping to match endpoint package (elastic#63372)
  update in-app links to metricbeat configuration docs (elastic#63295)
  investigation notes field (documentation / metadata) (elastic#63386)
  [Maps] fix bug where toggling Scaling type does not re-fetch data (elastic#63326)
  [Alerting] set correct parameter for unauthented email action (elastic#63086)
  [Telemetry] force staging urls in tests (elastic#63356)
  Migrate legacy maps service to NP & update refs (elastic#60942)
  Fix task manager query to return tasks to retry (elastic#63360)
  [Endpoint] Policy list support for URL pagination state (elastic#63291)
  [Canvas] Migrate saved object mappings and migrations to Kibana Platform (elastic#58891)
  [DOCS] Add ILM tutorial (elastic#59502)
  [Maps] Add SOURCE_TYPES enumeration (elastic#62975)
  [Maps] update geospatial filters to use geo_shape query for geo_point fields (elastic#62966)
  Move away from npStart for embeddables in canvas (elastic#62680)
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.

Given this a look over too and looks right.
I wish we could add a FT around this, but looking at the implementation it's actually quite hard because you can't have an FT run for over 5 minutes to get to the retry.

We might have to manually create a task who's retryAt is in the past through some hacky means 🤔
I'll give it some thought.

wayneseymour pushed a commit that referenced this pull request Apr 15, 2020
* Fix task manager query to also return tasks to retry

* Fix failing jest tests
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 Platform ResponseOps team (formerly the Cases and Alerting teams) t// v7.7.0 v7.8.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants