placement: fix majority schedule when primaryRegion and regions are s…

…ame (#31312) close #31271
pingcap · Jan 5, 2022 · 3de7265 · 3de7265
1 parent 29c398c
commit 3de7265
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 17 deletions.
diff --git a/ddl/placement/bundle.go b/ddl/placement/bundle.go
@@ -153,18 +153,20 @@ func NewBundleFromSugarOptions(options *model.PlacementSettings) (*Bundle, error
 	}
 	schedule := options.Schedule
 
-	primaryIndex := 0
-	// regions must include the primary
-	// but we don't see empty primaryRegion and regions as an error
-	if primaryRegion != "" || len(regions) > 0 {
-		sort.Strings(regions)
-		primaryIndex = sort.SearchStrings(regions, primaryRegion)
-		if primaryIndex >= len(regions) || regions[primaryIndex] != primaryRegion {
-			return nil, fmt.Errorf("%w: primary region must be included in regions", ErrInvalidPlacementOptions)
-		}
+	var Rules []*Rule
+
+	// in case empty primaryRegion and regions, just return an empty bundle
+	if primaryRegion == "" && len(regions) == 0 {
+		Rules = append(Rules, NewRule(Voter, followers+1, NewConstraintsDirect()))
+		return &Bundle{Rules: Rules}, nil
 	}
 
-	var Rules []*Rule
+	// regions must include the primary
+	sort.Strings(regions)
+	primaryIndex := sort.SearchStrings(regions, primaryRegion)
+	if primaryIndex >= len(regions) || regions[primaryIndex] != primaryRegion {
+		return nil, fmt.Errorf("%w: primary region must be included in regions", ErrInvalidPlacementOptions)
+	}
 
 	// primaryCount only makes sense when len(regions) > 0
 	// but we will compute it here anyway for reusing code
@@ -179,15 +181,15 @@ func NewBundleFromSugarOptions(options *model.PlacementSettings) (*Bundle, error
 		return nil, fmt.Errorf("%w: unsupported schedule %s", ErrInvalidPlacementOptions, schedule)
 	}
 
-	if len(regions) == 0 {
-		Rules = append(Rules, NewRule(Voter, followers+1, NewConstraintsDirect()))
-	} else {
-		Rules = append(Rules, NewRule(Voter, primaryCount, NewConstraintsDirect(NewConstraintDirect("region", In, primaryRegion))))
+	Rules = append(Rules, NewRule(Voter, primaryCount, NewConstraintsDirect(NewConstraintDirect("region", In, primaryRegion))))
+	if followers+1 > primaryCount {
+		// delete primary from regions
+		regions = regions[:primaryIndex+copy(regions[primaryIndex:], regions[primaryIndex+1:])]
 
-		if followers+1 > primaryCount {
-			// delete primary from regions
-			regions = regions[:primaryIndex+copy(regions[primaryIndex:], regions[primaryIndex+1:])]
+		if len(regions) > 0 {
 			Rules = append(Rules, NewRule(Follower, followers+1-primaryCount, NewConstraintsDirect(NewConstraintDirect("region", In, regions...))))
+		} else {
+			Rules = append(Rules, NewRule(Follower, followers+1-primaryCount, NewConstraintsDirect()))
 		}
 	}
 

diff --git a/ddl/placement/bundle_test.go b/ddl/placement/bundle_test.go
@@ -416,6 +416,21 @@ func (s *testBundleSuite) TestNewBundleFromOptions(c *C) {
 		},
 	})
 
+	tests = append(tests, TestCase{
+		name: "sugar syntax: normal case 2",
+		input: &model.PlacementSettings{
+			PrimaryRegion: "us",
+			Regions:       "us",
+			Schedule:      "majority_in_primary",
+		},
+		output: []*Rule{
+			NewRule(Voter, 2, NewConstraintsDirect(
+				NewConstraintDirect("region", In, "us"),
+			)),
+			NewRule(Follower, 1, NewConstraintsDirect()),
+		},
+	})
+
 	tests = append(tests, TestCase{
 		name: "sugar syntax: few followers",
 		input: &model.PlacementSettings{