Skip to content

Add percentage based sharding to rulers #6680

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 17 commits into from
Apr 16, 2025
Merged

Add percentage based sharding to rulers #6680

merged 17 commits into from
Apr 16, 2025

Conversation

wilguo
Copy link
Contributor

@wilguo wilguo commented Mar 31, 2025 •
edited
Loading

What this PR does:

  • This PR adds support for ruler tenant shard size to be a percentage with a set percentage of 40% if no ruler_tenant_shard_size override is provided
  • For shard size:
    • If the configured value is < 1, it'll be treated as a percentage.
    • If the value is > 1, it'll be treated as a constant number

Which issue(s) this PR fixes:
Fixes #6652

Checklist

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

@dosubot dosubot bot added the component/rules Bits & bobs todo with rules and alerts: the ruler, config service etc. label Mar 31, 2025
@wilguo wilguo force-pushed the ruler-percentage-based-sharding branch 4 times, most recently from fdfb92b to 306c97d Compare April 1, 2025 21:07
rajagopalanand
rajagopalanand reviewed Apr 1, 2025
View reviewed changes
Copy link
Contributor

@rajagopalanand rajagopalanand left a comment

Choose a reason for hiding this comment

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

Did a quick review. Will continue later

@wilguo wilguo marked this pull request as draft April 1, 2025 21:47
rajagopalanand
rajagopalanand suggested changes Apr 9, 2025
View reviewed changes
@wilguo wilguo force-pushed the ruler-percentage-based-sharding branch from bd55fb4 to d5252ce Compare April 10, 2025 00:38
rajagopalanand
rajagopalanand reviewed Apr 10, 2025
View reviewed changes
rajagopalanand
rajagopalanand approved these changes Apr 11, 2025
View reviewed changes
Copy link
Contributor

@rajagopalanand rajagopalanand left a comment

Choose a reason for hiding this comment

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

LGTM

@wilguo wilguo marked this pull request as ready for review April 11, 2025 17:19
harry671003
harry671003 suggested changes Apr 14, 2025
View reviewed changes
Copy link
Contributor

@harry671003 harry671003 left a comment

Choose a reason for hiding this comment

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

Percentage based sharding is already supported in querier and store-gateways and it works well.

Left some minor comments.

yeya24
yeya24 approved these changes Apr 15, 2025
View reviewed changes
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.

The change LGTM. However, I am not sure if we need to switch to use zone stable shuffle sharding here. #5489

wilguo added 11 commits April 15, 2025 11:48
@wilguo
Add support for percentage based shard size
16a9c20 
Signed-off-by: Wilbert Guo <[email protected]>
@wilguo
Refactor implementation and add unit tests
81301ae 
Signed-off-by: Wilbert Guo <[email protected]>
@wilguo
Update CHANGELOG
edd54fc 
Signed-off-by: Wilbert Guo <[email protected]>
@wilguo
Add support for percentage based shard size
dda0f65 
Signed-off-by: Wilbert Guo <[email protected]>
@wilguo
Refactor implementation and add unit tests
eb02e82 
Signed-off-by: Wilbert Guo <[email protected]>
@wilguo
Update configuration doc
3eafcc7 
Signed-off-by: Wilbert Guo <[email protected]>
@wilguo
Remove wrong rebase change
7600cd4 
Signed-off-by: Wilbert Guo <[email protected]>
@wilguo
Refactor shard size logic and add more unit tests
33023e9 
Signed-off-by: Wilbert Guo <[email protected]>
@wilguo
Minor refactoring
f34469c 
Signed-off-by: Wilbert Guo <[email protected]>
@wilguo
Update CHANGELOG to trigger test re-run
e8809e3 
Signed-off-by: Wilbert Guo <[email protected]>
@wilguo
Remove default shardsize value
af7773f 
Signed-off-by: Wilbert Guo <[email protected]>
wilguo added 3 commits April 15, 2025 11:48
@wilguo
Addressing PR comments
ef371fa 
Signed-off-by: Wilbert Guo <[email protected]>
@wilguo
Minor refactor
e79590a 
Signed-off-by: Wilbert Guo <[email protected]>
@wilguo
Minor comment update
32a22b5 
Signed-off-by: Wilbert Guo <[email protected]>
@wilguo wilguo force-pushed the ruler-percentage-based-sharding branch from 8b55f90 to 32a22b5 Compare April 15, 2025 18:48
@yeya24 yeya24 merged commit 3d24367 into cortexproject:master Apr 16, 2025
17 checks passed
@wilguo wilguo deleted the ruler-percentage-based-sharding branch April 16, 2025 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harry671003 harry671003 harry671003 requested changes

@rajagopalanand rajagopalanand rajagopalanand approved these changes

@yeya24 yeya24 yeya24 approved these changes

Assignees
No one assigned
Labels
component/rules Bits & bobs todo with rules and alerts: the ruler, config service etc. size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Percentage Based Sharding for Rulers
4 participants
@wilguo @rajagopalanand @harry671003 @yeya24