Skip to content

Commit

Permalink
pd-ctl: print warn information when set max-replica to less than curr…
Browse files Browse the repository at this point in the history
…ent (#8983)

close #8959

Signed-off-by: lhy1024 <[email protected]>
  • Loading branch information
lhy1024 authored Jan 13, 2025
1 parent 50c1bbb commit ad172c7
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 0 deletions.
5 changes: 5 additions & 0 deletions server/api/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (

"github.com/pingcap/errcode"
"github.com/pingcap/errors"
"github.com/pingcap/failpoint"
"github.com/pingcap/log"

"github.com/tikv/pd/pkg/errs"
Expand Down Expand Up @@ -411,6 +412,10 @@ func (h *confHandler) SetScheduleConfig(w http.ResponseWriter, r *http.Request)
// @Success 200 {object} sc.ReplicationConfig
// @Router /config/replicate [get]
func (h *confHandler) GetReplicationConfig(w http.ResponseWriter, r *http.Request) {
failpoint.Inject("getReplicationConfigFailed", func(v failpoint.Value) {
code := v.(int)
h.rd.JSON(w, code, "get config failed")
})
if h.svr.IsServiceIndependent(constant.SchedulingServiceName) &&
r.Header.Get(apiutil.XForbiddenForwardToMicroserviceHeader) != "true" {
cfg, err := h.getSchedulingServerConfig()
Expand Down
21 changes: 21 additions & 0 deletions tools/pd-ctl/pdctl/command/config_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,8 @@ func postConfigDataWithPath(cmd *cobra.Command, key, value, path string) error {
val, err := strconv.ParseFloat(value, 64)
if err != nil {
val = value
} else if key == "max-replicas" {
checkMaxReplicas(cmd, val.(float64))
}
data[key] = val
reqData, err := json.Marshal(data)
Expand All @@ -386,6 +388,25 @@ func postConfigDataWithPath(cmd *cobra.Command, key, value, path string) error {
return nil
}

func checkMaxReplicas(cmd *cobra.Command, newReplica float64) {
header := buildHeader(cmd)
r, err := doRequest(cmd, replicatePrefix, http.MethodGet, header)
if err != nil {
cmd.Printf("Failed to get config when checking config: %s\n", err)
return
}
oldConfig := make(map[string]any)
err = json.Unmarshal([]byte(r), &oldConfig)
if err != nil {
cmd.Printf("Failed to unmarshal config when checking config: %s\n", err)
return
}
oldReplica, ok := oldConfig["max-replicas"].(float64)
if ok && newReplica < oldReplica {
cmd.Printf("Setting max-replica to %v which is less than the current replicas (%v). This may pose a risk. Please confirm the setting.\n", newReplica, oldReplica)
}
}

func setConfigCommandFunc(cmd *cobra.Command, args []string) {
if len(args) != 2 {
cmd.Println(cmd.UsageString())
Expand Down
45 changes: 45 additions & 0 deletions tools/pd-ctl/tests/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1199,6 +1199,51 @@ func (suite *configTestSuite) checkUpdateDefaultReplicaConfig(cluster *pdTests.T
checkRuleIsolationLevel("host")
}

func (suite *configTestSuite) TestMaxReplicaChanged() {
suite.env.RunTest(suite.checkMaxReplicaChanged)
}

func (suite *configTestSuite) checkMaxReplicaChanged(cluster *pdTests.TestCluster) {
re := suite.Require()
leaderServer := cluster.GetLeaderServer()
pdAddr := leaderServer.GetAddr()
cmd := ctl.GetRootCmd()

store := &metapb.Store{
Id: 1,
State: metapb.StoreState_Up,
}
pdTests.MustPutStore(re, cluster, store)

// test set max-replicas with invalid value
output, err := tests.ExecuteCommand(cmd, "-u", pdAddr, "config", "set", "max-replicas", "z")
re.NoError(err)
re.NotContains(string(output), "Success!")
// test set max-replicas with less value
output, err = tests.ExecuteCommand(cmd, "-u", pdAddr, "config", "set", "max-replicas", "2")
re.NoError(err)
re.Contains(string(output), "Success!")
re.Contains(string(output), "which is less than the current replicas")
// test set max-replicas with greater value
output, err = tests.ExecuteCommand(cmd, "-u", pdAddr, "config", "set", "max-replicas", "3")
re.NoError(err)
re.Contains(string(output), "Success!")
re.NotContains(string(output), "which is less than the current replicas")
// test meet error when get config failed
failpoint.Enable("github.com/tikv/pd/server/api/getReplicationConfigFailed", `return(200)`)
output, err = tests.ExecuteCommand(cmd, "-u", pdAddr, "config", "set", "max-replicas", "3")
re.NoError(err)
re.Contains(string(output), "Success!")
re.Contains(string(output), "Failed to unmarshal config when checking config")
failpoint.Disable("github.com/tikv/pd/server/api/getReplicationConfigFailed")
failpoint.Enable("github.com/tikv/pd/server/api/getReplicationConfigFailed", `return(500)`)
output, err = tests.ExecuteCommand(cmd, "-u", pdAddr, "config", "set", "max-replicas", "3")
re.NoError(err)
re.Contains(string(output), "Success!")
re.Contains(string(output), "Failed to get config when checking config")
failpoint.Disable("github.com/tikv/pd/server/api/getReplicationConfigFailed")
}

func (suite *configTestSuite) TestPDServerConfig() {
suite.env.RunTest(suite.checkPDServerConfig)
}
Expand Down

0 comments on commit ad172c7

Please sign in to comment.