resourcemanager: add settings-only apply path

[okJiang]

What problem does this PR solve?

Issue Number: ref #9737

PatchSettings currently couples metadata settings update and token delta patch together.
For the upcoming PD-side metadata write path, we need a settings-only apply path to avoid
changing runtime tokens when only metadata is updated.

What is changed and how does it work?

• Refactor ResourceGroup.PatchSettings to shared internal logic.
• Add ResourceGroup.ApplySettings, which applies settings without patching token delta.
• Deep copy token bucket settings in NewGroupTokenBucket and guarded settings copy in patch.
• Add unit test TestApplySettingsWithoutTokenPatch to assert fill/burst update but token unchanged.

resourcemanager: add settings-only apply path

Check List

Tests

• Unit test

Code changes

• No configuration change
• No HTTP API interface change
• No persistent data schema change

Side effects

• Possible performance regression: no
• Increased code complexity: low
• Breaking backward compatibility: no

Related changes

• PR to update pingcap/docs/pingcap/docs-cn: N/A
• PR to update pingcap/tiup: N/A
• Need to cherry-pick to the release branch: no

Release note

None.

Summary by CodeRabbit

Release Notes

• New Features

  • Added new method to apply resource group settings with optional token delta handling.

• Refactor

  • Consolidated resource group settings patching logic for improved maintainability.
  • Enhanced token bucket initialization with proper state isolation to prevent unintended interactions between instances.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign siddontang for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes introduce a new ApplySettings method to apply resource group settings without patching token deltas, refactor patching logic into a reusable helper with a conditional flag, and add corresponding settings cloning support in token bucket handling.

Changes

Cohort / File(s)	Summary
Resource Group Settings Application pkg/mcs/resourcemanager/server/resource_group.go, pkg/mcs/resourcemanager/server/resource_group_test.go	Introduces public ApplySettings() method to apply settings without token patching, extracts shared patching logic into internal patchSettingsLocked() helper that accepts a flag to conditionally patch token deltas, and adds test coverage verifying that ApplySettings() updates rate/limit while preserving existing token counts.
Token Bucket Support pkg/mcs/resourcemanager/server/token_buckets.go	Adds proper cloning of token bucket settings in NewGroupTokenBucket and introduces new applySettings() helper method on GroupTokenBucket to apply cloned settings without patching token deltas, aligning with the optional patching behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Settings flow like carrots in spring,
No token patches to change a thing!
Apply with grace, or patch with care,
A helper method wears both hats with flair.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title clearly and concisely describes the main change: adding a settings-only apply path for resource group settings, which is the primary objective of this changeset.
Description check	✅ Passed	The PR description is well-structured, includes the issue reference, explains the problem and solution, provides detailed change information, and completes the checklist with appropriate selections.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

pkg/mcs/resourcemanager/server/token_buckets.go (1)

403-413: Consider extracting the shared settings-clone logic to reduce duplication.

Lines 408–411 are identical to lines 393–396 in patch. A small helper (e.g., updateSettings) could eliminate this duplication and ensure both paths stay in sync if the cloning logic evolves.

♻️ Suggested refactor

+// updateSettings clones and applies the token bucket settings.
+func (gtb *GroupTokenBucket) updateSettings(tb *rmpb.TokenBucket) bool {
+	if settings := tb.GetSettings(); settings != nil {
+		gtb.Settings = proto.Clone(settings).(*rmpb.TokenLimitSettings)
+		gtb.settingChanged = true
+		return true
+	}
+	return false
+}
+
 // patch patches the token bucket settings.
 func (gtb *GroupTokenBucket) patch(tb *rmpb.TokenBucket) {
 	if tb == nil {
 		return
 	}
-	if settings := tb.GetSettings(); settings != nil {
-		setting := proto.Clone(settings).(*rmpb.TokenLimitSettings)
-		gtb.Settings = setting
-		gtb.settingChanged = true
-	}
+	gtb.updateSettings(tb)
 	// The settings in token is delta of the last update and now.
 	gtb.Tokens += tb.GetTokens()
 }
 
 // applySettings applies the token bucket settings only without patching token delta.
 func (gtb *GroupTokenBucket) applySettings(tb *rmpb.TokenBucket) {
 	if tb == nil {
 		return
 	}
-	if settings := tb.GetSettings(); settings != nil {
-		setting := proto.Clone(settings).(*rmpb.TokenLimitSettings)
-		gtb.Settings = setting
-		gtb.settingChanged = true
-	}
+	gtb.updateSettings(tb)
 }

pkg/mcs/resourcemanager/server/resource_group_test.go (1)

64-100: Good coverage of the core invariant.

The test clearly validates that ApplySettings updates fill rate and burst limit while leaving the token count unchanged.

One suggestion: consider adding a complementary assertion that PatchSettings with the same input would add the token delta (i.e., tokens become 123.0 + 999.0 = 1122.0). This would make the distinction between the two paths explicit in the test suite and guard against accidental regression where both paths behave identically.

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}