region: Add accelerate regions schedule API #2599

Yisaer · 2020-07-01T12:52:52Z

Add one new region HTTP API.

What problem does this PR solve?

close #2593

What is changed and how it works?

Add one new region related http api to accelerate regions schedule by given key range.

Check List

Tests

Unit test

Related changes

Need to cherry-pick to the release branch

Release note

Support Region HTTP API to promote regions schedule by given key range

disksing · 2020-07-01T14:40:39Z

server/api/region.go

+// @Success 200 {object} string
+// @Failure 400 {string} string "The input is invalid."
+// @Router /regions/keys/promote [get]
+func (h *regionsHandler) PromoteRegionsInRange(w http.ResponseWriter, r *http.Request) {


I think promote is kind ambiguous, especially we already use the term for the meaning of promote a learner peer to voter.

Could it be better to accept hex formatted keys?

@disksing It's ok to me to change the API/function name here and do you know the API examples in the repo to know about the better format to accept hex formatted keys?

I think maybe you can refer to parseKey in region_command.go?

@disksing Do you think we need to add a format parameter and support multi key formats here?

IMO, hex format is enough because it is our preferred format in the future. Of course, supporting more formats won't make it worse.

Thanks for clarifying.

Yisaer · 2020-07-02T07:59:01Z

/run-all-tests

rleungx · 2020-07-02T09:46:09Z

I think #2524 wants to solve a similar problem. Maybe we can find a common way to do it?

disksing · 2020-07-02T09:58:27Z

I think #2524 wants to solve a similar problem. Maybe we can find a common way to do it?

IMO not very alike.

disksing · 2020-07-02T10:14:20Z

Basically LGTM. But I wonder if we need to support a large number of regions in the range. One optimization method is to directly save the range, and then scan the range with higher priority first when scanning.

disksing · 2020-07-02T10:14:49Z

server/api/region.go

+		return
+	}
+
+	limit := defaultRegionLimit


16 seems to small

@disksing Do you have any advice? What about 256?

Sounds good to me

disksing · 2020-07-02T10:15:18Z

server/api/router.go

@@ -148,6 +148,7 @@ func createRouter(ctx context.Context, prefix string, svr *server.Server) *mux.R
 	clusterRouter.HandleFunc("/regions/check/hist-size", regionsHandler.GetSizeHistogram).Methods("GET")
 	clusterRouter.HandleFunc("/regions/check/hist-keys", regionsHandler.GetKeysHistogram).Methods("GET")
 	clusterRouter.HandleFunc("/regions/sibling/{id}", regionsHandler.GetRegionSiblings).Methods("GET")
+	clusterRouter.HandleFunc("/regions/keys/accelerate-schedule", regionsHandler.AccelerateRegionsScheduleInRange).Methods("POST")


could regions/accelerate-schedule be bettter?

lhy1024 · 2020-07-02T11:34:19Z

server/api/region.go

+		for _, region := range regions {
+			regionsIDList = append(regionsIDList, region.GetID())
+		}
+		rc.AddSuspectRegions(regionsIDList...)


What will happen if repeatedly adding the same regions?

The duplicated operators would be discarded due to checkAddOperator (due to same priority) in AddWaitingOperator if the operators added in the first time haven't been successes yet. Is that true? @disksing

I think true.

server/api/region_test.go

Yisaer · 2020-07-06T05:30:31Z

@rleungx @disksing Ready to be reviewed.

server/api/region.go

Yisaer · 2020-07-06T08:01:34Z

/run-test

Yisaer · 2020-07-06T08:05:13Z

/run-all-test

lhy1024 · 2020-07-06T09:38:21Z

/merge

ti-srebot · 2020-07-06T09:38:23Z

Sorry @lhy1024, you don't have permission to trigger auto merge event on this branch. The number of LGTM for this PR is 0 while it needs 2 at least

lhy1024

LGTM

lhy1024 · 2020-07-06T09:39:31Z

/merge

ti-srebot · 2020-07-06T09:39:32Z

Sorry @lhy1024, you don't have permission to trigger auto merge event on this branch. The number of LGTM for this PR is 0 while it needs 2 at least

lhy1024 · 2020-07-06T09:39:58Z

LGTM

ti-srebot · 2020-07-06T09:40:01Z

@lhy1024,Thanks for your review.

lhy1024 · 2020-07-06T09:40:06Z

/merge

ti-srebot · 2020-07-06T09:40:08Z

Sorry @lhy1024, you don't have permission to trigger auto merge event on this branch. The number of LGTM for this PR is 1 while it needs 2 at least

rleungx · 2020-07-06T09:52:46Z

LGTM

ti-srebot · 2020-07-06T09:52:51Z

@rleungx,Thanks for your review.

ti-srebot · 2020-07-06T10:01:26Z

Sorry @Yisaer, you don't have permission to trigger auto merge event on this branch. You are not a committer or co-leader or leader for the related sigs:scheduling(slack).

rleungx · 2020-07-06T10:02:16Z

/merge

ti-srebot · 2020-07-06T10:04:39Z

/run-all-tests

ti-srebot · 2020-07-06T10:11:07Z

@Yisaer merge failed.

Yisaer · 2020-07-06T10:43:28Z

/run-integration-ddl-test

Yisaer · 2020-07-29T05:38:39Z

/run-cherry-picker

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot · 2020-07-29T05:39:08Z

cherry pick to release-4.0 in PR #2687

Signed-off-by: ti-srebot <ti-srebot@pingcap.com> Co-authored-by: Song Gao <disxiaofei@163.com>

Yisaer marked this pull request as draft July 1, 2020 12:56

disksing reviewed Jul 1, 2020

View reviewed changes

Yisaer changed the title ~~[WIP] region: Add promote regions schedule API~~ [WIP] region: Add accelerate regions schedule API Jul 2, 2020

Yisaer force-pushed the promote_region_ranges branch from 84a27db to 15e4618 Compare July 2, 2020 07:21

Yisaer requested a review from disksing July 2, 2020 07:59

Yisaer force-pushed the promote_region_ranges branch from 0f7626c to 9e368d1 Compare July 2, 2020 08:28

Yisaer marked this pull request as ready for review July 2, 2020 08:28

Yisaer force-pushed the promote_region_ranges branch from 9e368d1 to 0d48d93 Compare July 2, 2020 08:30

Yisaer requested review from rleungx, lhy1024 and nolouch July 2, 2020 08:44

Yisaer added the component/api HTTP API. label Jul 2, 2020

Yisaer changed the title ~~[WIP] region: Add accelerate regions schedule API~~ region: Add accelerate regions schedule API Jul 2, 2020

disksing reviewed Jul 2, 2020

View reviewed changes

Yisaer force-pushed the promote_region_ranges branch from 8470ec4 to ff05a45 Compare July 2, 2020 10:49

Yisaer requested a review from disksing July 2, 2020 10:51

lhy1024 reviewed Jul 2, 2020

View reviewed changes

rleungx reviewed Jul 3, 2020

View reviewed changes

server/api/region_test.go Show resolved Hide resolved

Yisaer requested review from rleungx and lhy1024 July 4, 2020 03:35

Yisaer force-pushed the promote_region_ranges branch from f75fe35 to 25193c3 Compare July 6, 2020 05:28

rleungx reviewed Jul 6, 2020

View reviewed changes

server/api/region.go Outdated Show resolved Hide resolved

Yisaer requested a review from rleungx July 6, 2020 08:26

lhy1024 approved these changes Jul 6, 2020

View reviewed changes

ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 6, 2020

ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Jul 6, 2020

ti-srebot approved these changes Jul 6, 2020

View reviewed changes

ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Jul 6, 2020

Yisaer added the status/can-merge Indicates a PR has been approved by a committer. label Jul 6, 2020

Merge branch 'master' into promote_region_ranges

9c7fe4b

Yisaer merged commit 9f1a55c into tikv:master Jul 6, 2020

solotzg mentioned this pull request Jul 14, 2020

TiFlash temporary unavailable for newly added partition pingcap/tiflash#865

Closed

Yisaer added the needs-cherry-pick-release-4.0 The PR needs to cherry pick to release-4.0 branch. label Jul 29, 2020

ti-srebot pushed a commit to ti-srebot/pd that referenced this pull request Jul 29, 2020

cherry pick tikv#2599 to release-4.0

c5bbf6d

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot mentioned this pull request Jul 29, 2020

region: Add accelerate regions schedule API (#2599) #2687

Merged

Yisaer added a commit that referenced this pull request Jul 29, 2020

cherry pick #2599 to release-4.0 (#2687)

4108b4b

Signed-off-by: ti-srebot <ti-srebot@pingcap.com> Co-authored-by: Song Gao <disxiaofei@163.com>

region: Add accelerate regions schedule API #2599

region: Add accelerate regions schedule API #2599

Conversation

Yisaer commented Jul 1, 2020 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yisaer Jul 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yisaer Jul 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yisaer commented Jul 2, 2020

rleungx commented Jul 2, 2020

disksing commented Jul 2, 2020

disksing commented Jul 2, 2020

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

Choose a reason for hiding this comment

Yisaer commented Jul 6, 2020

Yisaer commented Jul 6, 2020

Yisaer commented Jul 6, 2020

lhy1024 commented Jul 6, 2020

ti-srebot commented Jul 6, 2020

lhy1024 left a comment

Choose a reason for hiding this comment

lhy1024 commented Jul 6, 2020

ti-srebot commented Jul 6, 2020

lhy1024 commented Jul 6, 2020

ti-srebot commented Jul 6, 2020

lhy1024 commented Jul 6, 2020

ti-srebot commented Jul 6, 2020

rleungx commented Jul 6, 2020

ti-srebot commented Jul 6, 2020

ti-srebot commented Jul 6, 2020

rleungx commented Jul 6, 2020

ti-srebot commented Jul 6, 2020

ti-srebot commented Jul 6, 2020

Yisaer commented Jul 6, 2020

Yisaer commented Jul 29, 2020

ti-srebot commented Jul 29, 2020

Yisaer commented Jul 1, 2020 •

edited

Loading

Yisaer Jul 2, 2020 •

edited

Loading

Yisaer Jul 3, 2020 •

edited

Loading