Skip to content

Commit

Permalink
ddl: align placement syntax implementation to RFC (pingcap#18957)
Browse files Browse the repository at this point in the history
  • Loading branch information
@xhebox
xhebox authored Aug 7, 2020
1 parent 437c7f1 commit e75683c
Show file tree
Hide file tree
Showing 5 changed files with 200 additions and 70 deletions.
177 changes: 126 additions & 51 deletions ddl/ddl_api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package ddl
import (
"bytes"
"encoding/hex"
"encoding/json"
"fmt"
"math"
"strconv"
Expand Down Expand Up @@ -5171,9 +5172,110 @@ func (d *ddl) AlterIndexVisibility(ctx sessionctx.Context, ident ast.Ident, inde
return errors.Trace(err)
}

func checkPlacementLabelConstraint(label string) (placement.LabelConstraint, error) {
r := placement.LabelConstraint{}

if len(label) < 4 {
return r, errors.Errorf("label constraint should be in format '{+|-}key=value', but got '%s'", label)
}

var op placement.LabelConstraintOp
switch label[0] {
case '+':
op = placement.In
case '-':
op = placement.NotIn
default:
return r, errors.Errorf("label constraint should be in format '{+|-}key=value', but got '%s'", label)
}

kv := strings.Split(label[1:], "=")
if len(kv) != 2 {
return r, errors.Errorf("label constraint should be in format '{+|-}key=value', but got '%s'", label)
}

key := strings.TrimSpace(kv[0])
if key == "" {
return r, errors.Errorf("label constraint should be in format '{+|-}key=value', but got '%s'", label)
}

val := strings.TrimSpace(kv[1])
if val == "" {
return r, errors.Errorf("label constraint should be in format '{+|-}key=value', but got '%s'", label)
}

r.Key = key
r.Op = op
r.Values = []string{val}
return r, nil
}

func checkPlacementLabelConstraints(rule *placement.Rule, labels []string) error {
for _, str := range labels {
label, err := checkPlacementLabelConstraint(strings.TrimSpace(str))
if err != nil {
return err
}
rule.LabelConstraints = append(rule.LabelConstraints, label)
}
return nil
}

func checkPlacementSpecConstraint(rules []*placement.Rule, rule *placement.Rule, cnstr string) ([]*placement.Rule, error) {
cnstr = strings.TrimSpace(cnstr)
var err error
if len(cnstr) > 0 && cnstr[0] == '[' {
constraints := []string{}

err = json.Unmarshal([]byte(cnstr), &constraints)
if err != nil {
return rules, err
}

err = checkPlacementLabelConstraints(rule, constraints)
if err != nil {
return rules, err
}

rules = append(rules, rule)
} else if len(cnstr) > 0 && cnstr[0] == '{' {
constraints := map[string]int{}
err = json.Unmarshal([]byte(cnstr), &constraints)
if err != nil {
return rules, err
}

for labels, cnt := range constraints {
newRule := &placement.Rule{}
*newRule = *rule
if cnt <= 0 {
err = errors.Errorf("count should be non-positive, but got %d", cnt)
break
}
// TODO: handle or remove rule.Count in later commits
rule.Count -= cnt
newRule.Count = cnt
err = checkPlacementLabelConstraints(newRule, strings.Split(strings.TrimSpace(labels), ","))
if err != nil {
break
}
rules = append(rules, newRule)
}
} else {
err = errors.Errorf("constraint should be a JSON array or object, but got '%s'", cnstr)
}
return rules, err
}

func checkPlacementSpecs(specs []*ast.PlacementSpec) ([]*placement.Rule, error) {
rules := make([]*placement.Rule, 0, len(specs))
for k, spec := range specs {

var err error
var sb strings.Builder
restoreFlags := format.RestoreStringSingleQuotes | format.RestoreKeyWordLowercase | format.RestoreNameBackQuotes
restoreCtx := format.NewRestoreCtx(restoreFlags, &sb)

for _, spec := range specs {
rule := &placement.Rule{
GroupID: placementRuleDefaultGroupID,
Count: int(spec.Replicas),
Expand All @@ -5183,62 +5285,35 @@ func checkPlacementSpecs(specs []*ast.PlacementSpec) ([]*placement.Rule, error)
switch spec.Tp {
case ast.PlacementAdd:
default:
return rules, errors.Errorf("invalid placement spec[%d], unknown action type: %d", k, spec.Tp)
}

switch spec.Role {
case ast.PlacementRoleFollower:
rule.Role = placement.Follower
case ast.PlacementRoleLeader:
rule.Role = placement.Leader
case ast.PlacementRoleLearner:
rule.Role = placement.Learner
case ast.PlacementRoleVoter:
rule.Role = placement.Voter
default:
return rules, errors.Errorf("invalid placement spec[%d], unknown role: %d", k, spec.Role)
}

for _, label := range strings.Split(spec.Constraints, ",") {
label = strings.TrimSpace(label)

if len(label) < 4 {
return rules, errors.Errorf("invalid placement spec[%d], constraint too short to be valid: %s", k, label)
}

var op placement.LabelConstraintOp
switch label[0] {
case '+':
op = placement.In
case '-':
op = placement.NotIn
err = errors.Errorf("unknown action type: %d", spec.Tp)
}

if err == nil {
switch spec.Role {
case ast.PlacementRoleFollower:
rule.Role = placement.Follower
case ast.PlacementRoleLeader:
rule.Role = placement.Leader
case ast.PlacementRoleLearner:
rule.Role = placement.Learner
case ast.PlacementRoleVoter:
rule.Role = placement.Voter
default:
return rules, errors.Errorf("invalid placement spec[%d], unknown operation: %c", k, label[0])
}

kv := strings.Split(label[1:], "=")
if len(kv) != 2 {
return rules, errors.Errorf("invalid placement spec[%d], invalid constraint format: %s", k, label)
err = errors.Errorf("unknown role: %d", spec.Role)
}
}

key := strings.TrimSpace(kv[0])
if key == "" {
return rules, errors.Errorf("invalid placement spec[%d], empty constraint key: %s", k, label)
}
if err == nil {
rules, err = checkPlacementSpecConstraint(rules, rule, spec.Constraints)
}

val := strings.TrimSpace(kv[1])
if val == "" {
return rules, errors.Errorf("invalid placement spec[%d], empty constraint val: %s", k, label)
if err != nil {
sb.Reset()
if e := spec.Restore(restoreCtx); e != nil {
return rules, ErrInvalidPlacementSpec.GenWithStackByArgs("", err)
}

rule.LabelConstraints = append(rule.LabelConstraints, placement.LabelConstraint{
Key: key,
Op: op,
Values: []string{val},
})
return rules, ErrInvalidPlacementSpec.GenWithStackByArgs(sb.String(), err)
}

rules = append(rules, rule)
}
return rules, nil
}
Expand Down
3 changes: 3 additions & 0 deletions ddl/error.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,4 +232,7 @@ var (
ErrTableOptionUnionUnsupported = terror.ClassDDL.New(mysql.ErrTableOptionUnionUnsupported, mysql.MySQLErrName[mysql.ErrTableOptionUnionUnsupported])
// ErrTableOptionInsertMethodUnsupported is returned when create/alter table with insert method option.
ErrTableOptionInsertMethodUnsupported = terror.ClassDDL.New(mysql.ErrTableOptionInsertMethodUnsupported, mysql.MySQLErrName[mysql.ErrTableOptionInsertMethodUnsupported])

// ErrInvalidPlacementSpec is returned when add/alter an invalid placement rule
ErrInvalidPlacementSpec = terror.ClassDDL.New(mysql.ErrInvalidPlacementSpec, mysql.MySQLErrName[mysql.ErrInvalidPlacementSpec])
)
87 changes: 68 additions & 19 deletions ddl/placement_rule_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,79 +33,128 @@ PARTITION BY RANGE (c) (
PARTITION p3 VALUES LESS THAN (21)
);`)

// normal cases
_, err := tk.Exec(`alter table t1 alter partition p0
add placement policy
constraints='+zone=sh'
constraints='["+zone=sh"]'
role=leader
replicas=3`)
c.Assert(err, ErrorMatches, ".*pd unavailable.*")

_, err = tk.Exec(`alter table t1 alter partition p0
add placement policy
constraints='+ zone = sh , - zone = bj '
constraints='["+ zone = sh ", "- zone = bj "]'
role=leader
replicas=3`)
c.Assert(err, ErrorMatches, ".*pd unavailable.*")

_, err = tk.Exec(`alter table t1 alter partition p0
add placement policy
constraints='{"+ zone = sh ": 1}'
role=leader
replicas=3`)
c.Assert(err, ErrorMatches, ".*pd unavailable.*")

_, err = tk.Exec(`alter table t1 alter partition p0
add placement policy
constraints='{"+ zone = sh, -zone = bj ": 1}'
role=leader
replicas=3`)
c.Assert(err, ErrorMatches, ".*pd unavailable.*")

_, err = tk.Exec(`alter table t1 alter partition p0
add placement policy
constraints='{"+ zone = sh ": 1, "- zone = bj": 2}'
role=leader
replicas=3`)
c.Assert(err, ErrorMatches, ".*pd unavailable.*")

// list/dict detection
_, err = tk.Exec(`alter table t1 alter partition p0
add placement policy
constraints=',,,'
role=leader
replicas=3`)
c.Assert(err, ErrorMatches, ".*constraint too short to be valid.*")
c.Assert(err, ErrorMatches, ".*array or object.*")

_, err = tk.Exec(`alter table t1 alter partition p0
add placement policy
constraints='[,,,'
role=leader
replicas=3`)
c.Assert(err, ErrorMatches, ".*invalid character.*")

_, err = tk.Exec(`alter table t1 alter partition p0
add placement policy
constraints='{,,,'
role=leader
replicas=3`)
c.Assert(err, ErrorMatches, ".*invalid character.*")

// checkPlacementSpecConstraint
_, err = tk.Exec(`alter table t1 alter partition p0
add placement policy
constraints='[",,,"]'
role=leader
replicas=3`)
c.Assert(err, ErrorMatches, ".*label constraint should be in format.*")

_, err = tk.Exec(`alter table t1 alter partition p0
add placement policy
constraints='0000'
constraints='["+ "]'
role=leader
replicas=3`)
c.Assert(err, ErrorMatches, ".*unknown operation.*")
c.Assert(err, ErrorMatches, ".*label constraint should be in format.*")

// unknown operation
_, err = tk.Exec(`alter table t1 alter partition p0
add placement policy
constraints='+000'
constraints='["0000"]'
role=leader
replicas=3`)
c.Assert(err, ErrorMatches, ".*invalid constraint format.*")
c.Assert(err, ErrorMatches, ".*label constraint should be in format.*")

// without =
_, err = tk.Exec(`alter table t1 alter partition p0
add placement policy
constraints='+ '
constraints='["+000"]'
role=leader
replicas=3`)
c.Assert(err, ErrorMatches, ".*too short to be valid.*")
c.Assert(err, ErrorMatches, ".*label constraint should be in format.*")

// empty key
_, err = tk.Exec(`alter table t1 alter partition p0
add placement policy
constraints='+ =zone1'
constraints='["+ =zone1"]'
role=leader
replicas=3`)
c.Assert(err, ErrorMatches, ".*empty constraint key.*")
c.Assert(err, ErrorMatches, ".*label constraint should be in format.*")

_, err = tk.Exec(`alter table t1 alter partition p0
add placement policy
constraints='+ = z'
constraints='["+ = z"]'
role=leader
replicas=3`)
c.Assert(err, ErrorMatches, ".*empty constraint key.*")
c.Assert(err, ErrorMatches, ".*label constraint should be in format.*")

// empty value
_, err = tk.Exec(`alter table t1 alter partition p0
add placement policy
constraints='+zone='
constraints='["+zone="]'
role=leader
replicas=3`)
c.Assert(err, ErrorMatches, ".*empty constraint val.*")
c.Assert(err, ErrorMatches, ".*label constraint should be in format.*")

_, err = tk.Exec(`alter table t1 alter partition p0
add placement policy
constraints='+z = '
constraints='["+z = "]'
role=leader
replicas=3`)
c.Assert(err, ErrorMatches, ".*empty constraint val.*")
c.Assert(err, ErrorMatches, ".*label constraint should be in format.*")

_, err = tk.Exec(`alter table t1 alter partition p
add placement policy
constraints='+zone=sh'
constraints='["+zone=sh"]'
role=leader
replicas=3`)
c.Assert(err, ErrorMatches, ".*Unknown partition.*")
Expand All @@ -115,7 +164,7 @@ add placement policy

_, err = tk.Exec(`alter table t1 alter partition p
add placement policy
constraints='+zone=sh'
constraints='["+zone=sh"]'
role=leader
replicas=3`)
c.Assert(ddl.ErrPartitionMgmtOnNonpartitioned.Equal(err), IsTrue)
Expand Down
1 change: 1 addition & 0 deletions errno/errcode.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1080,6 +1080,7 @@ const (
ErrUnsupportedConstraintCheck = 8231
ErrTableOptionUnionUnsupported = 8232
ErrTableOptionInsertMethodUnsupported = 8233
ErrInvalidPlacementSpec = 8234

// TiKV/PD errors.
ErrPDServerTimeout = 9001
Expand Down
2 changes: 2 additions & 0 deletions errno/errname.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1079,6 +1079,8 @@ var MySQLErrName = map[uint16]string{
ErrBRIEImportFailed: "Import failed: %s",
ErrBRIEExportFailed: "Export failed: %s",

ErrInvalidPlacementSpec: "Invalid placement policy '%s': %s",

// TiKV/PD errors.
ErrPDServerTimeout: "PD server timeout",
ErrTiKVServerTimeout: "TiKV server timeout",
Expand Down

0 comments on commit e75683c

Please sign in to comment.