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.

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

pd-ctl: rename scatter-range to scatter-range-scheduler #8607

Merged
merged 8 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion client/http/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const (
RegionLabelRulesByIDs = "/pd/api/v1/config/region-label/rules/ids"
// Scheduler
Schedulers = "/pd/api/v1/schedulers"
scatterRangeScheduler = "/pd/api/v1/schedulers/scatter-range-"
scatterRangeScheduler = "/pd/api/v1/schedulers/scatter-range-scheduler-"
Copy link
Member Author

Choose a reason for hiding this comment

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

need update tidb repo

// Admin
ResetTS = "/pd/api/v1/admin/reset-ts"
BaseAllocID = "/pd/api/v1/admin/base-alloc-id"
Expand Down Expand Up @@ -183,6 +183,7 @@ func SchedulerByName(name string) string {
}

// ScatterRangeSchedulerWithName returns the scatter range scheduler API with name parameter.
// It is used in https://github.com/pingcap/tidb/blob/2a3352c45dd0f8dd5102adb92879bbfa964e7f5f/pkg/server/handler/tikvhandler/tikv_handler.go#L1252.
func ScatterRangeSchedulerWithName(name string) string {
Copy link
Member

@rleungx rleungx Sep 9, 2024

Choose a reason for hiding this comment

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

It's a breaking change.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is not used🤔

Copy link
Member

Choose a reason for hiding this comment

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

TiDB uses it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have not found it in TiDB. And where is the service interface implement in PD server? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

return fmt.Sprintf("%s%s", scatterRangeScheduler, name)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/schedule/schedulers/scatter_range.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (conf *scatterRangeSchedulerConfig) getEndKey() []byte {
func (conf *scatterRangeSchedulerConfig) getSchedulerName() string {
conf.RLock()
defer conf.RUnlock()
return fmt.Sprintf("scatter-range-%s", conf.RangeName)
return fmt.Sprintf("%s-%s", types.ScatterRangeScheduler, conf.RangeName)
Copy link
Member

Choose a reason for hiding this comment

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

need to consider the compatibility issue

Copy link
Member Author

Choose a reason for hiding this comment

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

7f14343

This commit fixed it. You can check it in its comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

friendly ping

}

type scatterRangeScheduler struct {
Expand Down
6 changes: 3 additions & 3 deletions pkg/schedule/types/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ const (
// RandomMergeScheduler is random merge scheduler name.
RandomMergeScheduler CheckerSchedulerType = "random-merge-scheduler"
// ScatterRangeScheduler is scatter range scheduler name.
// TODO: update to `scatter-range-scheduler`
ScatterRangeScheduler CheckerSchedulerType = "scatter-range"
ScatterRangeScheduler CheckerSchedulerType = "scatter-range-scheduler"
// ShuffleHotRegionScheduler is shuffle hot region scheduler name.
ShuffleHotRegionScheduler CheckerSchedulerType = "shuffle-hot-region-scheduler"
// ShuffleLeaderScheduler is shuffle leader scheduler name.
Expand Down Expand Up @@ -135,7 +134,8 @@ var (
"grant-hot-region-scheduler": GrantHotRegionScheduler,
"balance-hot-region-scheduler": BalanceHotRegionScheduler,
"random-merge-scheduler": RandomMergeScheduler,
// TODO: update to `scatter-range-scheduler`
"scatter-range-scheduler": ScatterRangeScheduler,
// TODO: remove `scatter-range` after remove `NewScatterRangeSchedulerCommand` from pd-ctl
"scatter-range": ScatterRangeScheduler,
"shuffle-hot-region-scheduler": ShuffleHotRegionScheduler,
"shuffle-leader-scheduler": ShuffleLeaderScheduler,
Expand Down
4 changes: 2 additions & 2 deletions tests/server/api/scheduler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -457,8 +457,8 @@ func (suite *scheduleTestSuite) checkAPI(cluster *tests.TestCluster) {
},
},
{
name: "scatter-range",
createdName: "scatter-range-test",
name: "scatter-range-scheduler",
createdName: "scatter-range-scheduler-test",
args: []arg{{"start_key", ""}, {"end_key", ""}, {"range_name", "test"}},
// Test the scheduler config handler.
extraTestFunc: func(name string) {
Expand Down
15 changes: 14 additions & 1 deletion tools/pd-ctl/pdctl/command/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ func NewAddSchedulerCommand() *cobra.Command {
c.AddCommand(NewShuffleRegionSchedulerCommand())
c.AddCommand(NewShuffleHotRegionSchedulerCommand())
c.AddCommand(NewScatterRangeSchedulerCommand())
c.AddCommand(NewScatterRangeSchedulerCommandV2())
c.AddCommand(NewBalanceLeaderSchedulerCommand())
c.AddCommand(NewBalanceRegionSchedulerCommand())
c.AddCommand(NewBalanceHotRegionSchedulerCommand())
Expand Down Expand Up @@ -422,7 +423,19 @@ func addSchedulerCommandFunc(cmd *cobra.Command, args []string) {
// NewScatterRangeSchedulerCommand returns a command to add a scatter-range-scheduler.
func NewScatterRangeSchedulerCommand() *cobra.Command {
c := &cobra.Command{
Use: "scatter-range [--format=raw|encode|hex] <start_key> <end_key> <range_name>",
Use: "scatter-range [--format=raw|encode|hex] <start_key> <end_key> <range_name>",
Short: "scatter-range will be deprecated in the future, please use scatter-range-scheduler instead",
Run: addSchedulerForScatterRangeCommandFunc,
HuSharp marked this conversation as resolved.
Show resolved Hide resolved
Deprecated: "scatter-range will be deprecated in the future, please use scatter-range-scheduler instead",
}
c.Flags().String("format", "hex", "the key format")
HuSharp marked this conversation as resolved.
Show resolved Hide resolved
return c
}

// NewScatterRangeSchedulerCommandV2 returns a command to add a scatter-range-scheduler.
func NewScatterRangeSchedulerCommandV2() *cobra.Command {
c := &cobra.Command{
Use: "scatter-range-scheduler [--format=raw|encode|hex] <start_key> <end_key> <range_name>",
Short: "add a scheduler to scatter range",
Run: addSchedulerForScatterRangeCommandFunc,
}
Expand Down
4 changes: 2 additions & 2 deletions tools/pd-ctl/tests/scheduler/scheduler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,9 +347,9 @@ func (suite *schedulerTestSuite) checkScheduler(cluster *pdTests.TestCluster) {
"test", "test#", "?test",
/* TODO: to handle case like "tes&t", we need to modify the server's JSON render to unescape the HTML characters */
} {
echo = mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "add", "scatter-range", "--format=raw", "a", "b", name}, nil)
echo = mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "add", "scatter-range-scheduler", "--format=raw", "a", "b", name}, nil)
re.Contains(echo, "Success!")
schedulerName := fmt.Sprintf("scatter-range-%s", name)
schedulerName := fmt.Sprintf("scatter-range-scheduler-%s", name)
// test show scheduler
testutil.Eventually(re, func() bool {
echo = mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "show"}, nil)
Expand Down