From e415f53ac16195a99555e5ac86564711403a037b Mon Sep 17 00:00:00 2001 From: xhe Date: Wed, 5 Jan 2022 12:30:36 +0800 Subject: [PATCH] placement: close #31271 Signed-off-by: xhe --- ddl/placement/bundle.go | 36 +++++++++++++++++++----------------- ddl/placement/bundle_test.go | 15 +++++++++++++++ 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/ddl/placement/bundle.go b/ddl/placement/bundle.go index 1ceb94410691f..8f03ec1b62ddd 100644 --- 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 index 94c51795f2731..de02ca6cd51eb 100644 --- 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{