ddl: support scatter region in cluster/global level #56157

River2000i · 2024-09-19T07:40:07Z

What problem does this PR solve?

Issue Number: close #55184 ref tikv/pd#8424 ref #54886

Problem Summary:
region distribution not balance in create table with enable tidb_scatter_region in cluster level.

What changed and how does it work?

tidb_scatter_region support "", "table", "global" instead of on and off.
In global, trigger PD scatter region is achieved by passing tableID as nil. When tableID is nil, PD selects new peers based on the cluster level by https://github.com/tikv/pd/blob/13174b5d4cab4d958f0b4ea2718ea7bb74992bc7/pkg/schedule/scatter/region_scatterer.go#L444

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

refer: https://github.com/pingcap/docs-cn/pull/18626/files

ti-chi-bot · 2024-09-19T07:40:18Z

Hi @River2000i. Thanks for your PR.

I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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/test-infra repository.

tiprow · 2024-09-19T07:40:25Z

Hi @River2000i. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

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.

River2000i · 2024-09-19T07:58:02Z

@D3Hunter @Benjamin2037 PTAL~

pkg/session/session.go

lance6716 · 2024-09-19T12:46:27Z

/ok-to-test

codecov · 2024-09-19T13:00:29Z

Codecov Report

Attention: Patch coverage is 89.28571% with 6 lines in your changes missing coverage. Please review.

Project coverage is 76.4343%. Comparing base (bf455f5) to head (6102476).
Report is 16 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #56157        +/-   ##
================================================
+ Coverage   73.4156%   76.4343%   +3.0186%     
================================================
  Files          1605       1656        +51     
  Lines        445006     460780     +15774     
================================================
+ Hits         326704     352194     +25490     
+ Misses        98251      87181     -11070     
- Partials      20051      21405      +1354

Flag	Coverage Δ
integration	`52.7683% <75.0000%> (?)`
unit	`73.1537% <89.2857%> (+0.6459%)`	⬆️

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

Components	Coverage Δ
dumpling	`52.9478% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`63.0870% <ø> (+17.0560%)`	⬆️

River2000i · 2024-09-20T05:41:03Z

/retest

River2000i · 2024-09-20T08:31:21Z

/test pull-br-integration-tes

River2000i · 2024-09-24T09:10:16Z

/test pull-br-integration-test

tiprow · 2024-09-24T09:10:41Z

@River2000i: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

In response to this:

/test pull-br-integration-test

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.

River2000i · 2024-09-24T09:11:25Z

/retest

River2000i · 2024-09-25T02:31:03Z

/hold

# Conflicts: # pkg/session/bootstrap.go

River2000i · 2024-09-25T04:19:50Z

/retest

River2000i · 2024-09-25T05:38:45Z

/retest

GMHDBJD

rest LGTM

GMHDBJD · 2024-09-25T06:00:37Z

pkg/ddl/split_region.go

 	tableStartKey := tablecodec.GenTablePrefix(physicalTableID)
-	regionIDs, err := store.SplitRegions(ctx, [][]byte{tableStartKey}, scatter, &tableID)
+	scatter, tID := getScatterConfig(scatterScope, tableID)
+	regionIDs, err := store.SplitRegions(ctx, [][]byte{tableStartKey}, scatter, &tID)


so -1 is ok or it should be nil here?

Both is ok. In client-go, the tableID will convert from *int64 to string. If it's nil, it will equal "0". The -1 is used to distinguish it from the default setting of 0 in client-go.
refer: https://github.com/tikv/client-go/blob/6ba909c4ad2de65b5b36d0e5036d0a85f3154cc0/tikv/split_region.go#L245-L248

River2000i · 2024-09-25T06:23:18Z

/retest

River2000i · 2024-09-25T07:57:17Z

/retest

River2000i · 2024-09-25T08:04:11Z

/unhold

GMHDBJD

LGTM

ti-chi-bot · 2024-09-25T08:43:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Benjamin2037, benmeadowcroft, GMHDBJD, Leavrth, wjhuang2016

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [Benjamin2037,GMHDBJD,Leavrth,wjhuang2016]
~~br/OWNERS~~ [Leavrth]
~~pkg/ddl/OWNERS~~ [Benjamin2037,GMHDBJD,wjhuang2016]
~~pkg/session/OWNERS~~ [benmeadowcroft]
~~pkg/sessionctx/variable/OWNERS~~ [benmeadowcroft]

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

Leavrth · 2024-09-25T12:44:40Z

/retest

River2000i · 2024-09-25T12:52:03Z

/retest

Leavrth · 2024-09-25T13:08:09Z

/retest

River2000i added 5 commits September 19, 2024 14:56

support session scope and add comment

58e4627

support scatter region in global level

9312f45

add test

601e21a

Merge branch 'master' into fix-55184

5986bac

fix test

082d6a8

ti-chi-bot bot removed the do-not-merge/needs-triage-completed label Sep 19, 2024

Benjamin2037 reviewed Sep 19, 2024

View reviewed changes

pkg/session/session.go Outdated Show resolved Hide resolved

compatible upgrade

38fe283

ti-chi-bot bot added ok-to-test Indicates a PR is ready to be tested. and removed needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels Sep 19, 2024

River2000i added 3 commits September 20, 2024 11:22

refactor scatterScope to const

3731bb6

fix test

8256974

fix test

65f01a8

River2000i requested a review from Benjamin2037 September 20, 2024 05:42

River2000i added 3 commits September 20, 2024 14:10

fix test

886e679

fix test

673936d

fix test

5979ef6

add comment to describe the testcase(TestScatterRegion) target

1f3e2ae

River2000i requested a review from Benjamin2037 September 24, 2024 10:00

wjhuang2016 approved these changes Sep 24, 2024

View reviewed changes

benmeadowcroft approved these changes Sep 24, 2024

View reviewed changes

ti-chi-bot bot added the approved label Sep 24, 2024

ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 25, 2024

River2000i added 2 commits September 25, 2024 10:33

Merge branch 'master' into fix-55184

868fc4f

# Conflicts: # pkg/session/bootstrap.go

resolve conflict

7a16058

River2000i closed this Sep 25, 2024

River2000i reopened this Sep 25, 2024

GMHDBJD reviewed Sep 25, 2024

View reviewed changes

Merge branch 'master' into fix-55184

6102476

ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 25, 2024

GMHDBJD approved these changes Sep 25, 2024

View reviewed changes

ti-chi-bot bot merged commit e5c9867 into pingcap:master Sep 25, 2024
22 of 25 checks passed

River2000i deleted the fix-55184 branch September 26, 2024 03:04

fzzf678 mentioned this pull request May 26, 2025

ddl: the use of sys var tidb_scatter_region seems incorrect #61321

Closed

ddl: support scatter region in cluster/global level #56157

ddl: support scatter region in cluster/global level #56157

Uh oh!

Conversation

River2000i commented Sep 19, 2024 • edited by lance6716 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Uh oh!

ti-chi-bot bot commented Sep 19, 2024

Uh oh!

tiprow bot commented Sep 19, 2024

Uh oh!

River2000i commented Sep 19, 2024

Uh oh!

Uh oh!

lance6716 commented Sep 19, 2024

Uh oh!

codecov bot commented Sep 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

River2000i commented Sep 20, 2024

Uh oh!

River2000i commented Sep 20, 2024

Uh oh!

River2000i commented Sep 24, 2024

Uh oh!

tiprow bot commented Sep 24, 2024

Uh oh!

River2000i commented Sep 24, 2024

Uh oh!

River2000i commented Sep 25, 2024

Uh oh!

River2000i commented Sep 25, 2024

Uh oh!

River2000i commented Sep 25, 2024

Uh oh!

GMHDBJD left a comment

Choose a reason for hiding this comment

Uh oh!

GMHDBJD Sep 25, 2024

Choose a reason for hiding this comment

Uh oh!

River2000i Sep 25, 2024

Choose a reason for hiding this comment

Uh oh!

River2000i commented Sep 25, 2024

Uh oh!

River2000i commented Sep 25, 2024

Uh oh!

River2000i commented Sep 25, 2024

Uh oh!

GMHDBJD left a comment

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Sep 25, 2024

Uh oh!

Leavrth commented Sep 25, 2024

Uh oh!

River2000i commented Sep 25, 2024

Uh oh!

Leavrth commented Sep 25, 2024

Uh oh!

Uh oh!

Uh oh!

River2000i commented Sep 19, 2024 •

edited by lance6716

Loading

codecov bot commented Sep 19, 2024 •

edited

Loading