-
Notifications
You must be signed in to change notification settings - Fork 780
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
Make max outstanding queries per tenant config in limits #4991
Conversation
And since the query vertical sharding size is also per tenant, the queue size needs to be adjusted per tenant if we enable sharding for some tenants only. |
@@ -2724,6 +2726,11 @@ The `limits_config` configures default and per-tenant limits imposed by Cortex s | |||
# CLI flag: -frontend.max-queriers-per-tenant | |||
[max_queriers_per_tenant: <int> | default = 0] | |||
|
|||
# Maximum number of outstanding requests per tenant per request queue (either | |||
# query frontend or query scheduler); requests beyond this error with HTTP 429. | |||
# CLI flag: -frontend.max-outstanding-requests-per-tenant |
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.
Yeah.. i think make sense this limit be related to frontend
if we use or not the scheduler.
pkg/frontend/v1/frontend.go
Outdated
@@ -37,13 +37,25 @@ type Config struct { | |||
|
|||
// RegisterFlags adds the flags required to config this to the given FlagSet. | |||
func (cfg *Config) RegisterFlags(f *flag.FlagSet) { | |||
f.IntVar(&cfg.MaxOutstandingPerTenant, "querier.max-outstanding-requests-per-tenant", 100, "Maximum number of outstanding requests per tenant per frontend; requests beyond this error with HTTP 429.") | |||
f.IntVar(&cfg.MaxOutstandingPerTenant, "querier.max-outstanding-requests-per-tenant", 0, "Deprecated (user frontend.max-outstanding-requests-per-tenant instead): Maximum number of outstanding requests per tenant per frontend; requests beyond this error with HTTP 429.") |
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.
We need to start documenting when the deprecated flags will be removed (on which version)
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. PTAL
d0987a9
to
0f538b4
Compare
Signed-off-by: Ben Ye <benye@amazon.com>
9fa309b
to
6f6cfbb
Compare
Signed-off-by: Ben Ye <benye@amazon.com>
6f6cfbb
to
a96f70e
Compare
Thanks! LGTM |
Signed-off-by: Ben Ye benye@amazon.com
What this PR does:
Now the QFE queue size is a global configuration https://github.com/cortexproject/cortex/blob/master/pkg/frontend/v1/frontend.go#L40, which means we cannot override the queue size for different tenants.
For different tenants, we may want to use different queue sizes based on their query concurrency. Making it a per-tenant config in limits makes it more flexible and easier to update during runtime.
Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]