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

Use resident set memory for the etcd memory alert. #9997

Merged
merged 4 commits into from
Nov 25, 2024

Conversation

seizethedave
Copy link
Contributor

@seizethedave seizethedave commented Nov 22, 2024

What this PR does

This changes the EtcdAllocatingTooMuchMemory alerts to use RSS memory rather than working_set.
The reason for this is etcd can use a lot of file cache memory for its file-backed database that will be dropped if it approaches the container's memory limit. This causes this alert to basically be scare-noise, at times. We had a situation where the Go process only had 50MiB of memory resident, but there was 700MiB of file cache memory.

RSS fits better with "AllocatingTooMuch" anyway.

Which issue(s) this PR fixes or relates to

Fixes #

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • about-versioning.md updated with experimental features.

@seizethedave seizethedave changed the title Use resident memory for the etcd memory alert. Use resident set memory for the etcd memory alert. Nov 22, 2024
@seizethedave seizethedave marked this pull request as ready for review November 22, 2024 23:47
@seizethedave seizethedave requested a review from a team as a code owner November 22, 2024 23:47
Copy link
Contributor

@duricanikolic duricanikolic 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

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

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

makes sense, thanks

@duricanikolic duricanikolic merged commit acda41c into main Nov 25, 2024
31 checks passed
@duricanikolic duricanikolic deleted the davidgrant/etcd-monitor-memory branch November 25, 2024 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants