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.

Sign up for GitHub

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

Jump to bottom

Remove CortexQuerierCapacityFull alert #342

Merged
merged 1 commit into from
Jul 2, 2021
Merged

Remove CortexQuerierCapacityFull alert #342

merged 1 commit into from
Jul 2, 2021

Conversation

pracucci
Copy link
Collaborator

@pracucci pracucci commented Jul 1, 2021 •
edited
Loading

What this PR does:
I propose to remove CortexQuerierCapacityFull alert. It was added as part of the initial commit.

Why removing it? My thoughts:

  • The alert fires when a single querier is running queries at full capacity (eg. if configured with 4 workers, all workers run queries for 5 consecutive minutes)
  • Having a querier running a full capacity is not an issue per se (if we give a querier 4 workers it means we're willing to use all 4 of them if required) and this looks more alerting on cause then symptom
  • We already have alerts on queries stuck in query-frontend/scheduler and we have SLO-based alerts, so I'm not really sure this alert is giving us a real value

Which issue(s) this PR fixes:
N/A

Checklist

  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@pracucci
Remove CortexQuerierCapacityFull alert
af74202 
Signed-off-by: Marco Pracucci <marco@pracucci.com>
@pracucci pracucci requested a review from a team as a code owner July 1, 2021 14:43
gouthamve
gouthamve approved these changes Jul 1, 2021
View reviewed changes
Copy link
Member

@gouthamve gouthamve left a comment

Choose a reason for hiding this comment

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

LGTM

@pracucci
Copy link
Collaborator Author

pracucci commented Jul 1, 2021

I'm not going to merge it immediately. I want to give other people the time to review and think about it.

pstibrany
pstibrany approved these changes Jul 1, 2021
View reviewed changes
Copy link
Member

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

Your reasoning makes sense to me. 👍

@pracucci
Copy link
Collaborator Author

pracucci commented Jul 2, 2021 •
edited
Loading

Going to merge this. I will promptly address any post-merge feedback (if any).

@pracucci pracucci merged commit b266d7b into main Jul 2, 2021
@pracucci pracucci deleted the remove-CortexQuerierCapacityFull-alert branch July 2, 2021 12:24
simonswine pushed a commit to grafana/mimir that referenced this pull request Oct 18, 2021
@pracucci
Merge pull request grafana/cortex-jsonnet#342 from grafana/remove-Cor…
dec2b14 
…texQuerierCapacityFull-alert

Remove CortexQuerierCapacityFull alert
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pstibrany pstibrany pstibrany approved these changes

@gouthamve gouthamve gouthamve approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@pracucci @pstibrany @gouthamve