config: adds `frontend.max-query-capacity` to tune per-tenant query capacity #11284

ashwanthgoli · 2023-11-22T07:13:10Z

What this PR does / why we need it:
Adds a new config frontend.max-query-capacity that allows users to configure what portion of the the available querier replicas can be used by a tenant. max_query_capacity is the corresponding YAML option that can be configured in limits or runtime overrides.

For example, setting this to 0.5 would allow a tenant to use half of the available queriers.

This complements the existing frontend.max-queriers-per-tenant. When both are configured, the smaller value of the resulting querier replica count is considered:

min(frontend.max-queriers-per-tenant, ceil(querier_replicas * frontend.max-query-capacity))

All queriers will handle requests for a tenant if neither limits are applied.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:
noticed that we don't pass down the shuffle sharding limits for frontend (only using it with schedulers)

loki/pkg/loki/modules.go

Line 895 in 26f0971

disabledShuffleShardingLimits{},

but the docs mention thatfrontend.max-queriers-per-tenant applies to frontend as well.

This option only works with queriers connecting to the query-frontend / query-scheduler, not when using downstream URL.

Checklist

Reviewed the CONTRIBUTING.md guide (required)
Documentation added
Tests updated
CHANGELOG.md updated
- If the change is worth mentioning in the release notes, add add-to-release-notes label
Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR
If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

…enant

github-actions · 2023-11-22T07:15:08Z

Trivy scan found the following vulnerabilities:

HIGH openssl: Incorrect cipher key and IV length processing in libcrypto3 v3.1.3-r0. Fixed in v3.1.4-r0
HIGH openssl: Incorrect cipher key and IV length processing in libssl3 v3.1.3-r0. Fixed in v3.1.4-r0

JStickler

[docs team] Docs LGTM

dannykopping

Looks pretty good, but I think there's some refactoring that needs to be done for safety and clarity.

pkg/queue/tenant_queues.go

pkg/queue/tenant_queues_test.go

pkg/queue/queue.go

ashwanthgoli · 2023-11-24T12:23:57Z

pkg/scheduler/scheduler.go


 		pendingRequests:    map[requestKey]*schedulerRequest{},
 		connectedFrontends: map[string]*connectedFrontend{},
 		queueMetrics:       queueMetrics,
 		ringManager:        ringManager,
-		requestQueue:       queue.NewRequestQueue(cfg.MaxOutstandingPerTenant, cfg.QuerierForgetDelay, queueMetrics),
+		requestQueue:       queue.NewRequestQueue(cfg.MaxOutstandingPerTenant, cfg.QuerierForgetDelay, limits.QueueLimits(schedulerLimits), queueMetrics),


this would look neater if we move it to the queue pkg queue.Limits(frontendLimits)
but I left it in scheduler pkg since it is an adapter between scheduler and queue limits.

dannykopping

Thanks for the refactor! It reads much more clearly now.
There were a couple troubling things that caught my eye though with the changes, but they shouldn't take long to resolve

pkg/queue/tenant_queues.go

pkg/scheduler/limits/definitions.go

pkg/scheduler/limits/definitions_test.go

dannykopping

Looking much cleaner now, thanks @ashwanthgoli!
Just minor nits now

docs/sources/configure/_index.md

pkg/scheduler/limits/definitions.go

pkg/validation/limits.go

Co-authored-by: Danny Kopping <danny.kopping@grafana.com>

dannykopping

LGTM!

…apacity (grafana#11284) **What this PR does / why we need it**: Adds a new config `frontend.max-query-capacity` that allows users to configure what portion of the the available querier replicas can be used by a tenant. `max_query_capacity` is the corresponding YAML option that can be configured in limits or runtime overrides. For example, setting this to 0.5 would allow a tenant to use half of the available queriers. This complements the existing `frontend.max-queriers-per-tenant`. When both are configured, the smaller value of the resulting querier replica count is considered: ``` min(frontend.max-queriers-per-tenant, ceil(querier_replicas * frontend.max-query-capacity)) ``` *All* queriers will handle requests for a tenant if neither limits are applied. **Which issue(s) this PR fixes**: Fixes #<issue number> **Special notes for your reviewer**: noticed that we don't pass down the shuffle sharding limits for frontend (only using it with schedulers) https://github.com/grafana/loki/blob/26f097162a856db48ecbd16bef2f0b750029855b/pkg/loki/modules.go#L895 but the [docs](https://github.com/grafana/loki/blob/26f097162a856db48ecbd16bef2f0b750029855b/pkg/validation/limits.go#L276) mention that`frontend.max-queriers-per-tenant` applies to frontend as well. ``` This option only works with queriers connecting to the query-frontend / query-scheduler, not when using downstream URL. ``` **Checklist** - [x] Reviewed the [`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md) guide (**required**) - [x] Documentation added - [x] Tests updated - [x] `CHANGELOG.md` updated - [ ] If the change is worth mentioning in the release notes, add `add-to-release-notes` label - [ ] Changes that require user attention or interaction to upgrade are documented in `docs/sources/setup/upgrade/_index.md` - [ ] For Helm chart changes bump the Helm chart version in `production/helm/loki/Chart.yaml` and update `production/helm/loki/CHANGELOG.md` and `production/helm/loki/README.md`. [Example PR](grafana@d10549e) - [ ] If the change is deprecating or removing a configuration option, update the `deprecated-config.yaml` and `deleted-config.yaml` files respectively in the `tools/deprecated-config-checker` directory. [Example PR](grafana@0d4416a) --------- Co-authored-by: J Stickler <julie.stickler@grafana.com> Co-authored-by: Danny Kopping <danny.kopping@grafana.com>

config: adds frontend.max-query-capacity to tune query capacity per t…

2d8f59e

…enant

pull-request-size bot added the size/L label Nov 22, 2023

add changelog

26f0971

github-actions bot added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Nov 22, 2023

ashwanthgoli marked this pull request as ready for review November 22, 2023 07:21

ashwanthgoli requested a review from a team as a code owner November 22, 2023 07:21

Merge branch 'main' into ashwath/add-query-capacity-config

a841c5d

JStickler reviewed Nov 22, 2023

View reviewed changes

dannykopping reviewed Nov 23, 2023

View reviewed changes

pkg/queue/tenant_queues.go Outdated Show resolved Hide resolved

pkg/queue/tenant_queues_test.go Outdated Show resolved Hide resolved

pkg/queue/queue.go Outdated Show resolved Hide resolved

refactor to avoid passing limit options to Enqueue call

ba7f680

pull-request-size bot added size/XL and removed size/L labels Nov 24, 2023

ashwanthgoli commented Nov 24, 2023

View reviewed changes

Merge branch 'main' into ashwath/add-query-capacity-config

0c75505

dannykopping reviewed Nov 24, 2023

View reviewed changes

ashwanthgoli added 3 commits November 24, 2023 19:18

update QueueLimits to conform with other limits conventions

69a7ea1

fix lint, check-quality

0109a2e

Merge branch 'main' into ashwath/add-query-capacity-config

bc7b3bc

dannykopping reviewed Nov 28, 2023

View reviewed changes

docs/sources/configure/_index.md Outdated Show resolved Hide resolved

docs/sources/configure/_index.md Outdated Show resolved Hide resolved

pkg/scheduler/limits/definitions.go Show resolved Hide resolved

pkg/validation/limits.go Show resolved Hide resolved

ashwanthgoli and others added 6 commits November 28, 2023 13:01

review suggestions

d38f12e

Update docs/sources/configure/_index.md

f19bef0

Co-authored-by: Danny Kopping <danny.kopping@grafana.com>

Update docs/sources/configure/_index.md

812019c

Co-authored-by: Danny Kopping <danny.kopping@grafana.com>

Merge branch 'main' into ashwath/add-query-capacity-config

3960c49

fixup! review suggestions

9fb23b1

remove \n from flag usage

10fd6ba

dannykopping approved these changes Dec 1, 2023

View reviewed changes

ashwanthgoli merged commit d9f3bf3 into main Dec 1, 2023
8 checks passed

ashwanthgoli deleted the ashwath/add-query-capacity-config branch December 1, 2023 14:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

config: adds `frontend.max-query-capacity` to tune per-tenant query capacity #11284

config: adds `frontend.max-query-capacity` to tune per-tenant query capacity #11284

ashwanthgoli commented Nov 22, 2023 •

edited

github-actions bot commented Nov 22, 2023 •

edited

JStickler left a comment

dannykopping left a comment

ashwanthgoli Nov 24, 2023

dannykopping left a comment

dannykopping left a comment

dannykopping left a comment

config: adds frontend.max-query-capacity to tune per-tenant query capacity #11284

config: adds frontend.max-query-capacity to tune per-tenant query capacity #11284

Conversation

ashwanthgoli commented Nov 22, 2023 • edited

github-actions bot commented Nov 22, 2023 • edited

JStickler left a comment

Choose a reason for hiding this comment

dannykopping left a comment

Choose a reason for hiding this comment

ashwanthgoli Nov 24, 2023

Choose a reason for hiding this comment

dannykopping left a comment

Choose a reason for hiding this comment

dannykopping left a comment

Choose a reason for hiding this comment

dannykopping left a comment

Choose a reason for hiding this comment

config: adds `frontend.max-query-capacity` to tune per-tenant query capacity #11284

config: adds `frontend.max-query-capacity` to tune per-tenant query capacity #11284

ashwanthgoli commented Nov 22, 2023 •

edited

github-actions bot commented Nov 22, 2023 •

edited