cluster: handle region after report split #6867

bufferflies · 2023-07-31T11:16:25Z

Signed-off-by: bufferflies [email protected]

What problem does this PR solve?

Issue Number: close #4157 and close tikv/tikv#15210

What is changed and how it works?

put region after batch report split

Check List

Tests

Unit test
Manual test

// test sql
use test;drop table if exists t2;create table t2 (a int, b varchar(10), unique index idx_a(a)) partition by hash (a) partitions 3;insert into t2 values (1,'a'), (11, 'h'), (22, 'o');show table t2 regions;show table t2 regions;show table t2 regions;show table t2 regions;show table t2 regions;show table t2 regions;show table t2
regions;show table t2 regions;show table t2 regions;show table t2 regions;show table t2 regions;show table t2 regions;show table t2 regions;show table t2 regions;show table t2 regions;show table t2 regions;show table t2 regions;


no any err `PD returned no region`

Code changes

Side effects

Related changes

Release note

handle region after report split

Signed-off-by: bufferflies <[email protected]>

ti-chi-bot · 2023-07-31T11:16:27Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

CabinfeverB
HuSharp

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

Signed-off-by: bufferflies <[email protected]>

lhy1024

ci failed

server/grpc_service.go

server/cluster/cluster_worker.go

server/cluster/cluster.go

CabinfeverB

Can we add some comment to show the difference processRegionSplit needs to do less than processRegionHeartbeat

server/cluster/cluster.go

Signed-off-by: bufferflies <[email protected]>

rleungx · 2023-08-03T03:38:00Z

server/cluster/cluster_worker.go

+			continue
+		}
+		// region split initiator store will be leader with a high probability
+		leader := region.Peers[0]


Will it cause the balance leader to wrong place?

good catch.

I will mark this store as split store and balance region scheduler doesn't pick this store as target store.

Signed-off-by: bufferflies <[email protected]>

server/cluster/cluster_worker.go

server/grpc_service.go

Signed-off-by: bufferflies <[email protected]>

codecov · 2023-08-07T07:19:53Z

Codecov Report

Merging #6867 (8df5867) into master (925181c) will increase coverage by 0.19%.
The diff coverage is 86.11%.

❗ Current head 8df5867 differs from pull request most recent head 697d100. Consider uploading reports for the commit 697d100 to get more accurate results

@@            Coverage Diff             @@
##           master    #6867      +/-   ##
==========================================
+ Coverage   74.29%   74.48%   +0.19%     
==========================================
  Files         440      440              
  Lines       47150    47195      +45     
==========================================
+ Hits        35030    35154     +124     
+ Misses       9022     8969      -53     
+ Partials     3098     3072      -26

Flag	Coverage Δ
unittests	`74.48% <86.11%> (+0.19%)`	⬆️

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

Signed-off-by: bufferflies <[email protected]>

CabinfeverB

rest LGTM

CabinfeverB · 2023-09-19T06:29:12Z

pkg/schedule/schedulers/balance_leader.go

@@ -486,7 +486,8 @@ func (l *balanceLeaderScheduler) transferLeaderOut(solver *solver, collector *pl
 // the worst follower peer and transfers the leader.
 func (l *balanceLeaderScheduler) transferLeaderIn(solver *solver, collector *plan.Collector) *operator.Operator {
 	solver.Region = filter.SelectOneRegion(solver.RandFollowerRegions(solver.TargetStoreID(), l.conf.Ranges),
-		nil, filter.NewRegionPendingFilter(), filter.NewRegionDownFilter())
+		nil, filter.NewRegionPendingFilter(), filter.NewRegionDownFilter(),
+		filter.NewStoreRecentlySplitFilter(solver.GetStores()))


we can avoid to create a new filter every time

Signed-off-by: bufferflies <[email protected]>

CabinfeverB · 2023-09-20T03:11:06Z

pkg/schedule/schedulers/balance_leader.go

@@ -184,6 +183,7 @@ func newBalanceLeaderScheduler(opController *operator.Controller, conf *balanceL
 		&filter.StoreStateFilter{ActionScope: s.GetName(), TransferLeader: true, ForbidRecentlySplitRegions: true, OperatorLevel: constant.High},
 		filter.NewSpecialUseFilter(s.GetName()),
 	}
+	s.regionFilters = filter.NewStoreRecentlySplitFilter(opController.GetCluster().GetStores())


It should be created in Schedule()

good catch ， has fixed

Signed-off-by: bufferflies <[email protected]>

nolouch · 2023-09-20T03:54:33Z

server/cluster/cluster_worker.go

+		changed := &core.RegionChanged{
+			IsNew: true, SaveKV: true, SaveCache: true, NeedSync: true,
+		}
+		if err := c.SaveRegion(region, changed); err != nil {


What if the new split region has been heartbeat to pd then the receive the report split? I think we need check the chagned should it be saved.

It can't be hapend. Tikv send this request before the region init.

@bufferflies network is unexpected. and they are not in the same stream.

CabinfeverB · 2023-09-20T05:44:52Z

/run-check-issue-triage-complete

Signed-off-by: bufferflies <[email protected]>

HuSharp · 2023-09-20T11:34:42Z

please fixed GenerateRegionGuideFunc in server/cluster.go

Signed-off-by: bufferflies <[email protected]>

bufferflies · 2023-09-21T02:07:30Z

/merge

ti-chi-bot · 2023-09-21T02:07:31Z

@bufferflies: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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 ti-community-infra/tichi repository.

ti-chi-bot · 2023-09-21T02:07:33Z

This pull request has been accepted and is ready to merge.

Commit hash: 8df5867

ti-chi-bot · 2023-09-21T02:07:46Z

@bufferflies: Your PR was out of date, I have automatically updated it for you.

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

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 ti-community-infra/tichi repository.

This reverts commit a997316.

This reverts commit a997316. Signed-off-by: Ryan Leung <[email protected]>

close #7133 Signed-off-by: Ryan Leung <[email protected]>

split region

6278295

Signed-off-by: bufferflies <[email protected]>

ti-chi-bot bot added do-not-merge/needs-linked-issue release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jul 31, 2023

ti-chi-bot bot requested review from nolouch and rleungx July 31, 2023 11:16

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

bufferflies mentioned this pull request Jul 31, 2023

cluster: handle region after report split #4230

Closed

bufferflies added 2 commits August 1, 2023 10:17

Merge branch 'master' into region_info

efa168f

allow key not found temporarity

94942f2

Signed-off-by: bufferflies <[email protected]>

bufferflies requested a review from lhy1024 August 1, 2023 07:14

CabinfeverB self-requested a review August 2, 2023 03:16

lhy1024 reviewed Aug 2, 2023

View reviewed changes

CabinfeverB reviewed Aug 2, 2023

View reviewed changes

server/grpc_service.go Outdated Show resolved Hide resolved

server/grpc_service.go Outdated Show resolved Hide resolved

CabinfeverB reviewed Aug 2, 2023

View reviewed changes

server/cluster/cluster_worker.go Outdated Show resolved Hide resolved

server/cluster/cluster.go Outdated Show resolved Hide resolved

CabinfeverB reviewed Aug 2, 2023

View reviewed changes

server/cluster/cluster.go Outdated Show resolved Hide resolved

server/cluster/cluster.go Outdated Show resolved Hide resolved

CabinfeverB reviewed Aug 2, 2023

View reviewed changes

server/cluster/cluster.go Outdated Show resolved Hide resolved

extrace metch for save region

6ea877d

Signed-off-by: bufferflies <[email protected]>

rleungx reviewed Aug 3, 2023

View reviewed changes

bufferflies added 4 commits August 3, 2023 14:41

linit

830eedd

Signed-off-by: bufferflies <[email protected]>

linit

a51b7ec

Signed-off-by: bufferflies <[email protected]>

Merge branch 'region_info' of github.com:bufferflies/pd into region_info

ea3b253

lint

99fbc63

Signed-off-by: bufferflies <[email protected]>

CabinfeverB reviewed Aug 7, 2023

View reviewed changes

server/cluster/cluster_worker.go Show resolved Hide resolved

server/grpc_service.go Outdated Show resolved Hide resolved

pass ut

b2931c7

Signed-off-by: bufferflies <[email protected]>

bufferflies force-pushed the region_info branch from e680dc0 to b2931c7 Compare August 7, 2023 07:06

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 7, 2023

ti-chi-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 10, 2023

bufferflies added 2 commits September 19, 2023 10:32

ci

7e0e5db

Signed-off-by: bufferflies <[email protected]>

Merge branch 'region_info' of github.com:bufferflies/pd into region_info

f2a01b2

bufferflies force-pushed the region_info branch from 55bfddf to f2a01b2 Compare September 19, 2023 03:48

CabinfeverB reviewed Sep 19, 2023

View reviewed changes

reduce NewStoreRecentlySplitFilter

159a375

Signed-off-by: bufferflies <[email protected]>

CabinfeverB reviewed Sep 20, 2023

View reviewed changes

move regionFilters to scheduler

b3aeb59

Signed-off-by: bufferflies <[email protected]>

nolouch reviewed Sep 20, 2023

View reviewed changes

ti-chi-bot bot removed the do-not-merge/needs-linked-issue label Sep 20, 2023

CabinfeverB removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 20, 2023

resolve conflict

5176e6c

Signed-off-by: bufferflies <[email protected]>

CabinfeverB approved these changes Sep 20, 2023

View reviewed changes

ti-chi-bot bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 20, 2023

resolve conflict

8df5867

Signed-off-by: bufferflies <[email protected]>

bufferflies force-pushed the region_info branch from b0a5348 to 8df5867 Compare September 21, 2023 01:42

ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 21, 2023

Merge branch 'master' into region_info

697d100

ti-chi-bot bot merged commit a997316 into tikv:master Sep 21, 2023
19 checks passed

bufferflies deleted the region_info branch September 21, 2023 02:36

rleungx added a commit to rleungx/pd that referenced this pull request Sep 22, 2023

Revert "cluster: handle region after report split (tikv#6867)"

1628bed

This reverts commit a997316.

rleungx mentioned this pull request Sep 22, 2023

Revert "cluster: handle region after report split (#6867)" #7134

Merged

rleungx added a commit to rleungx/pd that referenced this pull request Sep 22, 2023

Revert "cluster: handle region after report split (tikv#6867)"

4b15a35

This reverts commit a997316. Signed-off-by: Ryan Leung <[email protected]>

ti-chi-bot bot pushed a commit that referenced this pull request Sep 22, 2023

Revert "cluster: handle region after report split (#6867)" (#7134)

e6c8841

close #7133 Signed-off-by: Ryan Leung <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cluster: handle region after report split #6867

cluster: handle region after report split #6867

bufferflies commented Jul 31, 2023 •

edited by CabinfeverB

Loading

ti-chi-bot bot commented Jul 31, 2023 •

edited

Loading

lhy1024 left a comment

CabinfeverB left a comment

rleungx Aug 3, 2023

bufferflies Aug 3, 2023

bufferflies Aug 10, 2023

codecov bot commented Aug 7, 2023 •

edited

Loading

CabinfeverB left a comment

CabinfeverB Sep 19, 2023

CabinfeverB Sep 20, 2023

bufferflies Sep 20, 2023

nolouch Sep 20, 2023

bufferflies Sep 20, 2023

nolouch Sep 21, 2023

CabinfeverB commented Sep 20, 2023

HuSharp commented Sep 20, 2023

bufferflies commented Sep 21, 2023

ti-chi-bot bot commented Sep 21, 2023

ti-chi-bot bot commented Sep 21, 2023

ti-chi-bot bot commented Sep 21, 2023

cluster: handle region after report split #6867

cluster: handle region after report split #6867

Conversation

bufferflies commented Jul 31, 2023 • edited by CabinfeverB Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot bot commented Jul 31, 2023 • edited Loading

lhy1024 left a comment

Choose a reason for hiding this comment

CabinfeverB left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Aug 7, 2023 • edited Loading

Codecov Report

CabinfeverB left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CabinfeverB commented Sep 20, 2023

HuSharp commented Sep 20, 2023

bufferflies commented Sep 21, 2023

ti-chi-bot bot commented Sep 21, 2023

ti-chi-bot bot commented Sep 21, 2023

ti-chi-bot bot commented Sep 21, 2023

bufferflies commented Jul 31, 2023 •

edited by CabinfeverB

Loading

ti-chi-bot bot commented Jul 31, 2023 •

edited

Loading

codecov bot commented Aug 7, 2023 •

edited

Loading