Skip to content

server/cluster: avoid repeated region size scans - #11072

Open
lhy1024 wants to merge 4 commits into
tikv:masterfrom
lhy1024:fix/preparing-range-size-cache
Open

server/cluster: avoid repeated region size scans#11072
lhy1024 wants to merge 4 commits into
tikv:masterfrom
lhy1024:fix/preparing-range-size-cache

Conversation

@lhy1024

@lhy1024 lhy1024 commented Jul 31, 2026

Copy link
Copy Markdown
Member

What problem does this PR solve?

When multiple stores are in the Preparing state, PD recalculates the same
region sizes for every store during each checkStores round. With placement
rules enabled, calculateRange also calls GetRegionSizeByRange once for
every matching rule.

For non-empty key ranges, these repeated scans can significantly increase
Region tree lock contention and delay Region heartbeat processing.

Issue Number: ref #9574

What is changed and how does it work?

Cache region sizes by key range within one checkStores round so all Preparing
stores reuse the same calculation.

Load each range size before processing its matching placement rules instead of
scanning the range once per rule.

Discard the cache after every `checkStores` round so the next round reloads
region sizes from the current Region tree.

Limitations

This PR eliminates duplicate range scans across placement rules and Preparing
stores within one checkStores round. Each unique non-empty range is still
scanned in O(N) time once per round. Supporting O(log N) range queries or
incremental range-size aggregation will be handled separately.

Check List

Tests

  • Unit test

Related changes

Release note

Reduce repeated Region tree scans when multiple stores are preparing.

Summary by CodeRabbit

  • Bug Fixes

    • Improved store readiness and placement-rule threshold calculations by reusing region-size results within each calculation round.
    • Corrected placement-rule range evaluation to begin with the first rule matching the target store.
    • Preserved separate calculations across rounds to prevent stale sizing results.
  • Tests

    • Added coverage for cache reuse across stores and placement rules, independent range caching, and calculation-round isolation.

Cache each key range size for one checkStores round so preparing stores share the same calculation. Move the range size lookup out of the placement rule processing loop.

Ref tikv#9574

Signed-off-by: lhy1024 <19542290+lhy1024@users.noreply.github.com>
@ti-chi-bot ti-chi-bot Bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has signed the dco. labels Jul 31, 2026
@ti-chi-bot

ti-chi-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

[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 husharp 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:

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

@ti-chi-bot ti-chi-bot Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 31, 2026
@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The cluster now caches region sizes by key range for each checkStores round. Store readiness and placement-rule threshold calculations reuse cached values. Tests verify reuse across stores and rules, plus isolation between calculation rounds.

Changes

Region-size threshold calculations

Layer / File(s) Summary
Per-round cache wiring
server/cluster/cluster.go, server/cluster/cluster_test.go
checkStores creates a range-keyed cache and passes it through store and threshold checks. Tests verify shared and isolated cache behavior.
Cached threshold and rule evaluation
server/cluster/cluster.go, server/cluster/cluster_test.go
Threshold and weighted range calculations use cached region sizes. Placement-rule evaluation starts at the first matching rule and continues through later applicable rules.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: avoiding repeated region-size scans in the cluster package.
Description check ✅ Passed The description explains the problem, implementation, limitations, tests, related changes, release impact, and issue reference.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands.

Pass the store snapshot collected by checkStores into each store state check instead of rebuilding the complete store slice for every preparing store.

Ref tikv#9574

Signed-off-by: lhy1024 <19542290+lhy1024@users.noreply.github.com>
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.28571% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.24%. Comparing base (070828c) to head (ccc20b6).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11072      +/-   ##
==========================================
+ Coverage   79.23%   79.24%   +0.01%     
==========================================
  Files         540      541       +1     
  Lines       76010    76509     +499     
==========================================
+ Hits        60226    60631     +405     
- Misses      11533    11595      +62     
- Partials     4251     4283      +32     
Flag Coverage Δ
unittests 79.24% <94.28%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

lhy1024 added 2 commits July 31, 2026 14:59
This reverts commit 1bacbb0.

Signed-off-by: lhy1024 <19542290+lhy1024@users.noreply.github.com>
Signed-off-by: lhy1024 <19542290+lhy1024@users.noreply.github.com>
@ti-chi-bot

ti-chi-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

@lhy1024: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-unit-test-next-gen-2 ccc20b6 link true /test pull-unit-test-next-gen-2

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dco-signoff: yes Indicates the PR's author has signed the dco. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant