pdctl: support config `balance-leader` #4656

CabinfeverB · 2022-02-18T05:46:18Z

Signed-off-by: Cabinfever_B cabinfeveroier@gmail.com

What problem does this PR solve?

Issue Number: ref #4610
This PR should be merged after #4652 #4655

What is changed and how it works?

pdctl supports update balance-leader-scheduler config

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)

Code changes

Has HTTP API interfaces change (Don't forget to add the declarative for API)

Related changes

PR to update pingcap/docs/pingcap/docs-cn:

Release note

pdctl supports update balance-leader-scheduler config.

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

ti-chi-bot · 2022-02-18T05:46:19Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

bufferflies
rleungx

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.

codecov · 2022-02-18T05:54:56Z

Codecov Report

Merging #4656 (393b884) into master (7e26092) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4656      +/-   ##
==========================================
- Coverage   75.09%   75.03%   -0.06%     
==========================================
  Files         284      284              
  Lines       28022    28038      +16     
==========================================
- Hits        21043    21039       -4     
- Misses       5113     5129      +16     
- Partials     1866     1870       +4

Flag	Coverage Δ
unittests	`75.03% <100.00%> (-0.06%)`	⬇️

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

Impacted Files	Coverage Δ
server/schedulers/balance_leader.go	`92.30% <100.00%> (+0.12%)`	⬆️
tools/pd-ctl/pdctl/command/scheduler.go	`75.37% <100.00%> (+0.76%)`	⬆️
pkg/errs/errs.go	`75.00% <0.00%> (-25.00%)`	⬇️
server/tso/local_allocator.go	`64.86% <0.00%> (-13.52%)`	⬇️
server/id/id.go	`76.19% <0.00%> (-4.77%)`	⬇️
server/election/leadership.go	`75.25% <0.00%> (-2.07%)`	⬇️
server/tso/allocator_manager.go	`61.85% <0.00%> (-1.97%)`	⬇️
server/member/member.go	`65.24% <0.00%> (-1.61%)`	⬇️
server/server.go	`73.94% <0.00%> (-1.42%)`	⬇️
server/schedule/operator_controller.go	`84.44% <0.00%> (-0.81%)`	⬇️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7e26092...393b884. Read the comment docs.

bufferflies · 2022-02-18T08:43:00Z

tools/pd-ctl/pdctl/command/scheduler.go

@@ -442,10 +442,31 @@ func NewConfigSchedulerCommand() *cobra.Command {
 		newConfigHotRegionCommand(),
 		newConfigShuffleRegionCommand(),
 		newConfigGrantHotRegionCommand(),
+		newConfigBalanceLeaderCommand(),


How about other scheduler.

How about other scheduler.

other schedulers should modify their config handler firstly

bufferflies · 2022-02-18T08:44:03Z

tools/pd-ctl/pdctl/command/scheduler.go

 	)
 	return c
 }

+func newConfigBalanceLeaderCommand() *cobra.Command {


The function is similar with newConfigHotRegionCommand, maybe exract same code.

The function is similar with newConfigHotRegionCommand, maybe exract same code.

maybe I can abstract out a function for all schedulers

Currently, each scheduler has a different config API. It is possible to refactor it in the future, so I think just implement it simply now.

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

…leader

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

bufferflies · 2022-03-15T03:49:56Z

server/schedulers/balance_leader.go

+	Ranges  []core.KeyRange `json:"ranges"`
+	Batch   int             `json:"batch"`
+}
+


add some annotation.

bufferflies · 2022-03-15T03:51:50Z

server/schedulers/balance_leader.go

+}
+
+func (handler *balanceLeaderHandler) UpdateConfig(w http.ResponseWriter, r *http.Request) {
+	data, _ := io.ReadAll(r.Body)


why ignor this error.

http.Request.Body only has EOF error. if EOF , it means data is nil, and it will be handled in line 97

bufferflies

the rest LGTM

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

rleungx · 2022-03-15T09:02:29Z

tools/pd-ctl/pdctl/command/scheduler.go

+
+	c.AddCommand(&cobra.Command{
+		Use:   "show",
+		Short: "list the config item",


Suggested change

Short: "list the config item",

Short: "show the config item",

bufferflies · 2022-03-15T09:22:34Z

pkg/apiutil/apiutil.go

@@ -213,26 +213,37 @@ func NewAccessPath(path, method string) AccessPath {
 	return AccessPath{Path: path, Method: method}
 }

+type CheckOption func([]byte, int)


where is it used?

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

rleungx · 2022-03-16T09:39:12Z

server/schedulers/balance_leader.go

@@ -113,12 +116,13 @@ func (conf *balanceLeaderSchedulerConfig) Update(data []byte) (int, interface{})
 	return http.StatusBadRequest, "config item not found"
 }

-func (conf *balanceLeaderSchedulerConfig) Adjust() {
+func (conf *balanceLeaderSchedulerConfig) Validate() bool {
 	if conf.Batch < 1 {


rleungx · 2022-03-16T09:39:40Z

server/schedulers/balance_leader.go

@@ -113,12 +116,13 @@ func (conf *balanceLeaderSchedulerConfig) Update(data []byte) (int, interface{})
 	return http.StatusBadRequest, "config item not found"
 }

-func (conf *balanceLeaderSchedulerConfig) Adjust() {
+func (conf *balanceLeaderSchedulerConfig) Validate() bool {


Suggested change

func (conf *balanceLeaderSchedulerConfig) Validate() bool {

func (conf *balanceLeaderSchedulerConfig) validate() bool {

rleungx

The rest LGTM

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

rleungx · 2022-03-16T12:17:20Z

/merge

ti-chi-bot · 2022-03-16T12:17:20Z

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

/run-all-tests

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 · 2022-03-16T12:17:22Z

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

Commit hash: 4c08c71

ref tikv#4610, ref tikv#4652, ref tikv#4655 pdctl supports update balance-leader-scheduler config Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>

CabinfeverB added 8 commits February 17, 2022 13:12

speed up balance leader

ab6f582

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

balance leader config handler

53e0973

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

Merge branch 'speed_up_balance_leader' into balance_leader_conf

6342864

fix test

f5be6dd

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

fix scheduleController

263cc1c

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

add handler

83c67aa

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

add handler

26b44f7

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

support config balance leader

fb15d6a

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

ti-chi-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Feb 18, 2022

ti-chi-bot requested review from disksing and rleungx February 18, 2022 05:46

CabinfeverB mentioned this pull request Feb 18, 2022

Speed up balance leader #4610

Closed

bufferflies reviewed Feb 18, 2022

View reviewed changes

CabinfeverB added 13 commits March 6, 2022 23:48

Merge branch 'master' into speed_up_balance_leader

0976e10

change store uesd limit

7798e88

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

refactor

0528f86

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

refactor controller

3f139b8

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

refactor controller

065f012

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

refactor controller

15925f9

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

Merge remote-tracking branch 'upstream/master' into speed_up_balance_…

f55e76b

…leader

simply

6f9232b

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

Merge remote-tracking branch 'upstream/master' into balance_leader_conf

2694e4c

change update method

a6a178b

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

merge

ebd48ed

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

add test

795c69b

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

add test

9672a21

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

bufferflies reviewed Mar 15, 2022

View reviewed changes

bufferflies approved these changes Mar 15, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 15, 2022

fix util

ef8b984

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

rleungx reviewed Mar 15, 2022

View reviewed changes

bufferflies reviewed Mar 15, 2022

View reviewed changes

CabinfeverB added 7 commits March 16, 2022 00:35

fix

ccd2858

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

fix

0245a45

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

fix config name

b098573

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

add upper and lower bound

ecef60b

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

merge master

0e5a34f

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

merge master

6738352

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

fix test

2f24cb6

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

rleungx reviewed Mar 16, 2022

View reviewed changes

rleungx approved these changes Mar 16, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 16, 2022

CabinfeverB added 3 commits March 16, 2022 17:44

fix test

55cb08d

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

fix typo

2a0f3af

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

fix typo

521e0b9

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

lhy1024 approved these changes Mar 16, 2022

View reviewed changes

fix invalidate

4c08c71

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 16, 2022

Merge branch 'master' into balance_leader_pdctl

393b884

ti-chi-bot merged commit 1a353ae into tikv:master Mar 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pdctl: support config `balance-leader` #4656

pdctl: support config `balance-leader` #4656

CabinfeverB commented Feb 18, 2022 •

edited

Loading

ti-chi-bot commented Feb 18, 2022 •

edited

Loading

codecov bot commented Feb 18, 2022 •

edited

Loading

bufferflies Feb 18, 2022

CabinfeverB Feb 18, 2022

bufferflies Feb 18, 2022

CabinfeverB Feb 18, 2022 •

edited

Loading

CabinfeverB Mar 11, 2022

bufferflies Mar 15, 2022

bufferflies Mar 15, 2022

CabinfeverB Mar 15, 2022 •

edited

Loading

bufferflies left a comment

rleungx Mar 15, 2022

bufferflies Mar 15, 2022 •

edited

Loading

rleungx Mar 16, 2022

rleungx Mar 16, 2022

rleungx left a comment

rleungx commented Mar 16, 2022

ti-chi-bot commented Mar 16, 2022

ti-chi-bot commented Mar 16, 2022

	Short: "list the config item",
	Short: "show the config item",

	func (conf *balanceLeaderSchedulerConfig) Validate() bool {
	func (conf *balanceLeaderSchedulerConfig) validate() bool {

pdctl: support config balance-leader #4656

pdctl: support config balance-leader #4656

Conversation

CabinfeverB commented Feb 18, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Feb 18, 2022 • edited Loading

codecov bot commented Feb 18, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CabinfeverB Feb 18, 2022 • edited Loading

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 Mar 15, 2022 • edited Loading

Choose a reason for hiding this comment

bufferflies left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bufferflies Mar 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rleungx left a comment

Choose a reason for hiding this comment

rleungx commented Mar 16, 2022

ti-chi-bot commented Mar 16, 2022

ti-chi-bot commented Mar 16, 2022

pdctl: support config `balance-leader` #4656

pdctl: support config `balance-leader` #4656

CabinfeverB commented Feb 18, 2022 •

edited

Loading

ti-chi-bot commented Feb 18, 2022 •

edited

Loading

codecov bot commented Feb 18, 2022 •

edited

Loading

CabinfeverB Feb 18, 2022 •

edited

Loading

CabinfeverB Mar 15, 2022 •

edited

Loading

bufferflies Mar 15, 2022 •

edited

Loading