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

Add Alert ThanosQueryOverload to Mixin #5439

Merged
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

@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
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
@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? 🙏

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

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:

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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants