Repository navigation
Conversation
Signed-off-by: lhy1024 <19542290+lhy1024@users.noreply.github.com>
|
[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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe documentation states that table-level data affinity is available from v8.5.5 outside PD Microservices Mode, and from v8.5.7 in that mode when the scheduling microservice handles scheduling. It removes the limitation that affinity does not take effect in PD Microservices Mode. ChangesAffinity scheduling documentation
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~3 minutes Change: Other Suggested reviewers: Merge Risk: 🔵 Low · up to The updated page extends its affinity guidance to PD Microservices Mode, but its splitting note does not describe the MCS size and key-count triggers. Operators may misjudge when automatic Region splitting occurs; this is a bounded documentation risk. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 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. Comment |
Signed-off-by: lhy1024 <19542290+lhy1024@users.noreply.github.com>
Signed-off-by: lhy1024 <19542290+lhy1024@users.noreply.github.com>
Signed-off-by: lhy1024 <19542290+lhy1024@users.noreply.github.com>
|
@rleungx: adding LGTM is restricted to approvers and reviewers in OWNERS files. DetailsIn response to this: 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. |
|
|
||
| When you enable PD affinity scheduling and set the `AFFINITY` option of a table to `table` or `partition`, PD groups Regions belonging to the same table or partition into the same affinity group. During scheduling, PD prioritizes placing the Leader and Voter replicas of these Regions on the same subset of a few TiKV nodes. This reduces network latency caused by cross-node access during queries, thereby improving query performance. | ||
|
|
||
| Table-level data affinity is available in PD's classic deployment mode starting from v8.5.5. Starting from v8.5.6, it is also available when scheduling is handled by the scheduling microservice in [PD Microservices Mode](/pd-microservices.md). |
There was a problem hiding this comment.
v8.5.6 seems too early for PD Microservices Mode. In v8.5.6, the scheduling microservice syncs affinity groups (tikv/pd#10232), but its AskBatchSplit handler does not check affinity, and PD does not forward the split reason. As a result, Regions in an affinity group are still split automatically, which contradicts the "Automatic splitting of Regions" behavior described later on this page. The split restriction reached release-8.5 via tikv/pd#10714 (backport of tikv/pd#10652), which is first included in v8.5.7.
Also, "PD's classic deployment mode" is not a term used elsewhere in the docs, so it might be clearer to describe it relative to PD Microservices Mode.
| Table-level data affinity is available in PD's classic deployment mode starting from v8.5.5. Starting from v8.5.6, it is also available when scheduling is handled by the scheduling microservice in [PD Microservices Mode](/pd-microservices.md). | |
| Table-level data affinity is available starting from v8.5.5 when PD is not deployed in [PD Microservices Mode](/pd-microservices.md). Starting from v8.5.7, it is also available in PD Microservices Mode when scheduling is handled by the scheduling microservice. |
Signed-off-by: lhy1024 <19542290+lhy1024@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: pingcap/docs/.coderabbit.yaml
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
3cde11bd-35d2-4a2a-bbc6-113e78e405dd
📒 Files selected for processing (1)
table-affinity.md
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
|
|
||
| When you enable PD affinity scheduling and set the `AFFINITY` option of a table to `table` or `partition`, PD groups Regions belonging to the same table or partition into the same affinity group. During scheduling, PD prioritizes placing the Leader and Voter replicas of these Regions on the same subset of a few TiKV nodes. This reduces network latency caused by cross-node access during queries, thereby improving query performance. | ||
|
|
||
| Table-level data affinity is available starting from v8.5.5 when PD is not deployed in [PD Microservices Mode](/pd-microservices.md). Starting from v8.5.7, it is also available in PD Microservices Mode when scheduling is handled by the scheduling microservice. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
set -u
printf '%s\n' '--- changed file context ---'
nl -ba table-affinity.md | sed -n '10,30p'
printf '%s\n' '--- PR diff for the file ---'
git diff --no-ext-diff --unified=12 6ba85765e2f9e8d080a7f1789a3b46641a09d960 e70d552cc8e7dba3ee7c458c6080e2bc4a7660ea -- table-affinity.md
printf '%s\n' '--- local predicate/source references ---'
rg -n -F --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' -- 'affinityRegionSizeBufferMB' . || test "$?" -eq 1
rg -n -F --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' -- 'ApproximateKeys' . || test "$?" -eq 1
printf '%s\n' '--- splitting wording references ---'
rg -n -F -- 'four times' . || test "$?" -eq 1Repository: pingcap/docs
Length of output: 7915
🌐 Web query:
site:github.com/tikv/pd v8.5.7 pkg/schedule/affinity/split.go grpc_service.go MCS scheduling affinity split
💡 Result:
In **PD v8.5.7**, affinity split handling is split across these files:
- [`pkg/schedule/affinity/split.go`](https://github.com/tikv/pd/blob/v8.5.7/pkg/schedule/affinity/split.go): `Manager.AllowSplit` allows splitting for nil regions, admin splits, or when affinity scheduling is disabled. For affinity regions where regular scheduling is disallowed, it blocks automatic splits below a size/key threshold; larger regions may still split. ([raw.githubusercontent.com](https://raw.githubusercontent.com/tikv/pd/v8.5.7/pkg/schedule/affinity/split.go))
- [`pkg/mcs/scheduling/server/grpc_service.go`](https://github.com/tikv/pd/blob/v8.5.7/pkg/mcs/scheduling/server/grpc_service.go): `AskBatchSplit` calls `AllowSplit`. If denied, it disables auto-splitting for the region and returns `"cannot split affinity region"`; otherwise it allocates the new region and peer IDs. ([raw.githubusercontent.com](https://raw.githubusercontent.com/tikv/pd/v8.5.7/pkg/mcs/scheduling/server/grpc_service.go))
- [`server/grpc_service.go`](https://github.com/tikv/pd/blob/v8.5.7/server/grpc_service.go) is the PD server’s separate gRPC implementation. The MCS scheduling split path is in `pkg/mcs/scheduling/server/grpc_service.go`, not this file. ([raw.githubusercontent.com](https://raw.githubusercontent.com/tikv/pd/v8.5.7/server/grpc_service.go))
These links and findings are for the **v8.5.7** tag.
Citations:
- 1: https://raw.githubusercontent.com/tikv/pd/v8.5.7/pkg/schedule/affinity/split.go
- 2: https://raw.githubusercontent.com/tikv/pd/v8.5.7/pkg/mcs/scheduling/server/grpc_service.go
- 3: https://raw.githubusercontent.com/tikv/pd/v8.5.7/server/grpc_service.go
Document both automatic-splitting triggers for MCS.
The new availability statement makes the automatic-splitting note apply to MCS. In v8.5.7, AskBatchSplit calls AllowSplit, which permits splitting when approximate size reaches (configured merge size + 10 MiB) × 4 or approximate key count reaches its threshold. The current “only when ... exceeds four times” wording can cause operators to miss key-count-triggered splits and use an incorrect size threshold.
Suggested fix
-- **Automatic splitting of Regions**: when a Region belongs to an affinity group and affinity is in effect, automatic splitting of that Region is disabled by default to avoid the creation of too many Regions that could weaken the affinity effect. Automatic splitting is triggered only when the Region size exceeds four times the value of [`schedule.max-affinity-merge-region-size`](/pd-configuration-file.md#max-affinity-merge-region-size-new-in-v855-and-v900). Note that splits triggered by components other than TiKV or PD (such as manual splits triggered by [`SPLIT TABLE`](/sql-statements/sql-statement-split-region.md)) are not subject to this restriction.
+- **Automatic splitting of Regions**: when a Region belongs to an affinity group and affinity is in effect, automatic splitting of that Region is disabled by default to avoid the creation of too many Regions that could weaken the affinity effect. Automatic splitting is triggered when the Region's approximate size reaches four times the value of [`schedule.max-affinity-merge-region-size`](/pd-configuration-file.md#max-affinity-merge-region-size-new-in-v855-and-v900) plus the 10 MiB buffer, or when its approximate key count reaches the configured threshold. Note that splits triggered by components other than TiKV or PD (such as manual splits triggered by [`SPLIT TABLE`](/sql-statements/sql-statement-split-region.md)) are not subject to this restriction.|
@JmPotato PTAL |
|
@qiancai Could you help move this PR forward to merge? The content is final: following JmPotato's review, the PD microservices support now starts from v8.5.7, which includes tikv/pd#10714. It also needs to be cherry-picked to release-8.5. Thanks! |
What is changed, added, or deleted? (Required)
Replace the blanket PD microservices limitation in
table-affinity.mdwith an explicit distinction between deployment modes: table-level data affinity is available starting from v8.5.5 when PD is not deployed in PD Microservices Mode, and in PD Microservices Mode when scheduling is handled by the scheduling microservice starting from v8.5.7. The scheduling microservice affinity metadata synchronization was added by tikv/pd#10042 and backported in tikv/pd#10232; the split restriction was backported by tikv/pd#10714.Use the verified v8.5.7 version boundary in master and release-8.5.
Validation:
./scripts/markdownlint table-affinity.md; verified the affinity metadata and split restriction backport boundaries in the PD v8.5 tags.Which TiDB version(s) do your changes apply to? (Required)
Tips for choosing the affected version(s):
By default, CHOOSE MASTER ONLY so your changes will be applied to the next TiDB major or minor releases. If your PR involves a product feature behavior change or a compatibility change, CHOOSE THE AFFECTED RELEASE BRANCH(ES) AND MASTER.
For details, see tips for choosing the affected versions.
What is the related PR or file link(s)?
AI agent involvement
Do your changes match any of the following descriptions?
Summary by CodeRabbit