Skip to content

Commit

Permalink
scheduler: reduce duplicate params in grant hot region scheduler
Browse files Browse the repository at this point in the history
Signed-off-by: lhy1024 <[email protected]>
  • Loading branch information
lhy1024 committed Sep 27, 2024
1 parent 76dc560 commit e576d17
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 18 deletions.
18 changes: 6 additions & 12 deletions pkg/schedule/schedulers/grant_hot_region.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,14 @@ type grantHotRegionSchedulerConfig struct {
StoreLeaderID uint64 `json:"store-leader-id"`
}

func (conf *grantHotRegionSchedulerConfig) setStore(leaderID uint64, peers []uint64) bool {
func (conf *grantHotRegionSchedulerConfig) setStore(leaderID uint64, peers []uint64) {
conf.Lock()
defer conf.Unlock()
ret := slice.AnyOf(peers, func(i int) bool {
return leaderID == peers[i]
})
if ret {
conf.StoreLeaderID = leaderID
conf.StoreIDs = peers
if !slice.Contains(peers, leaderID) {
peers = append(peers, leaderID)
}
return ret
conf.StoreLeaderID = leaderID
conf.StoreIDs = peers
}

func (conf *grantHotRegionSchedulerConfig) getStoreLeaderID() uint64 {
Expand Down Expand Up @@ -194,10 +191,7 @@ func (handler *grantHotRegionHandler) updateConfig(w http.ResponseWriter, r *htt
handler.rd.JSON(w, http.StatusBadRequest, errs.ErrBytesToUint64)
return
}
if !handler.config.setStore(leaderID, storeIDs) {
handler.rd.JSON(w, http.StatusBadRequest, errs.ErrSchedulerConfig)
return
}
handler.config.setStore(leaderID, storeIDs)

if err = handler.config.persist(); err != nil {
handler.config.setStoreLeaderID(0)
Expand Down
4 changes: 1 addition & 3 deletions pkg/schedule/schedulers/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,7 @@ func schedulersRegister() {
}
storeIDs = append(storeIDs, storeID)
}
if !conf.setStore(leaderID, storeIDs) {
return errs.ErrSchedulerConfig
}
conf.setStore(leaderID, storeIDs)
return nil
}
})
Expand Down
2 changes: 1 addition & 1 deletion tools/pd-ctl/pdctl/command/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ func NewSplitBucketSchedulerCommand() *cobra.Command {
// NewGrantHotRegionSchedulerCommand returns a command to add a grant-hot-region-scheduler.
func NewGrantHotRegionSchedulerCommand() *cobra.Command {
c := &cobra.Command{
Use: "grant-hot-region-scheduler <store_leader_id> <store_leader_id,store_peer_id_1,store_peer_id_2>",
Use: "grant-hot-region-scheduler <store_leader_id> <store_follower_id_1,store_follower_id_2>",
Short: `add a scheduler to grant hot region to fixed stores.
Note: If there is balance-hot-region-scheduler running, please remove it first, otherwise grant-hot-region-scheduler will not work.`,
Run: addSchedulerForGrantHotRegionCommandFunc,
Expand Down
26 changes: 24 additions & 2 deletions tools/pd-ctl/tests/scheduler/scheduler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"encoding/json"
"fmt"
"reflect"
"sort"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -617,7 +618,7 @@ func (suite *schedulerTestSuite) checkGrantLeaderScheduler(cluster *pdTests.Test
"evict-slow-store-scheduler": true,
})

checkSchedulerCommand(re, cmd, pdAddr, []string{"-u", pdAddr, "scheduler", "add", "grant-hot-region-scheduler", "1", "1,2,3"}, map[string]bool{
checkSchedulerCommand(re, cmd, pdAddr, []string{"-u", pdAddr, "scheduler", "add", "grant-hot-region-scheduler", "1", "2,3"}, map[string]bool{
"balance-region-scheduler": true,
"balance-leader-scheduler": true,
"grant-hot-region-scheduler": true,
Expand All @@ -631,16 +632,37 @@ func (suite *schedulerTestSuite) checkGrantLeaderScheduler(cluster *pdTests.Test
"store-leader-id": float64(1),
}
mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "config", "grant-hot-region-scheduler"}, &conf3)
sort.Strings(conf3["store-id"].([]string))
sort.Strings(expected3["store-id"].([]string))
re.Equal(expected3, conf3)

echo = mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "config", "grant-hot-region-scheduler", "set", "2", "1,2,3"}, nil)
echo = mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "config", "grant-hot-region-scheduler", "set", "2", "1,3"}, nil)
re.Contains(echo, "Success!")
expected3["store-leader-id"] = float64(2)
testutil.Eventually(re, func() bool {
mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "config", "grant-hot-region-scheduler"}, &conf3)
return reflect.DeepEqual(expected3, conf3)
})

checkSchedulerCommand(re, cmd, pdAddr, []string{"-u", pdAddr, "scheduler", "remove", "grant-hot-region-scheduler"}, map[string]bool{
"balance-region-scheduler": true,
"balance-leader-scheduler": true,
"evict-slow-store-scheduler": true,
})

// use duplicate store id
checkSchedulerCommand(re, cmd, pdAddr, []string{"-u", pdAddr, "scheduler", "add", "grant-hot-region-scheduler", "3", "1,2,3"}, map[string]bool{
"balance-region-scheduler": true,
"balance-leader-scheduler": true,
"grant-hot-region-scheduler": true,
"evict-slow-store-scheduler": true,
})
expected3["store-leader-id"] = float64(3)
testutil.Eventually(re, func() bool {
mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "config", "grant-hot-region-scheduler"}, &conf3)
return reflect.DeepEqual(expected3, conf3)
})

// case 5: remove grant-hot-region-scheduler
echo = mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "add", "balance-hot-region-scheduler"}, nil)
re.Contains(echo, "grant-hot-region-scheduler is running, please remove it first")
Expand Down

0 comments on commit e576d17

Please sign in to comment.