Skip to content

Resource monitor: fix bugs + add tests #6717

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

Merged
merged 4 commits into from
Apr 24, 2025

Conversation

justinjung04
Copy link
Contributor

@justinjung04 justinjung04 commented Apr 23, 2025

What this PR does:
Resource monitor introduced from #6674 contained bugs that were not covered in unit tests.

This PR fixes the bugs and add unit tests to cover them.

Double checked in the dev environment that resource monitor and resource based limiter work as expected:

Screenshot 2025-04-22 at 8 31 33 PM
ts=2025-04-23T03:29:39.25743507Z caller=ingester.go:2222 level=warn msg="failed to accept request" err="cpu utilization limit reached (limit: 0.020, utilization: 0.028)"

Which issue(s) this PR fixes:
n/a

Checklist

  • Tests updated
  • [n/a] Documentation added
  • [n/a] CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Signed-off-by: Justin Jung <jungjust@amazon.com>
@justinjung04 justinjung04 force-pushed the resource-monitor-bugfix branch from 78e3a99 to 146ff7c Compare April 23, 2025 02:58
Signed-off-by: Justin Jung <jungjust@amazon.com>
@justinjung04 justinjung04 marked this pull request as ready for review April 23, 2025 03:31
@justinjung04 justinjung04 changed the title Fix bugs + add tests Resource monitor: fix bugs + add tests Apr 23, 2025
@pull-request-size pull-request-size bot added size/L and removed size/M labels Apr 23, 2025
Signed-off-by: Justin Jung <jungjust@amazon.com>
@justinjung04 justinjung04 force-pushed the resource-monitor-bugfix branch 6 times, most recently from d77068f to f14b01a Compare April 23, 2025 17:19
Signed-off-by: Justin Jung <jungjust@amazon.com>
@justinjung04 justinjung04 force-pushed the resource-monitor-bugfix branch from f14b01a to 060ae2a Compare April 23, 2025 17:40
@justinjung04
Copy link
Contributor Author

@yeya24 can you take a second look?

Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

Thanks!

@yeya24 yeya24 merged commit fd9fc16 into cortexproject:master Apr 24, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants