Skip to content

Enforced -querier.max-query-lookback in the query-frontend for range queries #3458

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

Conversation

pracucci
Copy link
Contributor

@pracucci pracucci commented Nov 4, 2020

What this PR does:
In the PR #3452 I've introduced -querier.max-query-lookback but I've intentionally skipped enforcing the limit in the query-frontend, in order to keep the change set smaller. In this PR I'm enforcing it in the query-frontend too, for range queries.

The main benefit of enforcing it in the query-frontend too is to reduce the number of query range requests enqueued for a tenant, if such queries cover a range outside the max lookback. Without this PR, the limit would be enforced anyway in the querier, but we may end up with a large number of splitted queries in the query-frontend/query-scheduler queue which would be noop once executed in the querier.

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

Checklist

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

Copy link
Contributor

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

Nice. I especially like how you always leave the code in a better shape with each PR!

…queries

Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
@pracucci
Copy link
Contributor Author

pracucci commented Nov 4, 2020

Thanks @pstibrany for your thoughtful review. I've addressed all comments!

@pracucci pracucci force-pushed the enforce-max-query-lookback-in-query-frontend branch from aa3ab21 to 254d437 Compare November 4, 2020 16:31
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.

I don't know whether I can approve or not, but this looks nice!

@pracucci
Copy link
Contributor Author

pracucci commented Nov 4, 2020

I don't know whether I can approve or not, but this looks nice!

Sure you can!

@pracucci pracucci merged commit 9084659 into cortexproject:master Nov 6, 2020
@pracucci pracucci deleted the enforce-max-query-lookback-in-query-frontend branch November 6, 2020 07:39
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.

4 participants