kvrpcpb: report noisy resource groups in HealthFeedback - #1544
mittalrishabh wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
ti-chi-bot
Bot
added
the
size/XXL
|
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe protobuf schema adds a ChangesHealth feedback reporting
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Feature Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable issue remains in the supplied health-feedback contract; the change is mergeable after normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. A rabbit reads the groups with care, Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
TiKV already knows which resource groups it blames for its own overload -- `ResourceGroupManager` keeps the set in `noisy_groups` and refreshes it on the control tick -- but the only way that verdict reaches a client today is the `|noisy_tenant` suffix on a `ServerIsBusy` reason, which arrives only when a request from that group is actually rejected. Every group has to be rejected once to be discovered, and the client has no way to learn that a group stopped being noisy. Carry the set on `HealthFeedback` instead, which is already attached to batch responses on a timer, so a client learns the whole set from any response. `noisy_groups` is a message rather than a bare repeated field so that a store which does not report the set at all stays distinguishable from one reporting an empty set: the former leaves the field unset, the latter sends an empty `names`, and only the latter licenses a client to clear what it knew. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 15db782) Signed-off-by: rishabh mittal <mittalrishabh@gmail.com>
09946a8 to
fb412a0
Compare
|
[FORMAT CHECKER NOTIFICATION] Notice: To remove the 📖 For more info, you can check the "Contribute Code" section in the development guide. |
|
mornyx PTAL |
1 participant
TiKV already knows which resource groups it blames for its own overload -- ResourceGroupManager keeps the set in noisy_groups and refreshes it on the control tick
Carry the set on HealthFeedback instead, which is already attached to batch responses on a timer, so a client learns the whole set from any response.
Summary by CodeRabbit