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

Add Alert ThanosQueryOverload to Mixin #5439

Merged
merged 1 commit into from
Aug 9, 2022
Merged

Add Alert ThanosQueryOverload to Mixin #5439

merged 1 commit into from
Aug 9, 2022

Conversation

raptorsun
Copy link
Contributor

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

This PR adds an alert ThanosQueryOverload to the mixins.

This alert warns the user when Thanos' query API is saturated by concurrent queries so that the user can take measurements before functionalities relying on Thanos stops working.

Verification

@raptorsun raptorsun force-pushed the alertThanosQueryOverload branch 2 times, most recently from c2a3171 to 748d572 Compare June 23, 2022 14:11
@raptorsun raptorsun mentioned this pull request Jun 23, 2022
Closed
2 tasks
@raptorsun raptorsun force-pushed the alertThanosQueryOverload branch from 748d572 to cf9baa3 Compare June 23, 2022 15:05
@fpetkovski
Copy link
Contributor

Having this on a dashboard would also be great 👍

@raptorsun
Copy link
Contributor Author

Having this on a dashboard would also be great +1

Good idea, I'm going to add a "Thanos Query Concurrent Capacity" dashboard with formula max_over_time(thanos_query_concurrent_gate_queries_max[5m]) - avg_over_time(thanos_query_concurrent_gate_queries_in_flight[5m])

bwplotka
bwplotka previously approved these changes Jul 5, 2022
View reviewed changes
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Thanks!

kakkoyun
kakkoyun previously approved these changes Jul 11, 2022
View reviewed changes
@kakkoyun kakkoyun dismissed stale reviews from bwplotka and themself via c51e9b8 July 11, 2022 06:48
@kakkoyun kakkoyun enabled auto-merge July 11, 2022 06:48
auto-merge was automatically disabled July 25, 2022 09:59

Head branch was pushed to by a user without write access

@raptorsun raptorsun force-pushed the alertThanosQueryOverload branch 2 times, most recently from 8b12068 to 8abb224 Compare July 25, 2022 14:19
@raptorsun
Copy link
Contributor Author

A PR is created to add the dashboard showing available capacity to serve concurrent queries.

@douglascamata
Copy link
Contributor

Hey @raptorsun, can you have a look at failed checks so that we can proceed with merging your contribution, please? 🙏

@raptorsun raptorsun force-pushed the alertThanosQueryOverload branch from 8abb224 to 686c1db Compare August 4, 2022 13:33
@douglascamata
Copy link
Contributor

This seems to be breaking some tests and I don't understand why. 🤔

Does someone have ideas? cc @bwplotka, @kakkoyun, @fpetkovski

@raptorsun raptorsun force-pushed the alertThanosQueryOverload branch from 686c1db to 26f68d5 Compare August 8, 2022 11:02
@raptorsun raptorsun force-pushed the alertThanosQueryOverload branch from 26f68d5 to b285221 Compare August 8, 2022 21:02
@raptorsun
Copy link
Contributor Author

This seems to be breaking some tests and I don't understand why. 🤔

Does someone have ideas? cc @bwplotka, @kakkoyun, @fpetkovski

The broken test has been fixed, just need to add a stub rule in the testing code.

douglascamata
douglascamata approved these changes Aug 9, 2022
View reviewed changes
Copy link
Contributor

@douglascamata douglascamata left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

kakkoyun
kakkoyun approved these changes Aug 9, 2022
View reviewed changes
Copy link
Member

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@kakkoyun kakkoyun merged commit 6d1b98d into thanos-io:main Aug 9, 2022
@raptorsun raptorsun deleted the alertThanosQueryOverload branch December 7, 2022 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kakkoyun kakkoyun kakkoyun approved these changes

@douglascamata douglascamata douglascamata approved these changes

@bwplotka bwplotka bwplotka left review comments

Assignees
No one assigned
Labels
size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@raptorsun @fpetkovski @douglascamata @kakkoyun @bwplotka