diff --git a/pkg/schedule/schedulers/evict_leader.go b/pkg/schedule/schedulers/evict_leader.go index d29128e9147e..fcc026c80666 100644 --- a/pkg/schedule/schedulers/evict_leader.go +++ b/pkg/schedule/schedulers/evict_leader.go @@ -74,6 +74,9 @@ func (conf *evictLeaderSchedulerConfig) getStores() []uint64 { } func (conf *evictLeaderSchedulerConfig) BuildWithArgs(args []string) error { + failpoint.Inject("buildWithArgsErr", func() { + failpoint.Return(errors.New("fail to build with args")) + }) if len(args) != 1 { return errs.ErrSchedulerConfig.FastGenByArgs("id") } @@ -144,25 +147,7 @@ func (conf *evictLeaderSchedulerConfig) removeStore(id uint64) (succ bool, last succ = true last = len(conf.StoreIDWithRanges) == 0 } -<<<<<<< HEAD return succ, last -======= - return false, errs.ErrScheduleConfigNotExist.FastGenByArgs() -} - -func (conf *evictLeaderSchedulerConfig) removeStore(id uint64) { - conf.Lock() - defer conf.Unlock() - // if the store is not existed, no need to resume leader transfer - _, _ = conf.removeStoreLocked(id) -} - -func (conf *evictLeaderSchedulerConfig) resetStoreLocked(id uint64, keyRange []core.KeyRange) { - if err := conf.cluster.PauseLeaderTransfer(id); err != nil { - log.Error("pause leader transfer failed", zap.Uint64("store-id", id), errs.ZapError(err)) - } - conf.StoreIDWithRanges[id] = keyRange ->>>>>>> 6b927e117 (*: reset config if the input is invalid (#8632)) } func (conf *evictLeaderSchedulerConfig) resetStore(id uint64, keyRange []core.KeyRange) { @@ -383,7 +368,6 @@ func (handler *evictLeaderHandler) UpdateConfig(w http.ResponseWriter, r *http.R var id uint64 idFloat, ok := input["store_id"].(float64) if ok { -<<<<<<< HEAD id = (uint64)(idFloat) handler.config.RLock() if _, exists = handler.config.StoreIDWithRanges[id]; !exists { @@ -392,12 +376,6 @@ func (handler *evictLeaderHandler) UpdateConfig(w http.ResponseWriter, r *http.R handler.rd.JSON(w, http.StatusInternalServerError, err.Error()) return } -======= - if batchFloat < 1 || batchFloat > 10 { - handler.config.removeStore(id) - handler.rd.JSON(w, http.StatusBadRequest, "batch is invalid, it should be in [1, 10]") - return ->>>>>>> 6b927e117 (*: reset config if the input is invalid (#8632)) } handler.config.RUnlock() args = append(args, strconv.FormatUint(id, 10)) @@ -405,33 +383,20 @@ func (handler *evictLeaderHandler) UpdateConfig(w http.ResponseWriter, r *http.R ranges, ok := (input["ranges"]).([]string) if ok { -<<<<<<< HEAD args = append(args, ranges...) } else if exists { args = append(args, handler.config.getRanges(id)...) } - handler.config.BuildWithArgs(args) - err := handler.config.Persist() -======= - if !inputHasStoreID { - handler.config.removeStore(id) - handler.rd.JSON(w, http.StatusInternalServerError, errs.ErrSchedulerConfig.FastGenByArgs("id")) - return - } - } else if exist { - ranges = handler.config.getRanges(id) - } - - newRanges, err = getKeyRanges(ranges) + err := handler.config.BuildWithArgs(args) if err != nil { - handler.config.removeStore(id) - handler.rd.JSON(w, http.StatusInternalServerError, err.Error()) + handler.config.Lock() + handler.config.cluster.ResumeLeaderTransfer(id) + handler.config.Unlock() + handler.rd.JSON(w, http.StatusBadRequest, err.Error()) return } - - err = handler.config.update(id, newRanges, batch) ->>>>>>> 6b927e117 (*: reset config if the input is invalid (#8632)) + err = handler.config.Persist() if err != nil { handler.config.removeStore(id) handler.rd.JSON(w, http.StatusInternalServerError, err.Error()) diff --git a/pkg/schedule/schedulers/grant_leader.go b/pkg/schedule/schedulers/grant_leader.go index 8fd302d4006f..d52eec5bdb60 100644 --- a/pkg/schedule/schedulers/grant_leader.go +++ b/pkg/schedule/schedulers/grant_leader.go @@ -296,18 +296,15 @@ func (handler *grantLeaderHandler) UpdateConfig(w http.ResponseWriter, r *http.R args = append(args, handler.config.getRanges(id)...) } -<<<<<<< HEAD - handler.config.BuildWithArgs(args) - err := handler.config.Persist() -======= - err := handler.config.buildWithArgs(args) + err := handler.config.BuildWithArgs(args) if err != nil { - _, _ = handler.config.removeStore(id) + handler.config.Lock() + handler.config.cluster.ResumeLeaderTransfer(id) + handler.config.Unlock() handler.rd.JSON(w, http.StatusBadRequest, err.Error()) return } - err = handler.config.persist() ->>>>>>> 6b927e117 (*: reset config if the input is invalid (#8632)) + err = handler.config.Persist() if err != nil { _, _ = handler.config.removeStore(id) handler.rd.JSON(w, http.StatusInternalServerError, err.Error()) diff --git a/plugin/scheduler_example/evict_leader.go b/plugin/scheduler_example/evict_leader.go index 372ed5a168fe..5497b6d2f97b 100644 --- a/plugin/scheduler_example/evict_leader.go +++ b/plugin/scheduler_example/evict_leader.go @@ -275,10 +275,6 @@ func (handler *evictLeaderHandler) UpdateConfig(w http.ResponseWriter, r *http.R args = append(args, handler.config.getRanges(id)...) } -<<<<<<< HEAD - handler.config.BuildWithArgs(args) - err := handler.config.Persist() -======= err := handler.config.BuildWithArgs(args) if err != nil { handler.config.mu.Lock() @@ -287,8 +283,8 @@ func (handler *evictLeaderHandler) UpdateConfig(w http.ResponseWriter, r *http.R handler.rd.JSON(w, http.StatusBadRequest, err.Error()) return } + err = handler.config.Persist() ->>>>>>> 6b927e117 (*: reset config if the input is invalid (#8632)) if err != nil { handler.config.mu.Lock() delete(handler.config.StoreIDWitRanges, id) @@ -314,7 +310,6 @@ func (handler *evictLeaderHandler) DeleteConfig(w http.ResponseWriter, r *http.R handler.config.mu.Lock() defer handler.config.mu.Unlock() -<<<<<<< HEAD _, exists := handler.config.StoreIDWitRanges[id] if exists { delete(handler.config.StoreIDWitRanges, id) @@ -333,27 +328,6 @@ func (handler *evictLeaderHandler) DeleteConfig(w http.ResponseWriter, r *http.R } handler.rd.JSON(w, http.StatusInternalServerError, errors.New("the config does not exist")) -======= - ranges, exists := handler.config.StoreIDWitRanges[id] - if !exists { - handler.rd.JSON(w, http.StatusInternalServerError, errors.New("the config does not exist")) - return - } - delete(handler.config.StoreIDWitRanges, id) - handler.config.cluster.ResumeLeaderTransfer(id) - - if err := handler.config.Persist(); err != nil { - handler.config.StoreIDWitRanges[id] = ranges - _ = handler.config.cluster.PauseLeaderTransfer(id) - handler.rd.JSON(w, http.StatusInternalServerError, err.Error()) - return - } - var resp any - if len(handler.config.StoreIDWitRanges) == 0 { - resp = noStoreInSchedulerInfo - } - handler.rd.JSON(w, http.StatusOK, resp) ->>>>>>> 6b927e117 (*: reset config if the input is invalid (#8632)) } func newEvictLeaderHandler(config *evictLeaderSchedulerConfig) http.Handler { diff --git a/tools/pd-ctl/tests/scheduler/scheduler_test.go b/tools/pd-ctl/tests/scheduler/scheduler_test.go index 037595c1ff50..fcb62c018cc6 100644 --- a/tools/pd-ctl/tests/scheduler/scheduler_test.go +++ b/tools/pd-ctl/tests/scheduler/scheduler_test.go @@ -81,6 +81,7 @@ func (suite *schedulerTestSuite) TearDownTest() { if slice.NoneOf(currentSchedulers, func(i int) bool { return currentSchedulers[i] == scheduler }) { + fmt.Println("xxxadd", scheduler) echo := mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "add", scheduler}, nil) re.Contains(echo, "Success!") } @@ -661,7 +662,7 @@ func (suite *schedulerTestSuite) checkScheduler(cluster *pdTests.TestCluster) { } func (suite *schedulerTestSuite) TestSchedulerDiagnostic() { - suite.env.RunTestInTwoModes(suite.checkSchedulerDiagnostic) + suite.env.RunTestInAPIMode(suite.checkSchedulerDiagnostic) } func (suite *schedulerTestSuite) checkSchedulerDiagnostic(cluster *pdTests.TestCluster) { @@ -723,8 +724,7 @@ func (suite *schedulerTestSuite) checkSchedulerDiagnostic(cluster *pdTests.TestC } func (suite *schedulerTestSuite) TestEvictLeaderScheduler() { - // FIXME: API mode may have the problem - suite.env.RunTestInPDMode(suite.checkEvictLeaderScheduler) + suite.env.RunFuncInTwoModes(suite.checkEvictLeaderScheduler) } func (suite *schedulerTestSuite) checkEvictLeaderScheduler(cluster *pdTests.TestCluster) { @@ -759,18 +759,23 @@ func (suite *schedulerTestSuite) checkEvictLeaderScheduler(cluster *pdTests.Test } pdTests.MustPutRegion(re, cluster, 1, 1, []byte("a"), []byte("b")) + suite.checkDefaultSchedulers(re, cmd, pdAddr) + output, err := tests.ExecuteCommand(cmd, []string{"-u", pdAddr, "scheduler", "add", "evict-leader-scheduler", "2"}...) re.NoError(err) re.Contains(string(output), "Success!") + failpoint.Enable("github.com/tikv/pd/pkg/schedule/schedulers/buildWithArgsErr", "return(true)") output, err = tests.ExecuteCommand(cmd, []string{"-u", pdAddr, "scheduler", "add", "evict-leader-scheduler", "1"}...) re.NoError(err) - re.Contains(string(output), "Success!") + re.Contains(string(output), "fail to build with args") + failpoint.Disable("github.com/tikv/pd/pkg/schedule/schedulers/buildWithArgsErr") output, err = tests.ExecuteCommand(cmd, []string{"-u", pdAddr, "scheduler", "remove", "evict-leader-scheduler"}...) re.NoError(err) re.Contains(string(output), "Success!") - output, err = tests.ExecuteCommand(cmd, []string{"-u", pdAddr, "scheduler", "add", "evict-leader-scheduler", "1"}...) - re.NoError(err) - re.Contains(string(output), "Success!") + testutil.Eventually(re, func() bool { + output, err = tests.ExecuteCommand(cmd, []string{"-u", pdAddr, "scheduler", "add", "evict-leader-scheduler", "1"}...) + return err == nil && strings.Contains(string(output), "Success!") + }) output, err = tests.ExecuteCommand(cmd, []string{"-u", pdAddr, "scheduler", "remove", "evict-leader-scheduler-1"}...) re.NoError(err) re.Contains(string(output), "Success!") @@ -794,3 +799,32 @@ func mightExec(re *require.Assertions, cmd *cobra.Command, args []string, v any) } json.Unmarshal(output, v) } + +func (suite *schedulerTestSuite) checkDefaultSchedulers(re *require.Assertions, cmd *cobra.Command, pdAddr string) { + // scheduler show command + expected := make(map[string]bool) + for _, scheduler := range suite.defaultSchedulers { + expected[scheduler] = true + } + checkSchedulerCommand(re, cmd, pdAddr, nil, expected) +} + +func checkSchedulerCommand(re *require.Assertions, cmd *cobra.Command, pdAddr string, args []string, expected map[string]bool) { + if args != nil { + echo := mustExec(re, cmd, args, nil) + re.Contains(echo, "Success!") + } + testutil.Eventually(re, func() bool { + var schedulers []string + mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "show"}, &schedulers) + if len(schedulers) != len(expected) { + return false + } + for _, scheduler := range schedulers { + if _, ok := expected[scheduler]; !ok { + return false + } + } + return true + }) +}