From 2a02e6e41dd2ff88e447db7bce1bb4c18c523d47 Mon Sep 17 00:00:00 2001 From: xhe Date: Sat, 2 Oct 2021 08:36:48 +0800 Subject: [PATCH] placement: refine the bundle contruction logic (#28450) --- ddl/db_test.go | 52 ----- ddl/placement/bundle.go | 87 +++----- ddl/placement/bundle_test.go | 269 +++++------------------- ddl/placement/constraint.go | 11 +- ddl/placement/constraints.go | 9 + ddl/placement/rule.go | 104 +-------- ddl/placement/rule_test.go | 111 ++++------ ddl/placement_policy.go | 26 +-- ddl/placement_policy_test.go | 99 +++------ ddl/placement_sql_test.go | 27 +-- executor/show_test.go | 18 +- privilege/privileges/privileges_test.go | 2 +- 12 files changed, 192 insertions(+), 623 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 2507a56f19549..3a5b5cb48a4e4 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -2862,58 +2862,6 @@ func (s *testSerialDBSuite) TestCreateTableWithLike2(c *C) { c.Assert(t1.Meta().TiFlashReplica.AvailablePartitionIDs, DeepEquals, []int64{partition.Definitions[0].ID, partition.Definitions[1].ID}) } -func (s *testSerialDBSuite) TestCreateTableWithSpecialComment(c *C) { - tk := testkit.NewTestKit(c, s.store) - tk.MustExec("use test") - - // case for direct options - tk.MustExec(`DROP TABLE IF EXISTS t`) - tk.MustExec("CREATE TABLE `t` (\n" + - " `a` int(11) DEFAULT NULL\n" + - ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin " + - "/*T![placement] PRIMARY_REGION=\"cn-east-1\" " + - "REGIONS=\"cn-east-1, cn-east-2\" " + - "FOLLOWERS=2 " + - "CONSTRAINTS=\"[+disk=ssd]\" */", - ) - tk.MustQuery(`show create table t`).Check(testutil.RowsWithSep("|", - "t CREATE TABLE `t` (\n"+ - " `a` int(11) DEFAULT NULL\n"+ - ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin "+ - "/*T![placement] PRIMARY_REGION=\"cn-east-1\" "+ - "REGIONS=\"cn-east-1, cn-east-2\" "+ - "FOLLOWERS=2 "+ - "CONSTRAINTS=\"[+disk=ssd]\" */", - )) - - // case for policy - tk.MustExec(`DROP TABLE IF EXISTS t`) - tk.MustExec("create placement policy x " + - "PRIMARY_REGION=\"cn-east-1\" " + - "REGIONS=\"cn-east-1, cn-east-2\" " + - "FOLLOWERS=2 " + - "CONSTRAINTS=\"[+disk=ssd]\" ") - tk.MustExec("create table t(a int)" + - "/*T![placement] PLACEMENT POLICY=`x` */") - tk.MustQuery(`show create table t`).Check(testutil.RowsWithSep("|", - "t CREATE TABLE `t` (\n"+ - " `a` int(11) DEFAULT NULL\n"+ - ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin "+ - "/*T![placement] PLACEMENT POLICY=`x` */", - )) - - // case for policy with quotes - tk.MustExec(`DROP TABLE IF EXISTS t`) - tk.MustExec("create table t(a int)" + - "/*T![placement] PLACEMENT POLICY=\"x\" */") - tk.MustQuery(`show create table t`).Check(testutil.RowsWithSep("|", - "t CREATE TABLE `t` (\n"+ - " `a` int(11) DEFAULT NULL\n"+ - ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin "+ - "/*T![placement] PLACEMENT POLICY=`x` */", - )) -} - func (s *testSerialDBSuite) TestCreateTable(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") diff --git a/ddl/placement/bundle.go b/ddl/placement/bundle.go index 3994e04ad3ca3..1817d5580d0fa 100644 --- a/ddl/placement/bundle.go +++ b/ddl/placement/bundle.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "math" + "sort" "strconv" "strings" @@ -65,9 +66,7 @@ func NewBundleFromConstraintsOptions(options *model.PlacementSettings) (*Bundle, leaderConstraints := options.LeaderConstraints learnerConstraints := options.LearnerConstraints followerConstraints := options.FollowerConstraints - voterConstraints := options.VoterConstraints followerCount := options.Followers - voterCount := options.Voters learnerCount := options.Learners CommonConstraints, err := NewConstraintsFromYaml([]byte(constraints)) @@ -90,21 +89,6 @@ func NewBundleFromConstraintsOptions(options *model.PlacementSettings) (*Bundle, Rules = append(Rules, NewRule(Leader, 1, LeaderConstraints)) } - if voterCount > 0 { - VoterRules, err := NewRules(Voter, voterCount, voterConstraints) - if err != nil { - return nil, fmt.Errorf("%w: invalid VoterConstraints", err) - } - for _, rule := range VoterRules { - for _, cnst := range CommonConstraints { - if err := rule.Constraints.Add(cnst); err != nil { - return nil, fmt.Errorf("%w: VoterConstraints conflicts with Constraints", err) - } - } - } - Rules = append(Rules, VoterRules...) - } - if followerCount > 0 { FollowerRules, err := NewRules(Follower, followerCount, followerConstraints) if err != nil { @@ -144,7 +128,7 @@ func NewBundleFromSugarOptions(options *model.PlacementSettings) (*Bundle, error return nil, fmt.Errorf("%w: options can not be nil", ErrInvalidPlacementOptions) } - if len(options.LeaderConstraints) > 0 || len(options.LearnerConstraints) > 0 || len(options.FollowerConstraints) > 0 || len(options.VoterConstraints) > 0 || options.Learners > 0 || options.Voters > 0 { + if len(options.LeaderConstraints) > 0 || len(options.LearnerConstraints) > 0 || len(options.FollowerConstraints) > 0 || len(options.Constraints) > 0 || options.Learners > 0 { return nil, fmt.Errorf("%w: should be PRIMARY_REGION=.. REGIONS=.. FOLLOWERS=.. SCHEDULE=.., mixed other constraints into options %s", ErrInvalidPlacementOptions, options) } @@ -167,60 +151,39 @@ func NewBundleFromSugarOptions(options *model.PlacementSettings) (*Bundle, error } schedule := options.Schedule - var constraints Constraints - var err error + // 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) + } + + var Rules []*Rule - Rules := []*Rule{} switch strings.ToLower(schedule) { case "", "even": - constraints, err = NewConstraints([]string{fmt.Sprintf("+region=%s", primaryRegion)}) - if err != nil { - return nil, fmt.Errorf("%w: invalid PrimaryRegion '%s'", err, primaryRegion) + primaryCount := uint64(math.Ceil(float64(followers+1) / float64(len(regions)))) + Rules = append(Rules, NewRule(Voter, primaryCount, NewConstraintsDirect(NewConstraintDirect("region", In, primaryRegion)))) + + if len(regions) > 1 { + // delete primary from regions + regions = regions[:primaryIndex+copy(regions[primaryIndex:], regions[primaryIndex+1:])] + Rules = append(Rules, NewRule(Follower, followers+1-primaryCount, NewConstraintsDirect(NewConstraintDirect("region", In, regions...)))) } - Rules = append(Rules, NewRule(Leader, 1, constraints)) case "majority_in_primary": - // We already have the leader, so we need to calculate how many additional followers - // need to be in the primary region for quorum - followersInPrimary := uint64(math.Ceil(float64(followers) / 2)) - constraints, err = NewConstraints([]string{fmt.Sprintf("+region=%s", primaryRegion)}) - if err != nil { - return nil, fmt.Errorf("%w: invalid PrimaryRegion, '%s'", err, primaryRegion) + // calculate how many replicas need to be in the primary region for quorum + primaryCount := uint64(math.Ceil(float64(followers+1)/2 + 1)) + Rules = append(Rules, NewRule(Voter, primaryCount, NewConstraintsDirect(NewConstraintDirect("region", In, primaryRegion)))) + + if len(regions) > 1 { + // delete primary from regions + regions = regions[:primaryIndex+copy(regions[primaryIndex:], regions[primaryIndex+1:])] + Rules = append(Rules, NewRule(Follower, followers+1-primaryCount, NewConstraintsDirect(NewConstraintDirect("region", In, regions...)))) } - Rules = append(Rules, NewRule(Leader, 1, constraints)) - Rules = append(Rules, NewRule(Follower, followersInPrimary, constraints)) - // even split the remaining followers - followers = followers - followersInPrimary default: return nil, fmt.Errorf("%w: unsupported schedule %s", ErrInvalidPlacementOptions, schedule) } - if uint64(len(regions)) > followers { - return nil, fmt.Errorf("%w: remain %d region to schedule, only %d follower left", ErrInvalidPlacementOptions, uint64(len(regions)), followers) - } - - if len(regions) == 0 { - constraints, err := NewConstraints(nil) - if err != nil { - return nil, err - } - Rules = append(Rules, NewRule(Follower, followers, constraints)) - } else { - count := followers / uint64(len(regions)) - rem := followers - count*uint64(len(regions)) - for _, region := range regions { - constraints, err = NewConstraints([]string{fmt.Sprintf("+region=%s", region)}) - if err != nil { - return nil, fmt.Errorf("%w: invalid region of 'Regions', '%s'", err, region) - } - replica := count - if rem > 0 { - replica += 1 - rem-- - } - Rules = append(Rules, NewRule(Follower, replica, constraints)) - } - } - return &Bundle{Rules: Rules}, nil } diff --git a/ddl/placement/bundle_test.go b/ddl/placement/bundle_test.go index 0ddde1816f708..cc782dadfd746 100644 --- a/ddl/placement/bundle_test.go +++ b/ddl/placement/bundle_test.go @@ -535,7 +535,7 @@ func (s *testBundleSuite) TestString(c *C) { c.Assert(err, IsNil) bundle.Rules = append(rules1, rules2...) - c.Assert(bundle.String(), Equals, `{"group_id":"TiDB_DDL_1","group_index":0,"group_override":false,"rules":[{"group_id":"","id":"","start_key":"","end_key":"","role":"voter","count":3,"label_constraints":[{"key":"zone","op":"in","values":["sh"]}]},{"group_id":"","id":"","start_key":"","end_key":"","role":"voter","count":4,"label_constraints":[{"key":"zone","op":"notIn","values":["sh"]},{"key":"zone","op":"in","values":["bj"]}]}]}`) + c.Assert(bundle.String(), Equals, `{"group_id":"TiDB_DDL_1","group_index":0,"group_override":false,"rules":[{"group_id":"","id":"","start_key":"","end_key":"","role":"voter","count":3,"label_constraints":[{"key":"zone","op":"in","values":["sh"]}],"location_labels":["region","zone","rack","host"],"isolation_level":"region"},{"group_id":"","id":"","start_key":"","end_key":"","role":"voter","count":4,"label_constraints":[{"key":"zone","op":"notIn","values":["sh"]},{"key":"zone","op":"in","values":["bj"]}],"location_labels":["region","zone","rack","host"],"isolation_level":"region"}]}`) c.Assert(failpoint.Enable("github.com/pingcap/tidb/ddl/placement/MockMarshalFailure", `return(true)`), IsNil) defer func() { @@ -580,40 +580,36 @@ func (s *testBundleSuite) TestNewBundleFromOptions(c *C) { name: "sugar syntax: normal case 1", input: &model.PlacementSettings{ PrimaryRegion: "us", + Regions: "us", }, output: []*Rule{ - { - Role: Leader, - Constraints: Constraints{ - { - Key: "region", - Op: In, - Values: []string{"us"}, - }, - }, - Count: 1, - }, - { - Role: Follower, - Constraints: Constraints{}, - Count: 2, - }, + NewRule(Voter, 3, NewConstraintsDirect( + NewConstraintDirect("region", In, "us"), + )), }, }) tests = append(tests, TestCase{ - name: "sugar syntax: invalid followers", + name: "sugar syntax: few followers", input: &model.PlacementSettings{ PrimaryRegion: "us", - Regions: "us,sh,bj", + Regions: "bj,sh,us", + }, + output: []*Rule{ + NewRule(Voter, 1, NewConstraintsDirect( + NewConstraintDirect("region", In, "us"), + )), + NewRule(Follower, 2, NewConstraintsDirect( + NewConstraintDirect("region", In, "bj", "sh"), + )), }, - err: ErrInvalidPlacementOptions, }) tests = append(tests, TestCase{ name: "sugar syntax: wrong schedule prop", input: &model.PlacementSettings{ PrimaryRegion: "us", + Regions: "us", Schedule: "wrong", }, err: ErrInvalidPlacementOptions, @@ -623,8 +619,9 @@ func (s *testBundleSuite) TestNewBundleFromOptions(c *C) { name: "sugar syntax: invalid region name 1", input: &model.PlacementSettings{ PrimaryRegion: ",=,", + Regions: ",=,", }, - err: ErrInvalidConstraintFormat, + err: ErrInvalidPlacementOptions, }) tests = append(tests, TestCase{ @@ -633,68 +630,32 @@ func (s *testBundleSuite) TestNewBundleFromOptions(c *C) { PrimaryRegion: "f", Regions: ",=", }, - err: ErrInvalidConstraintFormat, - }) - - tests = append(tests, TestCase{ - name: "sugar syntax: invalid region name 3", - input: &model.PlacementSettings{ - PrimaryRegion: ",=", - Followers: 5, - Schedule: "majority_in_primary", - }, - err: ErrInvalidConstraintFormat, + err: ErrInvalidPlacementOptions, }) tests = append(tests, TestCase{ name: "sugar syntax: invalid region name 4", input: &model.PlacementSettings{ PrimaryRegion: "", + Regions: "g", }, - output: []*Rule{}, + err: ErrInvalidPlacementOptions, }) tests = append(tests, TestCase{ name: "sugar syntax: normal case 2", input: &model.PlacementSettings{ PrimaryRegion: "us", - Regions: "us,sh", + Regions: "sh,us", Followers: 5, }, output: []*Rule{ - { - Role: Leader, - Constraints: Constraints{ - { - Key: "region", - Op: In, - Values: []string{"us"}, - }, - }, - Count: 1, - }, - { - Role: Follower, - Constraints: Constraints{ - { - Key: "region", - Op: In, - Values: []string{"us"}, - }, - }, - Count: 3, - }, - { - Role: Follower, - Constraints: Constraints{ - { - Key: "region", - Op: In, - Values: []string{"sh"}, - }, - }, - Count: 2, - }, + NewRule(Voter, 3, NewConstraintsDirect( + NewConstraintDirect("region", In, "us"), + )), + NewRule(Follower, 3, NewConstraintsDirect( + NewConstraintDirect("region", In, "sh"), + )), }, }) tests = append(tests, tests[len(tests)-1]) @@ -704,56 +665,18 @@ func (s *testBundleSuite) TestNewBundleFromOptions(c *C) { tests = append(tests, TestCase{ name: "sugar syntax: majority schedule", input: &model.PlacementSettings{ - PrimaryRegion: "us", + PrimaryRegion: "sh", Regions: "bj,sh", Followers: 5, Schedule: "majority_in_primary", }, output: []*Rule{ - { - Role: Leader, - Constraints: Constraints{ - { - Key: "region", - Op: In, - Values: []string{"us"}, - }, - }, - Count: 1, - }, - { - Role: Follower, - Constraints: Constraints{ - { - Key: "region", - Op: In, - Values: []string{"us"}, - }, - }, - Count: 3, - }, - { - Role: Follower, - Constraints: Constraints{ - { - Key: "region", - Op: In, - Values: []string{"bj"}, - }, - }, - Count: 1, - }, - { - Role: Follower, - Constraints: Constraints{ - { - Key: "region", - Op: In, - Values: []string{"sh"}, - }, - }, - Count: 1, - }, + NewRule(Voter, 4, NewConstraintsDirect( + NewConstraintDirect("region", In, "sh"), + )), + NewRule(Follower, 2, NewConstraintsDirect( + NewConstraintDirect("region", In, "bj"), + )), }, }) @@ -764,60 +687,12 @@ func (s *testBundleSuite) TestNewBundleFromOptions(c *C) { Followers: 2, }, output: []*Rule{ - { - Role: Leader, - Constraints: Constraints{ - { - Key: "region", - Op: In, - Values: []string{"us"}, - }, - }, - Count: 1, - }, - { - Role: Follower, - Constraints: Constraints{ - { - Key: "region", - Op: In, - Values: []string{"us"}, - }, - }, - Count: 2, - }, - }, - }) - - tests = append(tests, TestCase{ - name: "direct syntax: normal case 2", - input: &model.PlacementSettings{ - Constraints: "[+region=us]", - Voters: 2, - }, - output: []*Rule{ - { - Role: Leader, - Constraints: Constraints{ - { - Key: "region", - Op: In, - Values: []string{"us"}, - }, - }, - Count: 1, - }, - { - Role: Voter, - Constraints: Constraints{ - { - Key: "region", - Op: In, - Values: []string{"us"}, - }, - }, - Count: 2, - }, + NewRule(Leader, 1, NewConstraintsDirect( + NewConstraintDirect("region", In, "us"), + )), + NewRule(Follower, 2, NewConstraintsDirect( + NewConstraintDirect("region", In, "us"), + )), }, }) @@ -829,39 +704,15 @@ func (s *testBundleSuite) TestNewBundleFromOptions(c *C) { Learners: 2, }, output: []*Rule{ - { - Role: Leader, - Constraints: Constraints{ - { - Key: "region", - Op: In, - Values: []string{"us"}, - }, - }, - Count: 1, - }, - { - Role: Follower, - Constraints: Constraints{ - { - Key: "region", - Op: In, - Values: []string{"us"}, - }, - }, - Count: 2, - }, - { - Role: Learner, - Constraints: Constraints{ - { - Key: "region", - Op: In, - Values: []string{"us"}, - }, - }, - Count: 2, - }, + NewRule(Leader, 1, NewConstraintsDirect( + NewConstraintDirect("region", In, "us"), + )), + NewRule(Follower, 2, NewConstraintsDirect( + NewConstraintDirect("region", In, "us"), + )), + NewRule(Learner, 2, NewConstraintsDirect( + NewConstraintDirect("region", In, "us"), + )), }, }) @@ -875,16 +726,6 @@ func (s *testBundleSuite) TestNewBundleFromOptions(c *C) { err: ErrConflictingConstraints, }) - tests = append(tests, TestCase{ - name: "direct syntax: conflicts 2", - input: &model.PlacementSettings{ - Constraints: "[+region=us]", - VoterConstraints: "[-region=us]", - Voters: 2, - }, - err: ErrConflictingConstraints, - }) - tests = append(tests, TestCase{ name: "direct syntax: conflicts 3", input: &model.PlacementSettings{ @@ -926,16 +767,6 @@ func (s *testBundleSuite) TestNewBundleFromOptions(c *C) { err: ErrInvalidConstraintsFormat, }) - tests = append(tests, TestCase{ - name: "direct syntax: invalid format 3", - input: &model.PlacementSettings{ - Constraints: "[+region=us]", - VoterConstraints: "-region=us]", - Voters: 2, - }, - err: ErrInvalidConstraintsFormat, - }) - tests = append(tests, TestCase{ name: "direct syntax: invalid format 4", input: &model.PlacementSettings{ diff --git a/ddl/placement/constraint.go b/ddl/placement/constraint.go index 3263f104dd668..49970eb31570d 100644 --- a/ddl/placement/constraint.go +++ b/ddl/placement/constraint.go @@ -81,10 +81,19 @@ func NewConstraint(label string) (Constraint, error) { r.Key = key r.Op = op - r.Values = []string{val} + r.Values = strings.Split(val, ",") return r, nil } +// NewConstraintDirect will create a Constraint from argument directly. +func NewConstraintDirect(key string, op ConstraintOp, val ...string) Constraint { + return Constraint{ + Key: key, + Op: op, + Values: val, + } +} + // Restore converts a Constraint to a string. func (c *Constraint) Restore() (string, error) { var sb strings.Builder diff --git a/ddl/placement/constraints.go b/ddl/placement/constraints.go index fa4dbcf02a613..87619a8df32df 100644 --- a/ddl/placement/constraints.go +++ b/ddl/placement/constraints.go @@ -26,6 +26,10 @@ type Constraints []Constraint // NewConstraints will check each labels, and build the Constraints. func NewConstraints(labels []string) (Constraints, error) { + if len(labels) == 0 { + return nil, nil + } + constraints := make(Constraints, 0, len(labels)) for _, str := range labels { label, err := NewConstraint(strings.TrimSpace(str)) @@ -52,6 +56,11 @@ func NewConstraintsFromYaml(c []byte) (Constraints, error) { return NewConstraints(constraints) } +// NewConstraintsDirect is a helper for creating new constraints from individual constraint. +func NewConstraintsDirect(c ...Constraint) Constraints { + return c +} + // Restore converts label constraints to a string. func (constraints *Constraints) Restore() (string, error) { var sb strings.Builder diff --git a/ddl/placement/rule.go b/ddl/placement/rule.go index cf94eeba53ca1..216714789aec9 100644 --- a/ddl/placement/rule.go +++ b/ddl/placement/rule.go @@ -50,102 +50,16 @@ type Rule struct { IsolationLevel string `json:"isolation_level,omitempty"` } -type constraintCombineType uint8 - -const ( - listAndList constraintCombineType = 0x0 - dictAndList constraintCombineType = 0x1 -) - -// NewMergeRules constructs []*Rule from a yaml-compatible representation of array or map of constraint1 and constraint2. -// It is quite like NewRules but the common constraint2 can only be a list labels which will be appended to constraint1. -func NewMergeRules(replicas uint64, constr1, constr2 string) ([]*Rule, error) { - var ( - err1, err2, err3 error - combineType constraintCombineType - ) - rules := []*Rule{} - constraintsList1, constraintsList2 := []string{}, []string{} - constraintsDict1 := map[string]int{} - err1, err2 = yaml.UnmarshalStrict([]byte(constr1), &constraintsList1), yaml.UnmarshalStrict([]byte(constr2), &constraintsList2) - if err2 != nil { - // Common constraints can only be a list. - return nil, fmt.Errorf("%w: should be [constraint1, ...] (error %s)", ErrInvalidConstraintsFormat, err2) - } - if err1 != nil { - combineType = 0x01 - err3 = yaml.UnmarshalStrict([]byte(constr1), &constraintsDict1) - if err3 != nil { - return nil, fmt.Errorf("%w: should be [constraint1, ...] (error %s), {constraint1: cnt1, ...} (error %s), or any yaml compatible representation", ErrInvalidConstraintsFormat, err1, err3) - } - } - switch combineType { - case listAndList: - /* - * eg: followerConstraint = ["+zone=sh", "+zone=bj"], constraint = ["+disk=ssd"] - * res: followerConstraint = ["+zone=sh", "+zone=bj", "+disk=ssd"] - */ - if replicas == 0 { - if len(constr1) == 0 { - return rules, nil - } - return rules, fmt.Errorf("%w: should be positive", ErrInvalidConstraintsRelicas) - } - constraintsList1 = append(constraintsList1, constraintsList2...) - labelConstraints, err := NewConstraints(constraintsList1) - if err != nil { - return rules, err - } - rules = append(rules, &Rule{ - Count: int(replicas), - Constraints: labelConstraints, - }) - return rules, nil - case dictAndList: - /* - * eg: followerConstraint = { '+zone=sh, -zone=bj':2, '+zone=sh':1 }, constraint = ['+disk=ssd'] - * res: followerConstraint = { '+zone=sh, -zone=bj, +disk=ssd':2, '+zone=sh, +disk=ssd':1 } - */ - ruleCnt := 0 - for labels, cnt := range constraintsDict1 { - if cnt <= 0 { - return rules, fmt.Errorf("%w: count of labels '%s' should be positive, but got %d", ErrInvalidConstraintsMapcnt, labels, cnt) - } - ruleCnt += cnt - } - if replicas == 0 { - replicas = uint64(ruleCnt) - } - if int(replicas) < ruleCnt { - return rules, fmt.Errorf("%w: should be larger or equal to the number of total replicas, but REPLICAS=%d < total=%d", ErrInvalidConstraintsRelicas, replicas, ruleCnt) - } - for labels, cnt := range constraintsDict1 { - mergeLabels := append(strings.Split(labels, ","), constraintsList2...) - labelConstraints, err := NewConstraints(mergeLabels) - if err != nil { - return rules, err - } - rules = append(rules, &Rule{ - Count: cnt, - Constraints: labelConstraints, - }) - } - remain := int(replicas) - ruleCnt - if remain > 0 { - rules = append(rules, &Rule{ - Count: remain, - }) - } - return rules, nil - } - // empty - return rules, nil -} - // NewRule constructs *Rule from role, count, and constraints. It is here to // consistent the behavior of creating new rules. func NewRule(role PeerRoleType, replicas uint64, cnst Constraints) *Rule { - return &Rule{Role: role, Count: int(replicas), Constraints: cnst} + return &Rule{ + Role: role, + Count: int(replicas), + Constraints: cnst, + LocationLabels: []string{"region", "zone", "rack", "host"}, + IsolationLevel: "region", + } } // NewRules constructs []*Rule from a yaml-compatible representation of @@ -213,3 +127,7 @@ func (r *Rule) Clone() *Rule { *n = *r return n } + +func (r *Rule) String() string { + return fmt.Sprintf("%+v", *r) +} diff --git a/ddl/placement/rule_test.go b/ddl/placement/rule_test.go index 5abebd927a8a2..9432448127a4a 100644 --- a/ddl/placement/rule_test.go +++ b/ddl/placement/rule_test.go @@ -15,7 +15,6 @@ package placement import ( - "encoding/json" "errors" . "github.com/pingcap/check" @@ -34,25 +33,20 @@ func (t *testRuleSuite) TestClone(c *C) { c.Assert(newRule, DeepEquals, &Rule{ID: "121"}) } -func matchRule(r1 *Rule, t2 []*Rule) bool { - for _, r2 := range t2 { - if ok, _ := DeepEquals.Check([]interface{}{r1, r2}, nil); ok { - return true - } - } - return false -} - func matchRules(t1, t2 []*Rule, prefix string, c *C) { - expected, err := json.Marshal(t1) - c.Assert(err, IsNil) - got, err := json.Marshal(t2) - c.Assert(err, IsNil) - comment := Commentf("%s\nexpected %s\nbut got %s", prefix, expected, got) - c.Assert(len(t1), Equals, len(t2), comment) - for _, r1 := range t1 { - comment = Commentf("%s\nerror on %+v\nexpected %s\nbut got %s", prefix, r1, expected, got) - c.Assert(matchRule(r1, t2), IsTrue, comment) + c.Assert(len(t2), Equals, len(t1), Commentf(prefix)) + for i := range t1 { + found := false + for j := range t2 { + ok, _ := DeepEquals.Check([]interface{}{t2[j], t1[i]}, []string{}) + if ok { + found = true + break + } + } + if !found { + c.Errorf("%s\n\ncan not found %d rule\n%+v\n%+v", prefix, i, t1[i], t2) + } } } @@ -71,11 +65,7 @@ func (t *testRuleSuite) TestNewRuleAndNewRules(c *C) { input: "", replicas: 3, output: []*Rule{ - { - Count: 3, - Role: Voter, - Constraints: Constraints{}, - }, + NewRule(Voter, 3, NewConstraintsDirect()), }, }) @@ -86,40 +76,30 @@ func (t *testRuleSuite) TestNewRuleAndNewRules(c *C) { err: ErrInvalidConstraintsRelicas, }) - labels, err := NewConstraints([]string{"+zone=sh", "+zone=sh"}) - c.Assert(err, IsNil) tests = append(tests, TestCase{ name: "normal array constraints", - input: `["+zone=sh", "+zone=sh"]`, + input: `["+zone=sh", "+region=sh"]`, replicas: 3, output: []*Rule{ - { - Count: 3, - Role: Voter, - Constraints: labels, - }, + NewRule(Voter, 3, NewConstraintsDirect( + NewConstraintDirect("zone", In, "sh"), + NewConstraintDirect("region", In, "sh"), + )), }, }) - labels1, err := NewConstraints([]string{"+zone=sh", "-zone=bj"}) - c.Assert(err, IsNil) - labels2, err := NewConstraints([]string{"+zone=sh"}) - c.Assert(err, IsNil) tests = append(tests, TestCase{ name: "normal object constraints", input: `{"+zone=sh,-zone=bj":2, "+zone=sh": 1}`, replicas: 3, output: []*Rule{ - { - Count: 2, - Role: Voter, - Constraints: labels1, - }, - { - Count: 1, - Role: Voter, - Constraints: labels2, - }, + NewRule(Voter, 2, NewConstraintsDirect( + NewConstraintDirect("zone", In, "sh"), + NewConstraintDirect("zone", NotIn, "bj"), + )), + NewRule(Voter, 1, NewConstraintsDirect( + NewConstraintDirect("zone", In, "sh"), + )), }, }) @@ -128,20 +108,14 @@ func (t *testRuleSuite) TestNewRuleAndNewRules(c *C) { input: "{'+zone=sh,-zone=bj':2, '+zone=sh': 1}", replicas: 4, output: []*Rule{ - { - Count: 2, - Role: Voter, - Constraints: labels1, - }, - { - Count: 1, - Role: Voter, - Constraints: labels2, - }, - { - Count: 1, - Role: Voter, - }, + NewRule(Voter, 2, NewConstraintsDirect( + NewConstraintDirect("zone", In, "sh"), + NewConstraintDirect("zone", NotIn, "bj"), + )), + NewRule(Voter, 1, NewConstraintsDirect( + NewConstraintDirect("zone", In, "sh"), + )), + NewRule(Voter, 1, NewConstraintsDirect()), }, }) @@ -149,16 +123,13 @@ func (t *testRuleSuite) TestNewRuleAndNewRules(c *C) { name: "normal object constraints, without count", input: "{'+zone=sh,-zone=bj':2, '+zone=sh': 1}", output: []*Rule{ - { - Count: 2, - Role: Voter, - Constraints: labels1, - }, - { - Count: 1, - Role: Voter, - Constraints: labels2, - }, + NewRule(Voter, 2, NewConstraintsDirect( + NewConstraintDirect("zone", In, "sh"), + NewConstraintDirect("zone", NotIn, "bj"), + )), + NewRule(Voter, 1, NewConstraintsDirect( + NewConstraintDirect("zone", In, "sh"), + )), }, }) diff --git a/ddl/placement_policy.go b/ddl/placement_policy.go index 71f5cdfbc2d5a..ed1c9ef96c5d7 100644 --- a/ddl/placement_policy.go +++ b/ddl/placement_policy.go @@ -67,30 +67,8 @@ func onCreatePlacementPolicy(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64 } func checkPolicyValidation(info *model.PlacementSettings) error { - checkMergeConstraint := func(replica uint64, constr1, constr2 string) error { - // Constr2 only make sense when replica is set (whether it is in the replica field or included in the constr1) - if replica == 0 && constr1 == "" { - return nil - } - if _, err := placement.NewMergeRules(replica, constr1, constr2); err != nil { - return err - } - return nil - } - if err := checkMergeConstraint(1, info.LeaderConstraints, info.Constraints); err != nil { - return err - } - if err := checkMergeConstraint(info.Followers, info.FollowerConstraints, info.Constraints); err != nil { - return err - } - if err := checkMergeConstraint(info.Voters, info.VoterConstraints, info.Constraints); err != nil { - return err - } - if err := checkMergeConstraint(info.Learners, info.LearnerConstraints, info.Constraints); err != nil { - return err - } - // For constraint labels and default region label, they should be checked by `SHOW LABELS` if necessary when it is applied. - return nil + _, err := placement.NewBundleFromOptions(info) + return err } func getPolicyInfo(t *meta.Meta, policyID int64) (*model.PolicyInfo, error) { diff --git a/ddl/placement_policy_test.go b/ddl/placement_policy_test.go index beaa795532eee..0a7f52fcbc75c 100644 --- a/ddl/placement_policy_test.go +++ b/ddl/placement_policy_test.go @@ -52,8 +52,6 @@ func (s *testDBSuite6) TestPlacementPolicy(c *C) { s.dom.DDL().(ddl.DDLForTest).SetHook(hook) tk.MustExec("create placement policy x " + - "PRIMARY_REGION=\"cn-east-1\" " + - "REGIONS=\"cn-east-1,cn-east-2\" " + "LEARNERS=1 " + "LEARNER_CONSTRAINTS=\"[+region=cn-west-1]\" " + "VOTERS=3 " + @@ -62,8 +60,6 @@ func (s *testDBSuite6) TestPlacementPolicy(c *C) { checkFunc := func(policyInfo *model.PolicyInfo) { c.Assert(policyInfo.ID != 0, Equals, true) c.Assert(policyInfo.Name.L, Equals, "x") - c.Assert(policyInfo.PrimaryRegion, Equals, "cn-east-1") - c.Assert(policyInfo.Regions, Equals, "cn-east-1,cn-east-2") c.Assert(policyInfo.Followers, Equals, uint64(0)) c.Assert(policyInfo.FollowerConstraints, Equals, "") c.Assert(policyInfo.Voters, Equals, uint64(3)) @@ -135,64 +131,36 @@ func testGetPolicyByNameFromIS(c *C, ctx sessionctx.Context, policy string) *mod return po } -func (s *testDBSuite6) TestConstraintCompatibility(c *C) { +func (s *testDBSuite6) TestPlacementValidation(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") tk.MustExec("drop placement policy if exists x") cases := []struct { + name string settings string success bool errmsg string }{ - // Dict is not allowed for common constraint. { - settings: "PRIMARY_REGION=\"cn-east-1\" " + - "REGIONS=\"cn-east-1,cn-east-2\" " + - "LEARNERS=1 " + + name: "Dict is not allowed for common constraint", + settings: "LEARNERS=1 " + "LEARNER_CONSTRAINTS=\"[+zone=cn-west-1]\" " + "CONSTRAINTS=\"{'+disk=ssd':2}\"", - errmsg: "invalid label constraints format: should be [constraint1, ...] (error yaml: unmarshal errors:\n line 1: cannot unmarshal !!map into []string)", + errmsg: "invalid label constraints format: 'Constraints' should be [constraint1, ...] or any yaml compatible array representation", }, - // Special constraints may be incompatible with itself. { - settings: "PRIMARY_REGION=\"cn-east-1\" " + - "REGIONS=\"cn-east-1,cn-east-2\" " + - "LEARNERS=1 " + + name: "constraints may be incompatible with itself", + settings: "LEARNERS=1 " + "LEARNER_CONSTRAINTS=\"[+zone=cn-west-1, +zone=cn-west-2]\"", - errmsg: "conflicting label constraints: '+zone=cn-west-2' and '+zone=cn-west-1'", - }, - { - settings: "PRIMARY_REGION=\"cn-east-1\" " + - "REGIONS=\"cn-east-1,cn-east-2\" " + - "LEARNERS=1 " + - "LEARNER_CONSTRAINTS=\"[+zone=cn-west-1, -zone=cn-west-1]\"", - errmsg: "conflicting label constraints: '-zone=cn-west-1' and '+zone=cn-west-1'", + errmsg: "invalid label constraints format: should be [constraint1, ...] (error conflicting label constraints: '+zone=cn-west-2' and '+zone=cn-west-1'), {constraint1: cnt1, ...} (error yaml: unmarshal errors:\n" + + " line 1: cannot unmarshal !!seq into map[string]int), or any yaml compatible representation: invalid LearnerConstraints", }, { settings: "PRIMARY_REGION=\"cn-east-1\" " + - "REGIONS=\"cn-east-1,cn-east-2\" " + - "LEARNERS=1 " + - "LEARNER_CONSTRAINTS=\"[+zone=cn-west-1, +zone=cn-west-1]\"", + "REGIONS=\"cn-east-1,cn-east-2\" ", success: true, }, - // Special constraints may be incompatible with common constraint. - { - settings: "PRIMARY_REGION=\"cn-east-1\" " + - "REGIONS=\"cn-east-1, cn-east-2\" " + - "FOLLOWERS=2 " + - "FOLLOWER_CONSTRAINTS=\"[+zone=cn-east-1]\" " + - "CONSTRAINTS=\"[+zone=cn-east-2]\"", - errmsg: "conflicting label constraints: '+zone=cn-east-2' and '+zone=cn-east-1'", - }, - { - settings: "PRIMARY_REGION=\"cn-east-1\" " + - "REGIONS=\"cn-east-1, cn-east-2\" " + - "FOLLOWERS=2 " + - "FOLLOWER_CONSTRAINTS=\"[+zone=cn-east-1]\" " + - "CONSTRAINTS=\"[+disk=ssd,-zone=cn-east-1]\"", - errmsg: "conflicting label constraints: '-zone=cn-east-1' and '+zone=cn-east-1'", - }, } // test for create @@ -203,23 +171,23 @@ func (s *testDBSuite6) TestConstraintCompatibility(c *C) { tk.MustExec("drop placement policy if exists x") } else { err := tk.ExecToErr(sql) - c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, ca.errmsg) + c.Assert(err, NotNil, Commentf(ca.name)) + c.Assert(err.Error(), Equals, ca.errmsg, Commentf(ca.name)) } } // test for alter - tk.MustExec("create placement policy x regions=\"cn-east1,cn-east\"") + tk.MustExec("create placement policy x primary_region=\"cn-east-1\" regions=\"cn-east-1,cn-east\"") for _, ca := range cases { sql := fmt.Sprintf("%s %s", "alter placement policy x", ca.settings) if ca.success { tk.MustExec(sql) - tk.MustExec("alter placement policy x regions=\"cn-east1,cn-east\"") + tk.MustExec("alter placement policy x primary_region=\"cn-east-1\" regions=\"cn-east-1,cn-east\"") } else { err := tk.ExecToErr(sql) c.Assert(err, NotNil) c.Assert(err.Error(), Equals, ca.errmsg) - tk.MustQuery("show placement where target='POLICY x'").Check(testkit.Rows("POLICY x REGIONS=\"cn-east1,cn-east\"")) + tk.MustQuery("show placement where target='POLICY x'").Check(testkit.Rows("POLICY x PRIMARY_REGION=\"cn-east-1\" REGIONS=\"cn-east-1,cn-east\"")) } } tk.MustExec("drop placement policy x") @@ -229,18 +197,18 @@ func (s *testDBSuite6) TestAlterPlacementPolicy(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") tk.MustExec("drop placement policy if exists x") - tk.MustExec("create placement policy x primary_region=\"cn-east-1\" regions=\"cn-east1,cn-east\"") + tk.MustExec("create placement policy x primary_region=\"cn-east-1\" regions=\"cn-east-1,cn-east\"") defer tk.MustExec("drop placement policy if exists x") // test for normal cases - tk.MustExec("alter placement policy x REGIONS=\"bj,sh\"") - tk.MustQuery("show placement where target='POLICY x'").Check(testkit.Rows("POLICY x REGIONS=\"bj,sh\"")) + tk.MustExec("alter placement policy x PRIMARY_REGION=\"bj\" REGIONS=\"bj,sh\"") + tk.MustQuery("show placement where target='POLICY x'").Check(testkit.Rows("POLICY x PRIMARY_REGION=\"bj\" REGIONS=\"bj,sh\"")) tk.MustExec("alter placement policy x " + "PRIMARY_REGION=\"bj\" " + - "REGIONS=\"sh\" " + + "REGIONS=\"bj\" " + "SCHEDULE=\"EVEN\"") - tk.MustQuery("show placement where target='POLICY x'").Check(testkit.Rows("POLICY x PRIMARY_REGION=\"bj\" REGIONS=\"sh\" SCHEDULE=\"EVEN\"")) + tk.MustQuery("show placement where target='POLICY x'").Check(testkit.Rows("POLICY x PRIMARY_REGION=\"bj\" REGIONS=\"bj\" SCHEDULE=\"EVEN\"")) tk.MustExec("alter placement policy x " + "LEADER_CONSTRAINTS=\"[+region=us-east-1]\" " + @@ -273,19 +241,16 @@ func (s *testDBSuite6) TestCreateTableWithPlacementPolicy(c *C) { // Direct placement option: special constraints may be incompatible with common constraint. _, err := tk.Exec("create table t(a int) " + - "PRIMARY_REGION=\"cn-east-1\" " + - "REGIONS=\"cn-east-1, cn-east-2\" " + "FOLLOWERS=2 " + "FOLLOWER_CONSTRAINTS=\"[+zone=cn-east-1]\" " + "CONSTRAINTS=\"[+disk=ssd,-zone=cn-east-1]\"") c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "conflicting label constraints: '-zone=cn-east-1' and '+zone=cn-east-1'") + c.Assert(err, ErrorMatches, ".*conflicting label constraints.*") tk.MustExec("create table t(a int) " + "PRIMARY_REGION=\"cn-east-1\" " + "REGIONS=\"cn-east-1, cn-east-2\" " + - "FOLLOWERS=2 " + - "CONSTRAINTS=\"[+disk=ssd]\"") + "FOLLOWERS=2 ") tbl := testGetTableByName(c, tk.Se, "test", "t") c.Assert(tbl, NotNil) @@ -301,7 +266,7 @@ func (s *testDBSuite6) TestCreateTableWithPlacementPolicy(c *C) { c.Assert(policySetting.VoterConstraints, Equals, "") c.Assert(policySetting.Learners, Equals, uint64(0)) c.Assert(policySetting.LearnerConstraints, Equals, "") - c.Assert(policySetting.Constraints, Equals, "[+disk=ssd]") + c.Assert(policySetting.Constraints, Equals, "") c.Assert(policySetting.Schedule, Equals, "") } checkFunc(tbl.Meta().DirectPlacementOpts) @@ -321,8 +286,6 @@ func (s *testDBSuite6) TestCreateTableWithPlacementPolicy(c *C) { tk.MustGetErrCode("create table t(a int)"+ "PLACEMENT POLICY=\"x\"", mysql.ErrPlacementPolicyNotExists) tk.MustExec("create placement policy x " + - "PRIMARY_REGION=\"cn-east-1\" " + - "REGIONS=\"cn-east-1, cn-east-2\" " + "FOLLOWERS=2 " + "CONSTRAINTS=\"[+disk=ssd]\" ") tk.MustExec("create table t(a int)" + @@ -335,19 +298,7 @@ func (s *testDBSuite6) TestCreateTableWithPlacementPolicy(c *C) { c.Assert(tbl.Meta().PlacementPolicyRef.ID != 0, Equals, true) tk.MustExec("drop table if exists t") - // Only direct placement options should check the compatibility itself. - _, err = tk.Exec("create table t(a int)" + - "PRIMARY_REGION=\"cn-east-1\" " + - "REGIONS=\"cn-east-1, cn-east-2\" " + - "FOLLOWERS=2 " + - "FOLLOWER_CONSTRAINTS=\"[+zone=cn-east-1]\" " + - "CONSTRAINTS=\"[+disk=ssd, -zone=cn-east-1]\" ") - c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "conflicting label constraints: '-zone=cn-east-1' and '+zone=cn-east-1'") - tk.MustExec("create table t(a int)" + - "PRIMARY_REGION=\"cn-east-1\" " + - "REGIONS=\"cn-east-1, cn-east-2\" " + "FOLLOWERS=2 " + "CONSTRAINTS=\"[+disk=ssd]\" ") @@ -356,8 +307,8 @@ func (s *testDBSuite6) TestCreateTableWithPlacementPolicy(c *C) { c.Assert(tbl.Meta().DirectPlacementOpts, NotNil) checkFunc = func(policySetting *model.PlacementSettings) { - c.Assert(policySetting.PrimaryRegion, Equals, "cn-east-1") - c.Assert(policySetting.Regions, Equals, "cn-east-1, cn-east-2") + c.Assert(policySetting.PrimaryRegion, Equals, "") + c.Assert(policySetting.Regions, Equals, "") c.Assert(policySetting.Followers, Equals, uint64(2)) c.Assert(policySetting.FollowerConstraints, Equals, "") c.Assert(policySetting.Voters, Equals, uint64(0)) diff --git a/ddl/placement_sql_test.go b/ddl/placement_sql_test.go index 3b45e67ffc2e2..b9de3af147d04 100644 --- a/ddl/placement_sql_test.go +++ b/ddl/placement_sql_test.go @@ -731,8 +731,8 @@ func (s *testDBSuite6) TestCreateSchemaWithPlacement(c *C) { tk.Se.GetSessionVars().EnableAlterPlacement = false }() - tk.MustExec(`CREATE SCHEMA SchemaDirectPlacementTest PRIMARY_REGION='nl' REGIONS = "se,nz" FOLLOWERS=3`) - tk.MustQuery("SHOW CREATE SCHEMA schemadirectplacementtest").Check(testkit.Rows("SchemaDirectPlacementTest CREATE DATABASE `SchemaDirectPlacementTest` /*!40100 DEFAULT CHARACTER SET utf8mb4 */ PRIMARY_REGION=\"nl\" REGIONS=\"se,nz\" FOLLOWERS=3")) + tk.MustExec(`CREATE SCHEMA SchemaDirectPlacementTest PRIMARY_REGION='se' REGIONS = "se,nz" FOLLOWERS=3`) + tk.MustQuery("SHOW CREATE SCHEMA schemadirectplacementtest").Check(testkit.Rows("SchemaDirectPlacementTest CREATE DATABASE `SchemaDirectPlacementTest` /*!40100 DEFAULT CHARACTER SET utf8mb4 */ PRIMARY_REGION=\"se\" REGIONS=\"se,nz\" FOLLOWERS=3")) tk.MustExec(`CREATE PLACEMENT POLICY PolicySchemaTest LEADER_CONSTRAINTS = "[+region=nl]" FOLLOWER_CONSTRAINTS="[+region=se]" FOLLOWERS=4 LEARNER_CONSTRAINTS="[+region=be]" LEARNERS=4`) tk.MustExec(`CREATE PLACEMENT POLICY PolicyTableTest LEADER_CONSTRAINTS = "[+region=tl]" FOLLOWER_CONSTRAINTS="[+region=tf]" FOLLOWERS=2 LEARNER_CONSTRAINTS="[+region=tle]" LEARNERS=1`) @@ -747,21 +747,14 @@ func (s *testDBSuite6) TestCreateSchemaWithPlacement(c *C) { " `a` int(10) unsigned NOT NULL,\n" + " `b` varchar(255) DEFAULT NULL,\n" + " PRIMARY KEY (`a`) /*T![clustered_index] CLUSTERED */\n" + - ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin /*T![placement] PRIMARY_REGION=\"nl\" REGIONS=\"se,nz\" FOLLOWERS=3 */")) - tk.MustExec(`CREATE TABLE SchemaDirectPlacementTest.UseDirectPlacement (a int unsigned primary key, b varchar(255)) PRIMARY_REGION="se"`) + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin /*T![placement] PRIMARY_REGION=\"se\" REGIONS=\"se,nz\" FOLLOWERS=3 */")) + tk.MustExec(`CREATE TABLE SchemaDirectPlacementTest.UseDirectPlacement (a int unsigned primary key, b varchar(255)) PRIMARY_REGION="se" REGIONS="se"`) tk.MustQuery(`SHOW CREATE TABLE SchemaDirectPlacementTest.UseDirectPlacement`).Check(testkit.Rows( "UseDirectPlacement CREATE TABLE `UseDirectPlacement` (\n" + " `a` int(10) unsigned NOT NULL,\n" + " `b` varchar(255) DEFAULT NULL,\n" + " PRIMARY KEY (`a`) /*T![clustered_index] CLUSTERED */\n" + - ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin /*T![placement] PRIMARY_REGION=\"se\" */")) - tk.MustExec(`CREATE TABLE SchemaDirectPlacementTest.UsePolicy (a int unsigned primary key, b varchar(255)) PLACEMENT POLICY = "PolicyTableTest"`) - tk.MustQuery(`SHOW CREATE TABLE SchemaDirectPlacementTest.UsePolicy`).Check(testkit.Rows( - "UsePolicy CREATE TABLE `UsePolicy` (\n" + - " `a` int(10) unsigned NOT NULL,\n" + - " `b` varchar(255) DEFAULT NULL,\n" + - " PRIMARY KEY (`a`) /*T![clustered_index] CLUSTERED */\n" + - ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin /*T![placement] PLACEMENT POLICY=`PolicyTableTest` */")) + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin /*T![placement] PRIMARY_REGION=\"se\" REGIONS=\"se\" */")) tk.MustExec(`CREATE TABLE SchemaPolicyPlacementTest.UseSchemaDefault (a int unsigned primary key, b varchar(255))`) tk.MustQuery(`SHOW CREATE TABLE SchemaPolicyPlacementTest.UseSchemaDefault`).Check(testkit.Rows( @@ -770,13 +763,7 @@ func (s *testDBSuite6) TestCreateSchemaWithPlacement(c *C) { " `b` varchar(255) DEFAULT NULL,\n" + " PRIMARY KEY (`a`) /*T![clustered_index] CLUSTERED */\n" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin /*T![placement] PLACEMENT POLICY=`PolicySchemaTest` */")) - tk.MustExec(`CREATE TABLE SchemaPolicyPlacementTest.UseDirectPlacement (a int unsigned primary key, b varchar(255)) PRIMARY_REGION="se"`) - tk.MustQuery(`SHOW CREATE TABLE SchemaPolicyPlacementTest.UseDirectPlacement`).Check(testkit.Rows( - "UseDirectPlacement CREATE TABLE `UseDirectPlacement` (\n" + - " `a` int(10) unsigned NOT NULL,\n" + - " `b` varchar(255) DEFAULT NULL,\n" + - " PRIMARY KEY (`a`) /*T![clustered_index] CLUSTERED */\n" + - ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin /*T![placement] PRIMARY_REGION=\"se\" */")) + tk.MustExec(`CREATE TABLE SchemaPolicyPlacementTest.UsePolicy (a int unsigned primary key, b varchar(255)) PLACEMENT POLICY = "PolicyTableTest"`) tk.MustQuery(`SHOW CREATE TABLE SchemaPolicyPlacementTest.UsePolicy`).Check(testkit.Rows( "UsePolicy CREATE TABLE `UsePolicy` (\n" + @@ -791,7 +778,7 @@ func (s *testDBSuite6) TestCreateSchemaWithPlacement(c *C) { c.Assert(ok, IsTrue) c.Assert(db.PlacementPolicyRef, IsNil) c.Assert(db.DirectPlacementOpts, NotNil) - c.Assert(db.DirectPlacementOpts.PrimaryRegion, Matches, "nl") + c.Assert(db.DirectPlacementOpts.PrimaryRegion, Matches, "se") c.Assert(db.DirectPlacementOpts.Regions, Matches, "se,nz") c.Assert(db.DirectPlacementOpts.Followers, Equals, uint64(3)) c.Assert(db.DirectPlacementOpts.Learners, Equals, uint64(0)) diff --git a/executor/show_test.go b/executor/show_test.go index 3ded6078fb05b..8992895809517 100644 --- a/executor/show_test.go +++ b/executor/show_test.go @@ -947,29 +947,24 @@ func (s *testSuite5) TestShowCreateTable(c *C) { func (s *testAutoRandomSuite) TestShowCreateTablePlacement(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") + defer tk.MustExec(`DROP TABLE IF EXISTS t`) // case for direct opts tk.MustExec(`DROP TABLE IF EXISTS t`) tk.MustExec("create table t(a int) " + - "PRIMARY_REGION=\"cn-east-1\" " + - "REGIONS=\"cn-east-1, cn-east-2\" " + "FOLLOWERS=2 " + "CONSTRAINTS=\"[+disk=ssd]\"") tk.MustQuery(`show create table t`).Check(testutil.RowsWithSep("|", "t CREATE TABLE `t` (\n"+ " `a` int(11) DEFAULT NULL\n"+ ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin "+ - "/*T![placement] PRIMARY_REGION=\"cn-east-1\" "+ - "REGIONS=\"cn-east-1, cn-east-2\" "+ - "FOLLOWERS=2 "+ + "/*T![placement] FOLLOWERS=2 "+ "CONSTRAINTS=\"[+disk=ssd]\" */", )) // case for policy tk.MustExec(`DROP TABLE IF EXISTS t`) tk.MustExec("create placement policy x " + - "PRIMARY_REGION=\"cn-east-1\" " + - "REGIONS=\"cn-east-1, cn-east-2\" " + "FOLLOWERS=2 " + "CONSTRAINTS=\"[+disk=ssd]\" ") tk.MustExec("create table t(a int)" + @@ -981,7 +976,16 @@ func (s *testAutoRandomSuite) TestShowCreateTablePlacement(c *C) { "/*T![placement] PLACEMENT POLICY=`x` */", )) + // case for policy with quotes tk.MustExec(`DROP TABLE IF EXISTS t`) + tk.MustExec("create table t(a int)" + + "/*T![placement] PLACEMENT POLICY=\"x\" */") + tk.MustQuery(`show create table t`).Check(testutil.RowsWithSep("|", + "t CREATE TABLE `t` (\n"+ + " `a` int(11) DEFAULT NULL\n"+ + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin "+ + "/*T![placement] PLACEMENT POLICY=`x` */", + )) } func (s *testAutoRandomSuite) TestShowCreateTableAutoRandom(c *C) { diff --git a/privilege/privileges/privileges_test.go b/privilege/privileges/privileges_test.go index f6192e1a90a14..cf42d0fdbf5e6 100644 --- a/privilege/privileges/privileges_test.go +++ b/privilege/privileges/privileges_test.go @@ -2461,7 +2461,7 @@ func TestPlacementPolicyStmt(t *testing.T) { defer clean() se := newSession(t, store, dbName) mustExec(t, se, "drop placement policy if exists x") - createStmt := "create placement policy x PRIMARY_REGION=\"cn-east-1\" " + createStmt := "create placement policy x PRIMARY_REGION=\"cn-east-1\" REGIONS=\"cn-east-1\"" dropStmt := "drop placement policy if exists x" // high privileged user setting password for other user (passes)