address comment

Signed-off-by: bufferflies <1045931706@qq.com>

Author: bufferflies

pkg/schedule/schedulers/balance_range_test.go

@@ -533,6 +533,24 @@ func TestAddBalanceRangeJobWithInvalidFieldType(t *testing.T) {
 		config: conf,
 		rd:     render.New(render.Options{IndentJSON: true}),
 	}
+	count := 0
+	checkFn := func(data []byte, pass bool) {
+		req := httptest.NewRequest(http.MethodPut, "/job", bytes.NewReader(data))
+		resp := httptest.NewRecorder()
+		re.NotPanics(func() {
+			handler.addJob(resp, req)
+		})
+		if pass {
+			re.Equal(http.StatusOK, resp.Code)
+			count++
+			re.Len(conf.jobs, count)
+		} else {
+			re.Equal(http.StatusBadRequest, resp.Code)
+		}
+		re.Len(conf.jobs, count)
+	}
+
+	// invalid engine type
 	body, err := json.Marshal(map[string]any{
 		"alias":     "a",
 		"engine":    1,
@@ -541,11 +559,41 @@ func TestAddBalanceRangeJobWithInvalidFieldType(t *testing.T) {
 		"end-key":   "200",
 	})
 	re.NoError(err)
-	req := httptest.NewRequest(http.MethodPut, "/job", bytes.NewReader(body))
-	resp := httptest.NewRecorder()
-	re.NotPanics(func() {
-		handler.addJob(resp, req)
+	checkFn(body, false)
+
+	// invalid timeout type
+	body, err = json.Marshal(map[string]any{
+		"alias":     "a",
+		"engine":    "tikv",
+		"rule":      "leader-scatter",
+		"start-key": "100",
+		"end-key":   "200",
+		"timeout":   "123",
 	})
-	re.Equal(http.StatusBadRequest, resp.Code)
-	re.Empty(conf.jobs)
+	re.NoError(err)
+	checkFn(body, false)
+
+	// normal case
+	body, err = json.Marshal(map[string]any{
+		"alias":     "a",
+		"engine":    "tikv",
+		"rule":      "leader-scatter",
+		"start-key": "100",
+		"end-key":   "200",
+		"timeout":   "123s",
+	})
+	re.NoError(err)
+	checkFn(body, true)
+
+	// invalidate case
+	body, err = json.Marshal(map[string]any{
+		"alias":     "a",
+		"engine":    "tikv",
+		"rule":      "leader-scatter",
+		"start-key": "100",
+		"end-key":   "200",
+		"timeout":   "0s",
+	})
+	re.NoError(err)
+	checkFn(body, false)
 }

pkg/schedule/schedulers/init.go

@@ -593,6 +593,9 @@ func schedulersRegister() {
 			if err != nil {
 				return errs.ErrURLParse.Wrap(err)
 			}
+			if duration <= 0 {
+				return errs.ErrURLParse.FastGenByArgs("timeout must be greater than 0")
+			}
 			alias, err := url.QueryUnescape(args[3])
 			if err != nil {
 				return errs.ErrURLParse.Wrap(err)

server/api/scheduler.go

@@ -249,7 +249,7 @@ func (h *schedulerHandler) CreateScheduler(w http.ResponseWriter, r *http.Reques
 	}
 
 	if err := h.AddScheduler(tp, args...); err != nil {
-		h.r.JSON(w, http.StatusInternalServerError, err.Error())
+		h.r.JSON(w, http.StatusBadRequest, err.Error())
 		return
 	}
 

tools/pd-ctl/tests/scheduler/scheduler_test.go

@@ -522,6 +522,9 @@ func (suite *schedulerTestSuite) checkSchedulerConfig(cluster *pdTests.TestClust
 	re.Equal(core.HexRegionKeyStr([]byte("a")), ranges["start-key"])
 	re.Equal(core.HexRegionKeyStr([]byte("b")), ranges["end-key"])
 
+	echo = tests.MustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "add", "balance-range-scheduler", "--format=raw", "tiflash", "learner-scatter", "learner", "a", "b", "timeout", "0s"}, nil)
+	re.NotContains(echo, "Success!")
+
 	echo = tests.MustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "add", "balance-range-scheduler", "--format=raw", "tiflash", "learner-scatter", "learner", "a", "b"}, nil)
 	re.Contains(echo, "Success!")
 	testutil.Eventually(re, func() bool {