Skip to content

Conversation

SungJin1212
Copy link
Member

@SungJin1212 SungJin1212 commented Dec 20, 2024

This PR adds a -tenant-federation.max-concurrent flags to make the number of workers processing federated query configurable. and add a cortex_querier_federated_tenants_per_query histogram to track the number of tenants per query.

Which issue(s) this PR fixes:
Fixes #

Checklist

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

@dosubot dosubot bot added component/querier type/observability To help know what is going on inside Cortex labels Dec 20, 2024
@SungJin1212 SungJin1212 force-pushed the Add-number-of-tenants-per-query-histogram branch from d5beaf8 to c124391 Compare December 20, 2024 10:07
@yeya24
Copy link
Contributor

yeya24 commented Dec 20, 2024

Maybe I don't have the scenario of query federation. What's the usecase for this metric?

@SungJin1212 SungJin1212 force-pushed the Add-number-of-tenants-per-query-histogram branch from d2c95c6 to 5e697d5 Compare December 20, 2024 21:26
@SungJin1212 SungJin1212 changed the title Add number tenants per query histogram Make the number of workers processing federated query configurable Dec 20, 2024
@SungJin1212
Copy link
Member Author

@yeya24
This PR is for making the number of workers processing federated query configurable.
It could help users using more than 16 tenants.
I have updated the PR.

@SungJin1212 SungJin1212 force-pushed the Add-number-of-tenants-per-query-histogram branch from 5e697d5 to fc594df Compare December 20, 2024 21:30
}

func (cfg *Config) RegisterFlags(f *flag.FlagSet) {
f.BoolVar(&cfg.Enabled, "tenant-federation.enabled", false, "If enabled on all Cortex services, queries can be federated across multiple tenants. The tenant IDs involved need to be specified separated by a `|` character in the `X-Scope-OrgID` header (experimental).")
f.IntVar(&cfg.MaxConcurrent, "tenant-federation.max-concurrent", defaultMaxConcurrency, "The number of workers used for processing federated query.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the number of workers for a single federated query, correct? The description is a bit misleading as it sounds like concurrency for shared for all federated queries

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about The number of workers used to process each federated query.?

@SungJin1212 SungJin1212 force-pushed the Add-number-of-tenants-per-query-histogram branch from fc594df to 7b8783b Compare December 22, 2024 01:58
Copy link
Member

@friedrichg friedrichg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 24, 2024
@friedrichg friedrichg merged commit 452e144 into cortexproject:master Dec 24, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/querier lgtm This PR has been approved by a maintainer size/L type/observability To help know what is going on inside Cortex
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants