-
Notifications
You must be signed in to change notification settings - Fork 1
Sharding strategy and recommendations #355
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
Conversation
WalkthroughDocumentation updates reorganize and reword sharding/partitioning and sharding performance guidance, adjust shard-size and shard-count recommendations in examples, retitle one admin page, and replace two external Locust CLI links with internal anchors. No code or public API changes. Changes
Sequence Diagram(s)(No sequence diagrams: changes are documentation-only and do not introduce or modify runtime control flow.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
docs/performance/sharding.md
Outdated
| ::: | ||
|
|
||
| ## Optimising for query performance | ||
| ### Avoid imbalances |
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.
About detecting imbalances, let's quickly introduce and refer to the XMover utility at this spot as soon as it is released.
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.
Please review the patches enumerated here, so we can release the XMover utility and mention it here. It is a perfect fit for extending the document to guide readers into applying excellent tooling.
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.
- Tracking this here now: Sharding: Refer to XMover utility #377
seut
left a comment
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.
looks good now, thanks!
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.
Thank you for your effort here @amotl
Additionally I believe we could mention the increase/decrease number of shards after table creation 1, maybe with a special mention to 2, since if this value is not set correctly it might prevent users from correctly increasing their shards.
Edit: Diverted to GH-376.
Footnotes
95dba4d to
d4ca40a
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/admin/sharding-partitioning.md(4 hunks)docs/feature/cluster/index.md(1 hunks)docs/performance/scaling.md(1 hunks)docs/performance/sharding.md(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/performance/scaling.md
- docs/feature/cluster/index.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build docs
| ### Segments | ||
|
|
||
| The number of segments within a shard affects query performance because more | ||
| segments have to be visited. | ||
|
|
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.
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.
- Diverted to Sharding: Unflatten section about "Segments" #378.
b4d51b6 to
2849eb7
Compare
|
Hi. We created those tickets to track backlog items coming from this patch and comments on it. Thanks for your valuable feedback across the board. We hope you are fine to tackle them on subsequent iterations. |
matriv
left a comment
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.
Thank you! Please let @seut to have another go to check the new changes.
seut
left a comment
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! just a note related to the minimum shard size, slipped through on earlier reviews, sry.
40782c2 to
1d73a79
Compare
About
Thanks for making a start to improve the sharding guidelines, @WalBeh. We just added a few copy-edits.
Preview
References
Review
Please also add your comments and suggestions when applicable. 🙏
/cc @karynzv, @hammerhead, @WalBeh, @surister, @kneth