server/constraints: check max replicas and constraints

server/balancer_test.go

@@ -212,7 +212,7 @@ func (s *testStorageBalancerSuite) TestConstraints(c *C) {
 	cfg, opt := newTestScheduleConfig()
 	cfg.MinRegionCount = 1
 	cfg.MinBalanceDiffRatio = 0.01
-	opt.constraints = newConstraints(1, []*Constraint{
+	opt.constraints, _ = newConstraints(1, []*Constraint{
 		{
 			Labels:   map[string]string{"zone": "cn"},
 			Replicas: 1,
@@ -312,7 +312,7 @@ func (s *testReplicaCheckerSuite) TestConstraints(c *C) {
 	cfg, opt := newTestScheduleConfig()
 	cfg.MinRegionCount = 1
 	cfg.MinBalanceDiffRatio = 0.01
-	opt.constraints = newConstraints(3, []*Constraint{
+	opt.constraints, _ = newConstraints(3, []*Constraint{
 		{
 			Labels:   map[string]string{"zone": "us"},
 			Replicas: 2,

server/cluster_worker_test.go

@@ -178,7 +178,7 @@ func (s *testClusterWorkerSuite) SetUpTest(c *C) {
 
 	s.svr, s.cleanup = newTestServer(c)
 	s.svr.cfg.nextRetryDelay = 50 * time.Millisecond
-	s.svr.scheduleOpt.constraints = newConstraints(5, nil)
+	s.svr.scheduleOpt.constraints, _ = newConstraints(5, nil)
 
 	s.client = s.svr.client
 	s.clusterID = s.svr.clusterID
@@ -353,7 +353,7 @@ func (s *testClusterWorkerSuite) TestHeartbeatSplit(c *C) {
 	cluster := s.svr.GetRaftCluster()
 	c.Assert(cluster, NotNil)
 
-	s.svr.scheduleOpt.constraints = newConstraints(1, nil)
+	s.svr.scheduleOpt.constraints, _ = newConstraints(1, nil)
 
 	leaderPD := mustGetLeader(c, s.client, s.svr.getLeaderPath())
 	conn, err := rpcConnect(leaderPD.GetAddr())
@@ -481,7 +481,7 @@ func (s *testClusterWorkerSuite) TestHeartbeatChangePeer(c *C) {
 	region = s.checkRegionPeerCount(c, regionKey, 5)
 
 	// Remove 2 peers.
-	s.svr.scheduleOpt.constraints = newConstraints(3, nil)
+	s.svr.scheduleOpt.constraints, _ = newConstraints(3, nil)
 
 	// Remove 2 peers
 	for i := 0; i < 2; i++ {
@@ -504,7 +504,7 @@ func (s *testClusterWorkerSuite) TestHeartbeatSplitAddPeer(c *C) {
 	cluster := s.svr.GetRaftCluster()
 	c.Assert(cluster, NotNil)
 
-	s.svr.scheduleOpt.constraints = newConstraints(2, nil)
+	s.svr.scheduleOpt.constraints = &Constraints{MaxReplicas: 2}
 
 	leaderPD := mustGetLeader(c, s.client, s.svr.getLeaderPath())
 	conn, err := rpcConnect(leaderPD.GetAddr())

server/config.go

@@ -443,8 +443,10 @@ func (c *Config) parseConstraints() error {
 		}
 		constraints = append(constraints, constraint)
 	}
-	c.constraints = newConstraints(int(c.MaxPeerCount), constraints)
-	return nil
+
+	var err error
+	c.constraints, err = newConstraints(int(c.MaxPeerCount), constraints)
+	return errors.Trace(err)
 }
 
 // ParseUrls parse a string into multiple urls.

server/constraints.go

@@ -16,6 +16,8 @@ package server
 import (
 	"fmt"
 	"sort"
+
+	"github.com/juju/errors"
 )
 
 // Constraint is a replica constraint to place region peers.
@@ -42,25 +44,51 @@ func (c *Constraint) Match(store *storeInfo) bool {
 	return true
 }
 
+// Exclusive returns true if the constraints are mutually exclusive.
+func (c *Constraint) Exclusive(other *Constraint) bool {
+	for k, v := range c.Labels {
+		if value, ok := other.Labels[k]; ok && value != v {
+			return true
+		}
+	}
+	return false
+}
+
 // Constraints contains all replica constraints.
 type Constraints struct {
 	MaxReplicas int
 	Constraints []*Constraint
 }
 
-func newConstraints(maxReplicas int, constraints []*Constraint) *Constraints {
+func newConstraints(maxReplicas int, constraints []*Constraint) (*Constraints, error) {
+	// Ensure max replicas is valid.
 	sumReplicas := 0
 	for _, constraint := range constraints {
+		if constraint.Replicas == 0 {
+			return nil, errors.Errorf("constraint %v must have replicas", constraint)
+		}
 		sumReplicas += constraint.Replicas
 	}
-	if maxReplicas <= sumReplicas {
-		// Max replicas should not be smaller than the sum replicas.
-		maxReplicas = sumReplicas
+	if maxReplicas < sumReplicas {
+		return nil, errors.Errorf("max replicas %v must not be smaller than the sum replicas %v", maxReplicas, sumReplicas)
 	}
+	if maxReplicas%2 == 0 {
+		return nil, errors.Errorf("max replicas %v must not be even", maxReplicas)
+	}
+
+	// Ensure Constraints are mutually exclusive.
+	for i, constraint := range constraints {
+		for _, other := range constraints[i+1:] {
+			if !constraint.Exclusive(other) {
+				return nil, errors.Errorf("constraint %v and %v must be mutually exclusive", constraints, other)
+			}
+		}
+	}
+
 	return &Constraints{
 		MaxReplicas: maxReplicas,
 		Constraints: constraints,
-	}
+	}, nil
 }
 
 func (c *Constraints) String() string {

server/constraints_test.go

@@ -50,36 +50,83 @@ var _ = Suite(&testConstraintsSuite{})
 
 type testConstraintsSuite struct{}
 
-func (s *testConstraintsSuite) TestReplicas(c *C) {
+func (s *testConstraintsSuite) TestMaxReplicas(c *C) {
 	var constraints []*Constraint
 
-	// No constraints, add default constraint with 3 replicas.
-	cs := newConstraints(3, constraints)
-	c.Assert(cs.MaxReplicas, Equals, 3)
-	c.Assert(cs.Constraints, IsNil)
+	// constraint with no replicas, error.
+	_, err := newConstraints(3, []*Constraint{{}})
+	c.Assert(err, NotNil)
 
+	// max replicas 3 > sum replicas 2, OK.
 	constraints = append(constraints, &Constraint{
-		Labels:   map[string]string{"zone": "us-west", "disk": "ssd"},
+		Labels:   map[string]string{"test": "1"},
 		Replicas: 2,
 	})
+	_, err = newConstraints(3, constraints)
+	c.Assert(err, IsNil)
+
+	// max replicas 3 = sum replicas 3, OK.
+	constraints = append(constraints, &Constraint{
+		Labels:   map[string]string{"test": "2"},
+		Replicas: 1,
+	})
+	_, err = newConstraints(3, constraints)
+	c.Assert(err, IsNil)
+
+	// max replicas 3 < sum replicas 4, error.
 	constraints = append(constraints, &Constraint{
-		Labels:   map[string]string{"zone": "us-east", "disk": "hdd"},
+		Labels:   map[string]string{"test": "3"},
 		Replicas: 1,
 	})
+	_, err = newConstraints(3, constraints)
+	c.Assert(err, NotNil)
 
-	// max replicas 2 < sum replicas 3, adjust max replicas to 3.
-	cs = newConstraints(2, constraints)
-	c.Assert(cs.MaxReplicas, Equals, 3)
-	c.Assert(cs.Constraints, HasLen, 2)
-	c.Assert(cs.Constraints[0], DeepEquals, constraints[0])
-	c.Assert(cs.Constraints[1], DeepEquals, constraints[1])
-
-	// max replicas 4 > sum replicas 3, add default constraint with 1 replica.
-	cs = newConstraints(4, constraints)
-	c.Assert(cs.MaxReplicas, Equals, 4)
-	c.Assert(cs.Constraints, HasLen, 2)
-	c.Assert(cs.Constraints[0], DeepEquals, constraints[0])
-	c.Assert(cs.Constraints[1], DeepEquals, constraints[1])
+	// max replicas 4 is even, error.
+	_, err = newConstraints(4, constraints)
+	c.Assert(err, NotNil)
+
+	// max replicas 5 > sum replicas 4, OK.
+	_, err = newConstraints(5, constraints)
+	c.Assert(err, IsNil)
+}
+
+func (s *testConstraintsSuite) TestExclusive(c *C) {
+	// "zone"s and "disk"s are both not the same, OK.
+	constraints := []*Constraint{
+		{
+			Labels:   map[string]string{"zone": "us-west", "disk": "ssd"},
+			Replicas: 2,
+		},
+		{
+			Labels:   map[string]string{"zone": "us-east", "disk": "hdd"},
+			Replicas: 1,
+		},
+	}
+	_, err := newConstraints(3, constraints)
+	c.Assert(err, IsNil)
+
+	// "zone"s are the same, but "disk"s are not, OK.
+	constraints[0].Labels = map[string]string{"zone": "us-west", "disk": "ssd"}
+	constraints[1].Labels = map[string]string{"zone": "us-west", "disk": "hdd"}
+	_, err = newConstraints(3, constraints)
+	c.Assert(err, IsNil)
+
+	// All labels are the same, error.
+	constraints[0].Labels = constraints[1].Labels
+	_, err = newConstraints(3, constraints)
+	c.Assert(err, NotNil)
+
+	// "zone"s are the same.
+	constraints[0].Labels = map[string]string{"zone": "us-east"}
+	constraints[1].Labels = map[string]string{"zone": "us-east", "disk": "hdd"}
+	_, err = newConstraints(3, constraints)
+	c.Assert(err, NotNil)
+
+	// "disk"s are the same.
+	constraints[0].Labels = map[string]string{"zone": "us-east", "disk": "hdd"}
+	constraints[1].Labels = map[string]string{"test": "test", "disk": "hdd"}
+	_, err = newConstraints(3, constraints)
+	c.Assert(err, NotNil)
 }
 
 func (s *testConstraintsSuite) TestConstraints(c *C) {
@@ -93,7 +140,7 @@ func (s *testConstraintsSuite) TestConstraints(c *C) {
 			Replicas: 1,
 		},
 	}
-	cs := newConstraints(4, constraints)
+	cs, _ := newConstraints(3, constraints)
 
 	var stores []*storeInfo
 	result := cs.Match(stores)