Skip to content
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

Prevent kibana crashing when multiple processes start APM telemetry task #87645

Merged
merged 2 commits into from
Jan 11, 2021

Conversation

rudolf
Copy link
Contributor

@rudolf rudolf commented Jan 7, 2021

Summary

When multiple Kibana process are started in parallel they all attempt to start the apm-telemetry-task task in parallel. Because the taskManagerStart.runNow promise wasn't awaited it would sometimes cause an unhandled promise rejection when another process already started this task and crash Kibana.

Release notes

Fixed a bug that would sometimes cause the Kibana process to crash with Error: Failed to run task "apm-telemetry-task" as it is currently running.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@rudolf rudolf added bug Fixes for quality problems that affect the customer experience v7.11.0 v7.12 v8.0.0 release_note:fix labels Jan 7, 2021
@rudolf rudolf marked this pull request as ready for review January 7, 2021 15:11
@rudolf rudolf requested a review from a team as a code owner January 7, 2021 15:11
Copy link
Member

@dgieselaar dgieselaar left a comment

Choose a reason for hiding this comment

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

LGTM - why does runNow throw if another process already started this task though?

@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Jan 7, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

@rudolf
Copy link
Contributor Author

rudolf commented Jan 7, 2021

LGTM - why does runNow throw if another process already started this task though?

Good question. @elastic/kibana-alerting-services @gmmorris

@timroes timroes added v7.12.0 and removed v7.12 labels Jan 11, 2021
@rudolf
Copy link
Contributor Author

rudolf commented Jan 11, 2021

@elasticmachine merge upstream

@mikecote
Copy link
Contributor

LGTM - why does runNow throw if another process already started this task though?

Good question. @elastic/kibana-alerting-services @gmmorris

Based on this PR, the runNow API returns a promise that identifies if the task completed successfully or not (including error messages, etc). In the scenario the task is currently running (possibly by another Kibana instance), the API will throw since it can't capture the outcome. (@gmmorris keep me honest).

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@rudolf rudolf merged commit f8b1cdd into elastic:master Jan 11, 2021
@rudolf rudolf deleted the fix-apm-telemetry-task branch January 11, 2021 20:39
rudolf added a commit to rudolf/kibana that referenced this pull request Jan 11, 2021
rudolf added a commit to rudolf/kibana that referenced this pull request Jan 11, 2021
rudolf added a commit that referenced this pull request Jan 12, 2021
…ask in parallel (#87645) (#87923)

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
rudolf added a commit that referenced this pull request Jan 12, 2021
…ask in parallel (#87645) (#87924)

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience release_note:fix Team:APM All issues that need APM UI Team support v7.11.0 v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants