server: enable replication setting when only have one default placeme… (

#3163)

* server: enable replication setting when only have one default placement rule

Signed-off-by: Zheming Li <nkdudu@126.com>

* server: enable replication setting when only have one default placement rule

Signed-off-by: Zheming Li <nkdudu@126.com>

* server: enable replication setting when only have one default placement rule

Signed-off-by: Zheming Li <nkdudu@126.com>

Co-authored-by: Ryan Leung <rleungx@gmail.com>
Co-authored-by: Ti Prow Robot <71242396+ti-community-prow-bot@users.noreply.github.com>

server/api/cluster_test.go

@@ -56,12 +56,8 @@ func (s *testClusterSuite) TestCluster(c *C) {
 
 	c2 := &metapb.Cluster{}
 	r := config.ReplicationConfig{
-		EnablePlacementRules: true,
-		MaxReplicas:          6,
+		MaxReplicas: 6,
 	}
-	// Cannot set the replicas when placement rules is enabled.
-	c.Assert(s.svr.SetReplicationConfig(r), NotNil)
-	r.EnablePlacementRules = false
 	c.Assert(s.svr.SetReplicationConfig(r), IsNil)
 	err = readJSON(testDialClient, url, c2)
 	c.Assert(err, IsNil)

server/server.go

@@ -55,6 +55,7 @@ import (
 	syncer "github.com/tikv/pd/server/region_syncer"
 	"github.com/tikv/pd/server/schedule"
 	"github.com/tikv/pd/server/schedule/hbstream"
+	"github.com/tikv/pd/server/schedule/placement"
 	"github.com/tikv/pd/server/tso"
 	"github.com/tikv/pd/server/versioninfo"
 	"github.com/urfave/negroni"
@@ -833,19 +834,41 @@ func (s *Server) SetReplicationConfig(cfg config.ReplicationConfig) error {
 			}
 		}
 	}
+
+	var rule *placement.Rule
 	if cfg.EnablePlacementRules {
-		// replication.MaxReplicas and replication.LocationLabels won't work when placement rule is enabled
+		// replication.MaxReplicas won't work when placement rule is enabled and not only have one default rule.
 		if cfg.MaxReplicas != old.MaxReplicas {
-			return errors.New("cannot update MaxReplicas when placement rules feature is enabled, please update rule instead")
+			rules := s.GetRaftCluster().GetRuleManager().GetAllRules()
+			if !(len(rules) == 1 && len(rules[0].StartKey) == 0 && len(rules[0].EndKey) == 0) {
+				return errors.New("cannot update MaxReplicas when placement rules feature is enabled and not only default rule exists, please update rule instead")
+			}
+			rule = rules[0]
 		}
+		// replication.LocationLabels won't work when placement rule is enabled
 		if !reflect.DeepEqual(cfg.LocationLabels, old.LocationLabels) {
 			return errors.New("cannot update LocationLabels when placement rules feature is enabled, please update rule instead")
 		}
 	}
 
+	if rule != nil {
+		rule.Count = int(cfg.MaxReplicas)
+		if err := s.GetRaftCluster().GetRuleManager().SetRule(rule); err != nil {
+			log.Error("failed to update rule count",
+				errs.ZapError(err))
+			return err
+		}
+	}
+
 	s.persistOptions.SetReplicationConfig(&cfg)
 	if err := s.persistOptions.Persist(s.storage); err != nil {
 		s.persistOptions.SetReplicationConfig(old)
+		if rule != nil {
+			rule.Count = int(old.MaxReplicas)
+			if e := s.GetRaftCluster().GetRuleManager().SetRule(rule); e != nil {
+				log.Error("failed to roll back count of rule when update replication config", errs.ZapError(e))
+			}
+		}
 		log.Error("failed to update replication config",
 			zap.Reflect("new", cfg),
 			zap.Reflect("old", old),

tests/pdctl/config/config_test.go

@@ -393,10 +393,6 @@ func (s *configTestSuite) TestPlacementRuleBundle(c *C) {
 	c.Assert(err, IsNil)
 	c.Assert(strings.Contains(string(output), "Success!"), IsTrue)
 
-	_, output, err = pdctl.ExecuteCommandC(cmd, "-u", pdAddr, "config", "set", "max-replicas", "1")
-	c.Assert(err, IsNil)
-	c.Assert(strings.Contains(string(output), "please update rule instead"), IsTrue)
-
 	_, output, err = pdctl.ExecuteCommandC(cmd, "-u", pdAddr, "config", "set", "location-labels", "dc,rack")
 	c.Assert(err, IsNil)
 	c.Assert(strings.Contains(string(output), "please update rule instead"), IsTrue)
@@ -567,3 +563,90 @@ func (s *configTestSuite) TestReplicationMode(c *C) {
 	conf.DRAutoSync.PrimaryReplicas = 5
 	check()
 }
+
+func (s *configTestSuite) TestUpdateMaxReplicas(c *C) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	cluster, err := tests.NewTestCluster(ctx, 1)
+	c.Assert(err, IsNil)
+	err = cluster.RunInitialServers()
+	c.Assert(err, IsNil)
+	cluster.WaitLeader()
+	pdAddr := cluster.GetConfig().GetClientURL()
+	cmd := pdctl.InitCommand()
+
+	store := metapb.Store{
+		Id:    1,
+		State: metapb.StoreState_Up,
+	}
+	leaderServer := cluster.GetServer(cluster.GetLeader())
+	c.Assert(leaderServer.BootstrapCluster(), IsNil)
+	svr := leaderServer.GetServer()
+	pdctl.MustPutStore(c, svr, store.Id, store.State, store.Labels)
+	defer cluster.Destroy()
+
+	checkMaxReplicas := func(expect uint64) {
+		args := []string{"-u", pdAddr, "config", "show", "replication"}
+		_, output, err := pdctl.ExecuteCommandC(cmd, args...)
+		c.Assert(err, IsNil)
+		replicationCfg := config.ReplicationConfig{}
+		c.Assert(json.Unmarshal(output, &replicationCfg), IsNil)
+		c.Assert(replicationCfg.MaxReplicas, Equals, expect)
+	}
+
+	checkRuleCount := func(expect int) {
+		args := []string{"-u", pdAddr, "config", "placement-rules", "show", "--group", "pd", "--id", "default"}
+		_, output, err := pdctl.ExecuteCommandC(cmd, args...)
+		c.Assert(err, IsNil)
+		rule := placement.Rule{}
+		c.Assert(json.Unmarshal(output, &rule), IsNil)
+		c.Assert(rule.Count, Equals, expect)
+	}
+
+	// update successfully when placement rules is not enabled.
+	_, output, err := pdctl.ExecuteCommandC(cmd, "-u", pdAddr, "config", "set", "max-replicas", "2")
+	c.Assert(err, IsNil)
+	c.Assert(strings.Contains(string(output), "Success!"), IsTrue)
+	checkMaxReplicas(2)
+
+	// update successfully when only one default rule exists.
+	_, output, err = pdctl.ExecuteCommandC(cmd, "-u", pdAddr, "config", "placement-rules", "enable")
+	c.Assert(err, IsNil)
+	c.Assert(strings.Contains(string(output), "Success!"), IsTrue)
+
+	_, output, err = pdctl.ExecuteCommandC(cmd, "-u", pdAddr, "config", "set", "max-replicas", "3")
+	c.Assert(err, IsNil)
+	c.Assert(strings.Contains(string(output), "Success!"), IsTrue)
+	checkMaxReplicas(3)
+	checkRuleCount(3)
+
+	// update unsuccessfully when many rule exists.
+	f, _ := ioutil.TempFile("/tmp", "pd_tests")
+	fname := f.Name()
+	f.Close()
+	defer func() {
+		os.RemoveAll(fname)
+	}()
+
+	rules := []placement.Rule{
+		{
+			GroupID: "pd",
+			ID:      "test1",
+			Role:    "voter",
+			Count:   1,
+		},
+	}
+	b, err := json.Marshal(rules)
+	c.Assert(err, IsNil)
+	ioutil.WriteFile(fname, b, 0644)
+	_, _, err = pdctl.ExecuteCommandC(cmd, "-u", pdAddr, "config", "placement-rules", "save", "--in="+fname)
+	c.Assert(err, IsNil)
+	checkMaxReplicas(3)
+	checkRuleCount(3)
+
+	_, output, err = pdctl.ExecuteCommandC(cmd, "-u", pdAddr, "config", "set", "max-replicas", "4")
+	c.Assert(err, IsNil)
+	c.Assert(strings.Contains(string(output), "please update rule instead"), IsTrue)
+	checkMaxReplicas(3)
+	checkRuleCount(3)
+}