Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

grpc-service: Add splitAndScatterRegions #4212

Closed
wants to merge 86 commits into from
Closed

grpc-service: Add splitAndScatterRegions #4212

wants to merge 86 commits into from

Conversation

hzh0425
Copy link

@hzh0425 hzh0425 commented Oct 15, 2021 •
edited
Loading

ref: pingcap/tidb#29034

Add a method splitAndScatterRegions in grpc-service
let pd handle split and scatter regions

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has not been approved.

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.

@ti-chi-bot
Copy link
Member

@hzh0425: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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.

@ti-chi-bot ti-chi-bot added the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Oct 15, 2021
@ti-chi-bot ti-chi-bot requested review from disksing and Yisaer October 15, 2021 03:31
@hzh0425 hzh0425 changed the title Add splitAndScatterRegions in grpc-service grpc-service: Add splitAndScatterRegions Oct 15, 2021
@rleungx
Copy link
Member

rleungx commented Oct 15, 2021

Does this PR have any context?

@hzh0425
Copy link
Author

hzh0425 commented Oct 15, 2021

Does this PR have any context?

yep, this is a talent challenge program, see here : #4095
my mentor is @Yisaer

@nolouch
Copy link
Contributor

nolouch commented Oct 18, 2021 •
edited
Loading

Also need to update the context in this issue, and do you have a detail design RFC?

@hzh0425 hzh0425 mentioned this pull request Oct 20, 2021
Closed
4 tasks
@hzh0425
Copy link
Author

hzh0425 commented Oct 20, 2021

Also need to update the context in this issue, and do you have a detail design RFC?

@nolouch this pr is a child task in issue: #4224

@hzh0425
Copy link
Author

hzh0425 commented Oct 20, 2021

close #4224

@nolouch
Copy link
Contributor

nolouch commented Oct 21, 2021 •
edited
Loading

I know it's a subtask, But the design is different from the issue. Such as pingcap/tidb#25281, the first thing is to write the design RFC. we recommend writing proposal RFC before coding, for this topic you can discuss it at https://internals.tidb.io/t/topic/399.

@Yisaer
Copy link
Contributor

Yisaer commented Oct 21, 2021

I know it's a subtask, But the design is different from the issue. Such as pingcap/tidb#25281, the first thing is to write the design RFC. we recommend writing proposal RFC before coding, for this topic you can discuss it at https://internals.tidb.io/t/topic/399.

Currently this task is target to merge split and scatter in PD and allow tidb to directly call this merged function. I think it only needs to use Issue to track this task in TiDB repo instead of rfc. This task isn't complex enough to propose a rfc.

@hzh0425 hzh0425 mentioned this pull request Oct 22, 2021
Closed
6 tasks
@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 9, 2021
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 9, 2021
@hzh0425
Copy link
Author

hzh0425 commented Nov 12, 2021

ref: pingcap/tidb#29034

@hzh0425
Copy link
Author

hzh0425 commented Nov 12, 2021

/run-all-tests

disksing
disksing reviewed Nov 15, 2021
View reviewed changes
Copy link
Contributor

@disksing disksing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the rest LGTM.

server/grpc_service.go Outdated
if len(newRegions) > 1 {
// Divide a region into n, one of them may not need to be scattered,
// so n-1 needs to be scattered to other stores.
newRegions = newRegions[:len(newRegions)-1]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this. The process of scatter is to place the regions randomly on the available store, and the original store can be selected too.

@hzh0425
update go.mod
c7e6666 
update SplitAndScatterRegions
rleungx and others added 28 commits November 16, 2021 09:45
@rleungx @ti-chi-bot @hzh0425
metrics: do not consider TiFlash in region label monitor (tikv#4210)
4895a06 
* do not consider tiflash in region label monitor

Signed-off-by: Ryan Leung <rleungx@gmail.com>

* add a unit test

Signed-off-by: Ryan Leung <rleungx@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Signed-off-by: hzh0425 <642256541@qq.com>
@HunDunDM @ti-chi-bot @hzh0425
schedule: correctly calculate the remove cost of the down peer (tikv#…
63caf58 
…4213)

Signed-off-by: HunDunDM <hundundm@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Signed-off-by: hzh0425 <642256541@qq.com>
@Ehco1996 @ti-chi-bot @hzh0425
client: support pass cert bytes in SecurityOption (tikv#4203)
701244d 
* client: support pass cert bytes in SecurityOption

Signed-off-by: ehco1996 <zh19960202@gmail.com>

* add more test

Signed-off-by: ehco1996 <zh19960202@gmail.com>

* fix lint

Signed-off-by: ehco1996 <zh19960202@gmail.com>

* address comment

Signed-off-by: ehco1996 <zh19960202@gmail.com>

* address comment

Signed-off-by: ehco1996 <zh19960202@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Signed-off-by: hzh0425 <642256541@qq.com>
@JmPotato @ti-chi-bot @hzh0425
client: refine some client code (tikv#4205)
5ff1c0f 
* Refine some client code

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* ref tikv#3149

Signed-off-by: JmPotato <ghzpotato@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Signed-off-by: hzh0425 <642256541@qq.com>
@IcePigZDB @hzh0425
api,pd-ctl: support endKey in ScanRegions http interface (tikv#4142)
74d43e1 
* api: add http interface for scan regions in [startKey,endKey)
Signed-off-by: IcePigZDB <icepigzdb@gmail.com>
Signed-off-by: hzh0425 <642256541@qq.com>
@disksing @hzh0425
readme: fix staled deploy instructions (tikv#4239)
1d9e804 
close tikv#4237

Signed-off-by: disksing <i@disksing.com>
Signed-off-by: hzh0425 <642256541@qq.com>
@JmPotato @hzh0425
tso: introduce tsoBatchController to improve the batch effect of mass…
f5f067a 
…ive TSO requests (tikv#4054)

* Add tsoBatchController to control the TSO client batch interval

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Update the batch strategy of the TSO requests

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Introduce the dynamic bestBatchSize

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Refine the code

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Address the comments

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Refine the code

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Update the metrics for bestBatchSize

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Merge some methods

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Address the comment

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Simplify the code

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Address the comments

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Use break instead of goto

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Disable the new batch strategy by default

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Reduce the unnecessary change

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Add a TODO

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Make bestBatchSize start from a low value

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* ref tikv#3149

Signed-off-by: JmPotato <ghzpotato@gmail.com>
Signed-off-by: hzh0425 <642256541@qq.com>
@qidi1 @nolouch
@ti-chi-bot @hzh0425
core: remove compaction for hot region history (tikv#4236)
eaf524e 
* core:remove compaction for hot region history ref #pingcap/tidb/pull/25281

Signed-off-by: qidi1 <1083369179@qq.com>

* ref tikv#4020

Signed-off-by: nolouch <nolouch@gmail.com>

Co-authored-by: nolouch <nolouch@gmail.com>
Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Signed-off-by: hzh0425 <642256541@qq.com>
@xhebox @hzh0425
api: add api for querying replication state for regions in the given …
15ed419 
…key ranges (tikv#4215)

close tikv#4241

ref pingcap/tidb#28199

Signed-off-by: xhe <xw897002528@gmail.com>
Signed-off-by: hzh0425 <642256541@qq.com>
@JmPotato @ti-chi-bot @hzh0425
client: make the server selection for TSO proxy requests more random (t…
5df12cb 
…ikv#4204)

* Make the server selection for TSO proxy requests more random (ref tikv#3149)

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Refactor the handleDispatcher

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Refine the PD client log

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Stabilize the test TestGlobalAndLocalTSO

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Add function switch

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Use Client interface to switch

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Address the comment: only update connectionCtxs when it's necessary

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Refine the code

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Fix the panic

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Remove unnecessary changes

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Fix a corner case

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Re-choose the stream when enableTSOFollowerProxy is disabled

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Refine the stream choosing

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Refine the code more

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Reduce the unnecessary changes

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Fix the race

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Refine the code

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Address the comments to refine the code

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Address the comments

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Only make the Global TSO watch the updateConnectionCtxsCh

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Add connectionCtxs updateTicker

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Fix the data race of c.urls

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Refine the code

Signed-off-by: JmPotato <ghzpotato@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Signed-off-by: hzh0425 <642256541@qq.com>
@JmPotato @hzh0425
client: introduce UpdateOption method to configure the PD client onli…
c995dc5 
…ne (tikv#4240)

* Introduce ClientOption to configure the PD client (ref tikv#3149)

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Refine the code and add some comments

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Clean the code

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Use a interface method to update the client option

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Update the newOption

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Remove WithTSOFollowerProxy and WithMaxTSOBatchWaitInterval

Signed-off-by: JmPotato <ghzpotato@gmail.com>

* Address the comments and refine the test

Signed-off-by: JmPotato <ghzpotato@gmail.com>
Signed-off-by: hzh0425 <642256541@qq.com>
@hzh0425
dco
18a3bb8
@hzh0425
Merge branch 'tikv:master' into add_split_and_scatter
f7348fe
@w41ter @ti-chi-bot @hzh0425
core: use previous size if heartbeat's size is zero (tikv#4243)
65eadda 
* core: use previous size if heartbeat's size is zero

ref tikv/tikv#11114

Signed-off-by: p4tr1ck <patrick.li@pingcap.com>

* add issue link (tikv#4258)

Signed-off-by: p4tr1ck <patrick.li@pingcap.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Signed-off-by: hzh0425 <642256541@qq.com>
@nolouch @ti-chi-bot @hzh0425
config: default not record hot history region (tikv#4260)
6862016 
Signed-off-by: nolouch <nolouch@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Signed-off-by: hzh0425 <642256541@qq.com>
@tidb-dashboard-bot @breezewish
@HunDunDM @hzh0425
Update TiDB Dashboard to v2021.11.01.1 [master] (tikv#4253)
cf44687 
* Update TiDB Dashboard to v2021.11.01.1

Signed-off-by: tidb-dashboard-bot <tidb-dashboard-bot@pingcap.com>

* ref tikv#4257

Signed-off-by: Breezewish <me@breeswish.org>

Co-authored-by: tidb-dashboard-bot <tidb-dashboard-bot@pingcap.com>
Co-authored-by: Breezewish <me@breeswish.org>
Co-authored-by: 混沌DM <hundundm@gmail.com>
Signed-off-by: hzh0425 <642256541@qq.com>
@JmPotato @hzh0425
tests: use only one PD client during the TestGlobalAndLocalTSO (tikv#…
0e63c68 
…4271)

Signed-off-by: JmPotato <ghzpotato@gmail.com>
Signed-off-by: hzh0425 <642256541@qq.com>
@JmPotato @hzh0425
kv, tests: use failpoint to mock the failed KV saving (tikv#4275)
4dc1cc9 
Signed-off-by: JmPotato <ghzpotato@gmail.com>
Signed-off-by: hzh0425 <642256541@qq.com>
@rleungx @ti-chi-bot @hzh0425
tools: fix the help message of store limit (tikv#4274)
e14d998 
Signed-off-by: Ryan Leung <rleungx@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Signed-off-by: hzh0425 <642256541@qq.com>
@HunDunDM @ti-chi-bot @hzh0425
client: refine some code about tso and fix the leak of defer (tikv#4273)
e9082a5 
* client: refine dynamic option

Signed-off-by: HunDunDM <hundundm@gmail.com>

* client: refine ticker

ref tikv#3149

Signed-off-by: HunDunDM <hundundm@gmail.com>

* add comment

Signed-off-by: HunDunDM <hundundm@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Signed-off-by: hzh0425 <642256541@qq.com>
@JmPotato @hzh0425
kv, tests: use failpoint to mock the memory KV range limit (tikv#4287)
4199f3b 
Signed-off-by: JmPotato <ghzpotato@gmail.com>
Signed-off-by: hzh0425 <642256541@qq.com>
@lhy1024 @ti-chi-bot @hzh0425
metrics: fix tso alert (tikv#4282)
287363b 
* fix

Signed-off-by: lhy1024 <admin@liudos.us>

* close tikv#4283

Signed-off-by: lhy1024 <admin@liudos.us>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Signed-off-by: hzh0425 <642256541@qq.com>
@baurine @hzh0425
Update TiDB Dashboard to v2021.11.08.1 [master] (tikv#4297)
09b5032 
close tikv#4257

Signed-off-by: baurine <2008.hbl@gmail.com>
Signed-off-by: hzh0425 <642256541@qq.com>
@HunDunDM @hzh0425
cluster: fix the bug that region statistics are not updated after flo…
3325059 
…w-round-by-digit change (tikv#4304)

* cluster: fix the bug that region statistics are not updated after flow-round-by-digit change

close tikv#4295

Signed-off-by: HunDunDM <hundundm@gmail.com>

* refine some code

Signed-off-by: HunDunDM <hundundm@gmail.com>

* address comment

Signed-off-by: HunDunDM <hundundm@gmail.com>
Signed-off-by: hzh0425 <642256541@qq.com>
@zhouqiang-cl @disksing @hzh0425
Add code owner (tikv#4264)
d76256b 
Signed-off-by: Qiang Zhou <zhouqiang.cl@gmail.com>

Co-authored-by: disksing <i@disksing.com>
Signed-off-by: hzh0425 <642256541@qq.com>
@sunby @ti-chi-bot @hzh0425
server/schedule: priority to fit healthy peers (tikv#4286)
46ac93a 
Priority to fit healthy peers. ref tikv#4233

Signed-off-by: sunby <sunbingyi1992@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Signed-off-by: hzh0425 <642256541@qq.com>
@hzh0425
update go.mod
06df4ca 
update SplitAndScatterRegions

Signed-off-by: hzh0425 <642256541@qq.com>
@hzh0425
Merge remote-tracking branch 'origin/add_split_and_scatter' into add_…
d600dd7 
…split_and_scatter

# Conflicts:
#	client/option_test.go
#	pkg/grpcutil/grpcutil_test.go
#	server/api/config_test.go
#	server/api/pprof_test.go
#	server/cluster/cluster.go
#	server/cluster/unsafe_recovery_controller.go
#	server/core/store_test.go
#	server/encryptionkm/key_manager.go
#	server/grpc_service.go
#	server/handler.go
#	server/schedule/labeler/labeler.go
#	server/server.go
#	tests/pdctl/config/config_test.go
#	tests/server/core/hot_region_storage_test.go
@hzh0425 hzh0425 closed this Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@disksing disksing disksing left review comments

@Yisaer Yisaer Awaiting requested review from Yisaer

Assignees
No one assigned
Labels
do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.