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

[Task Manager] Removing perf_hooks from task manager #117294

Merged
merged 1 commit into from
Nov 3, 2021

Conversation

ymao1
Copy link
Contributor

@ymao1 ymao1 commented Nov 3, 2021

Resolves #116636

Summary

Removed perf_hooks from task manager. There were not used for anything and, as of Node 16.7, were potentially causing a memory leak because they were not being cleared.

@ymao1 ymao1 self-assigned this Nov 3, 2021
@ymao1 ymao1 added Feature:Task Manager release_note:skip Skip the PR/issue when compiling release notes v7.16.0 v8.0.0 v8.1.0 Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Nov 3, 2021
@ymao1 ymao1 marked this pull request as ready for review November 3, 2021 12:34
@ymao1 ymao1 requested a review from a team as a code owner November 3, 2021 12:34
@elasticmachine
Copy link
Contributor

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

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.

Nice clean nuking job! LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

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

cc @ymao1

@ymao1 ymao1 added the auto-backport Deprecated: Automatically backport this PR after it's merged label Nov 3, 2021
@ymao1 ymao1 merged commit 6cf9f8c into elastic:main Nov 3, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 3, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 3, 2021
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
8.0
7.16

The backport PRs will be merged automatically after passing CI.

@pmuellr
Copy link
Member

pmuellr commented Nov 3, 2021

Tested this PR branch with 200 rules at 1s, 100 workers, 1s polling, to try to force more tasks to be run. Too soon to tell for sure, but it appears that it's not leaking compared to a similar test with main. We'll be doing some days-long testing on this when it becomes available in BC3.

kibanamachine added a commit that referenced this pull request Nov 3, 2021
Co-authored-by: ymao1 <ying.mao@elastic.co>
kibanamachine added a commit that referenced this pull request Nov 3, 2021
Co-authored-by: ymao1 <ying.mao@elastic.co>
@ymao1 ymao1 deleted the task-manager/remove-perf-hooks branch January 27, 2022 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated: Automatically backport this PR after it's merged 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.16.0 v8.0.0 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[7.16.0 BC2] Potential memory leak observed - possibly caused by perf_hooks usage
4 participants