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

Mitigate memory leaks from long RequeueAfter periods #1989

Merged
merged 2 commits into from
Oct 14, 2019

Conversation

sebgl
Copy link
Contributor

@sebgl sebgl commented Oct 14, 2019

Fixes #1984.

We have an issue where the underlying timer used by client-go worker
queue implementation stays in memory until it expires.
Since one gets created at every reconciliation attempt, we end up with a
big bunch of timers in memory that will expire in 365 days by default.

To mitigate the memory leak, let's wait for no more than 10 hours to
reconcile. This does not prevent timers to leak, but restricts it to the next
10 hours.

This is done at the level of the aggregated results, to decouple this
workaround from any business logic like certs expiration. As opposed
to #1988 (closed).

The PR also refactors slightly the license controller to rely on the aggregated
Results structure. Hence benefit from the max 10 hour restriction.

We have an issue where the underlying timer used by client-go worker
queue implementation stays in memory until it expires.
Since one gets created at every reconciliation attempt, we end up with a
big bunch of timers in memory that will expire in 365 days by default.

To mitigate the memory leak, let's wait for no more than 10 hours to
reconcile.

This is done at the level of the aggregated results, to decouple this
wokaround from any business logic like certs expiration.

For more details, see
elastic#1984.
@sebgl sebgl added the >bug Something isn't working label Oct 14, 2019
@sebgl
Copy link
Contributor Author

sebgl commented Oct 14, 2019

I'm setting up a long-running ECK instance with metricsbeat monitoring enabled to monitor memory usage.

Copy link
Contributor

@barkbay barkbay left a comment

Choose a reason for hiding this comment

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

LGTM

@sebgl sebgl merged commit 6e6118b into elastic:master Oct 14, 2019
@pebrc pebrc added the v1.0.0 label Oct 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug Something isn't working v1.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potential memory leak in the operator
3 participants