-
Notifications
You must be signed in to change notification settings - Fork 526
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 MaxTotalQueryLength and fill it from MaxQueryLength if unset #3058
Conversation
5a424d3
to
bb951cb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice job! much cleaner now 👏 👏 👏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! I definitely see this need (see #2793), but please see my comments before merging. I think there are couple of things to improve / fix.
|
||
Both PromQL instant and range queries can fetch metrics data over a period of time. | ||
A [range query](https://prometheus.io/docs/prometheus/latest/querying/api/#range-queries) requires a `start` and `end` timestamp, so the difference of `end` minus `start` is the time range length of the query. | ||
An [instant query](https://prometheus.io/docs/prometheus/latest/querying/api/#instant-queries) requires a `time` parameter and the query is executed fetching samples at that point in time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new limit (and so error) applies only to range queries, isn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes , I quickly tested in a local mimir install. Updating docs one more time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated docs, please re-review the doc parts + changelog, thank you
# This is the 1st commit message: Add MaxTotalQueryLength and fill it from MaxQueryLength if 0 Add the ability to define a higher total query length limit on the frontend , compared to the current shared limit between querier and query-frontend. Ref: grafana/mimir-squad#889 Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> # Conflicts: # CHANGELOG.md # This is the commit message #2: Update runbook # This is the commit message #3: Update pkg/util/validation/limits.go Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com> # This is the commit message #4: Rename store.max-total-query-length to query-frontend... Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Co-authored-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
c0bbc9c
to
bebfc0b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! LGTM 👏
Installed and tested the final version locally with helm. Merging now. |
What this PR does
Add the ability to define a higher total query length limit on the
frontend , compared to the current shared limit between querier and
query-frontend.
Keep backwards compatibility.
Which issue(s) this PR fixes or relates to
Fixes https://github.com/grafana/mimir-squad/issues/889
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]