diff --git a/pkg/kv/kvserver/allocator/allocator2/BUILD.bazel b/pkg/kv/kvserver/allocator/allocator2/BUILD.bazel index 930531687757..a9b62f8696b7 100644 --- a/pkg/kv/kvserver/allocator/allocator2/BUILD.bazel +++ b/pkg/kv/kvserver/allocator/allocator2/BUILD.bazel @@ -27,8 +27,13 @@ go_test( "memo_helper_test.go", ], args = ["-test.timeout=295s"], + data = glob(["testdata/**"]), embed = [":allocator2"], - deps = ["@com_github_stretchr_testify//require"], + deps = [ + "//pkg/roachpb", + "@com_github_cockroachdb_datadriven//:datadriven", + "@com_github_stretchr_testify//require", + ], ) get_x_data(name = "get_x_data") diff --git a/pkg/kv/kvserver/allocator/allocator2/constraint.go b/pkg/kv/kvserver/allocator/allocator2/constraint.go index faac7f7c6542..f8b3db14cdb9 100644 --- a/pkg/kv/kvserver/allocator/allocator2/constraint.go +++ b/pkg/kv/kvserver/allocator/allocator2/constraint.go @@ -79,22 +79,11 @@ type normalizedSpanConfig struct { // multiple conjunctions. // // TODO(sumeer): - // - Document the above strictness requirement in roachpb.SpanConfig. - // // - For existing clusters this strictness requirement may not be met, so we // will do a structural-normalization to meet this requirement, and if // this structural-normalization is not possible, we will log an error and // not switch the cluster to the new allocator until the operator fixes // their SpanConfigs and retries. - // - // - Write the code for this structural-normalization. It will establish - // subset or non-intersecting relationships between every pair of - // ConstraintsConjunctions, and then try to map CC's in replicaConstraints - // to the containing set in constraints and vice-versa to (a) check for no - // over-satisfaction, (b) split sets in replicaConstraints into subsets - // based on CC's in constraints. See - // https://cockroachlabs.slack.com/archives/D0367JZG864/p1679064458668199 - // for an example where we need to do the latter. // constraints applies to all replicas. constraints []internedConstraintsConjunction @@ -117,13 +106,134 @@ type internedConstraint struct { value stringCode } +func (ic internedConstraint) less(b internedConstraint) bool { + if ic.typ < b.typ { + return true + } + if ic.key < b.key { + return true + } + if ic.value < b.value { + return true + } + return false +} + +// constraints are in increasing order using internedConstraint.less. +type constraintsConj []internedConstraint + +type conjunctionRelationship int + +func (nconf *normalizedSpanConfig) uninternedConfig() roachpb.SpanConfig { + var conf roachpb.SpanConfig + conf.NumReplicas = nconf.numReplicas + conf.NumVoters = nconf.numVoters + makeConstraints := func( + nconstraints []internedConstraintsConjunction) []roachpb.ConstraintsConjunction { + var rv []roachpb.ConstraintsConjunction + for _, ncc := range nconstraints { + var cc roachpb.ConstraintsConjunction + cc.NumReplicas = ncc.numReplicas + for _, c := range ncc.constraints { + cc.Constraints = append(cc.Constraints, roachpb.Constraint{ + Type: c.typ, + Key: nconf.interner.toString(c.key), + Value: nconf.interner.toString(c.value), + }) + } + rv = append(rv, cc) + } + return rv + } + conf.Constraints = makeConstraints(nconf.constraints) + conf.VoterConstraints = makeConstraints(nconf.voterConstraints) + for _, nlp := range nconf.leasePreferences { + var cc []roachpb.Constraint + for _, c := range nlp.constraints { + cc = append(cc, roachpb.Constraint{ + Type: c.typ, + Key: nconf.interner.toString(c.key), + Value: nconf.interner.toString(c.value), + }) + } + conf.LeasePreferences = append(conf.LeasePreferences, roachpb.LeasePreference{Constraints: cc}) + } + return conf +} + +// Relationship between conjunctions used for structural normalization. This +// relationship is solely defined based on the conjunctions, and not based on +// what stores actually match. It simply assumes that if two conjuncts are not +// equal their sets are non-overlapping. This simplifying assumption is made +// since we are only trying to do a best-effort structural normalization. +const ( + conjIntersecting conjunctionRelationship = iota + conjEqualSet + conjStrictSubset + conjStrictSuperset + conjNonIntersecting +) + +func (cc constraintsConj) relationship(b constraintsConj) conjunctionRelationship { + n := len(cc) + m := len(b) + extraInCC := 0 + extraInB := 0 + inBoth := 0 + for i, j := 0, 0; i < n || j < m; { + if i >= n { + extraInB++ + j++ + continue + } + if j >= m { + extraInCC++ + i++ + continue + } + if cc[i] == b[j] { + inBoth++ + i++ + j++ + continue + } + if cc[i].less(b[j]) { + // Found a conjunct that is not in b. + extraInCC++ + i++ + continue + } else { + extraInB++ + j++ + continue + } + } + if extraInCC > 0 && extraInB == 0 { + return conjStrictSubset + } + if extraInB > 0 && extraInCC == 0 { + return conjStrictSuperset + } + // (extraInCC == 0 || extraInBB > 0) && (extraInB == 0 || extraInCC > 0) + // => + // (extraInCC == 0 && extraInB == 0) || (extraInBB > 0 && extraInCC > 0) + if extraInCC == 0 && extraInB == 0 { + return conjEqualSet + } + // (extraInBB > 0 && extraInCC > 0) + if inBoth > 0 { + return conjIntersecting + } + return conjNonIntersecting +} + type internedConstraintsConjunction struct { numReplicas int32 - constraints []internedConstraint + constraints constraintsConj } type internedLeasePreference struct { - constraints []internedConstraint + constraints constraintsConj } // makeNormalizedSpanConfig is called infrequently, when there is a new @@ -153,14 +263,15 @@ func makeNormalizedSpanConfig( lps = append(lps, internedLeasePreference{ constraints: interner.internConstraints(conf.LeasePreferences[i].Constraints)}) } - return &normalizedSpanConfig{ + nConf := &normalizedSpanConfig{ numVoters: conf.NumVoters, numReplicas: conf.NumReplicas, constraints: normalizedConstraints, voterConstraints: normalizedVoterConstraints, leasePreferences: lps, interner: interner, - }, nil + } + return doStructuralNormalization(nConf) } func normalizeConstraints( @@ -203,14 +314,254 @@ func normalizeConstraints( } var rv []internedConstraintsConjunction for i := range nc { - rv = append(rv, internedConstraintsConjunction{ + icc := internedConstraintsConjunction{ numReplicas: nc[i].NumReplicas, constraints: interner.internConstraints(nc[i].Constraints), - }) + } + if len(icc.constraints) > 0 { + sort.Slice(icc.constraints, func(j, k int) bool { + return icc.constraints[j].less(icc.constraints[k]) + }) + j := 0 + // De-dup conjuncts in the conjunction. + for k := 1; k < len(icc.constraints); k++ { + if !(icc.constraints[j] == icc.constraints[k]) { + j++ + icc.constraints[j] = icc.constraints[k] + } + } + icc.constraints = icc.constraints[:j+1] + } + rv = append(rv, icc) } return rv, nil } +// Structural normalization establishes relationships between every pair of +// ConstraintsConjunctions in constraints and voterConstraints, and then tries +// to map conjunctions in voterConstraints to narrower conjunctions in +// constraints. This is done to handle configs which under-specify +// conjunctions in voterConstraints under the assumption that one does not +// need to repeat information provided in constraints (see the new +// "strictness" comment in roachpb.SpanConfig which now requires users to +// repeat the information). +// +// This function does some structural normalization even when returning an +// error. See the under-specified voter constraint examples in the datadriven +// test -- we sometimes see these in production settings, and we want to fix +// ones that we can, and raise an error for users to fix their configs. +func doStructuralNormalization(conf *normalizedSpanConfig) (*normalizedSpanConfig, error) { + if len(conf.constraints) == 0 || len(conf.voterConstraints) == 0 { + return conf, nil + } + // Relationships between each voter constraint and each all replica + // constraint. + type relationshipVoterAndAll struct { + voterIndex int + allIndex int + voterAndAllRel conjunctionRelationship + } + emptyConstraintIndex := -1 + emptyVoterConstraintIndex := -1 + var rels []relationshipVoterAndAll + for i := range conf.voterConstraints { + if len(conf.voterConstraints[i].constraints) == 0 { + emptyVoterConstraintIndex = i + } + for j := range conf.constraints { + if len(conf.constraints[j].constraints) == 0 { + emptyConstraintIndex = j + } + rels = append(rels, relationshipVoterAndAll{ + voterIndex: i, + allIndex: j, + voterAndAllRel: conf.voterConstraints[i].constraints.relationship(conf.constraints[j].constraints), + }) + } + } + // Sort these relationships in the order we want to examine them. + sort.Slice(rels, func(i, j int) bool { + return rels[i].voterAndAllRel < rels[j].voterAndAllRel + }) + // First are the intersecting constraints, which cause an error. + index := 0 + for rels[index].voterAndAllRel == conjIntersecting { + index++ + } + var err error + if index > 0 { + err = errors.Errorf("intersecting conjunctions in constraints and voter constraints") + } + // Even if there was an error, we will continue normalization. + + // For each all-replica constraint, track how many replicas are remaining + // (not already taken by a voter constraint). Additionally, when we find an + // all replica constraint that is a subset of one or more voter constraints, + // and create a new voter constraint, we record the index of that new voter + // constraint in newVoterIndex. + type allReplicaConstraintsInfo struct { + remainingReplicas int32 + newVoterIndex int + } + var allReplicaConstraints []allReplicaConstraintsInfo + for i := range conf.constraints { + allReplicaConstraints = append(allReplicaConstraints, + allReplicaConstraintsInfo{ + // Initially all of numReplicas are remaining. + remainingReplicas: conf.constraints[i].numReplicas, + newVoterIndex: -1, + }) + } + // For each voter replica constraint, we keep the current + // internedConstraintsConjunction. The numReplicas start with 0 and build up + // towards the desired number in the corresponding un-normalized voter + // constraint. In addition, if the voter constraint had a superset + // relationship with one or more all-replica constraint, we may take some of + // its voter count and construct narrower voter constraints. + // additionalReplicas tracks the total count of replicas in such narrower + // voter constraints. + type voterConstraintsAndAdditionalInfo struct { + internedConstraintsConjunction + additionalReplicas int32 + } + var voterConstraints []voterConstraintsAndAdditionalInfo + for _, constraint := range conf.voterConstraints { + constraint.numReplicas = 0 + voterConstraints = append(voterConstraints, voterConstraintsAndAdditionalInfo{ + internedConstraintsConjunction: constraint, + }) + } + // Now resume iterating from index, and consume all relationships that are + // conjEqualSet and conjStrictSubset. + for ; index < len(rels) && rels[index].voterAndAllRel <= conjStrictSubset; index++ { + rel := rels[index] + if rel.voterIndex == emptyVoterConstraintIndex { + // Don't try to satisfy the empty constraint with the corresponding + // empty constraint since the latter may be needed by some other voter + // constraint. + continue + } + remainingAll := allReplicaConstraints[rel.allIndex].remainingReplicas + // NB: we don't bother subtracting + // voterConstraints[rel.voterIndex].additionalReplicas since it is always + // 0 at this point in the code. + neededVoterReplicas := conf.voterConstraints[rel.voterIndex].numReplicas - + voterConstraints[rel.voterIndex].numReplicas + if neededVoterReplicas > 0 && remainingAll > 0 { + // We can satisfy some voter replicas. + toAdd := remainingAll + if toAdd > neededVoterReplicas { + toAdd = neededVoterReplicas + } + voterConstraints[rel.voterIndex].numReplicas += toAdd + allReplicaConstraints[rel.allIndex].remainingReplicas -= toAdd + } + } + // The only relationships remaining are conjStrictSuperset and + // conjNonIntersecting. We don't care about the latter. conjStrictSuperset + // can be used to narrow a voter constraint. As a heuristic we try to even + // out the satisfaction across various constraints. For example, if we have + // a voter constraint with conjunction c1 that needs 2 more replicas, and we + // have all constraints with conjuctions: + // - c1 and c2, with 2 remainingReplicas + // - c1 and c3, with 2 remainingReplicas + // instead of greedily adding a voter constraint c1 and c2 with 2 voter + // replicas, we add both with 1 voter replica each ("load-balancing"). + // + // Before we do the narrowing, we consider the pair of conjunctions that are + // empty: emptyVoterConstraintIndex and emptyConstraintIndex. We don't want + // to narrow unnecessarily, and so if emptyConstraintIndex has some + // remainingReplicas, we take them here. + if emptyVoterConstraintIndex > 0 && emptyConstraintIndex > 0 { + neededReplicas := conf.voterConstraints[emptyVoterConstraintIndex].numReplicas + actualReplicas := voterConstraints[emptyVoterConstraintIndex].numReplicas + remaining := neededReplicas - actualReplicas + if remaining > 0 { + remainingSatisfiable := allReplicaConstraints[emptyConstraintIndex].remainingReplicas + if remainingSatisfiable > 0 { + count := remainingSatisfiable + if count > remaining { + count = remaining + } + voterConstraints[emptyVoterConstraintIndex].numReplicas += count + allReplicaConstraints[emptyConstraintIndex].remainingReplicas -= count + } + } + } + + // The aforementioned "load-balancing" of the satisfaction is why we need + // the outer for loop below. + // + // The outer for loop causes repeated iteration over the strict superset + // relationship. When the inner loop finds a voter constraint that needs + // more replicas, and the subset conjunction in constraints has some + // remaining replicas, we replace the weaker voter constraint with the + // stronger/tighter conjunction for 1 voter. Note that we don't exclude the + // emptyVoterConstraintIndex for consideration here, since this is exactly + // the place where we want to see if we can replace it with tighter + // conjunctions. + for { + added := false + for i := index; i < len(rels) && rels[i].voterAndAllRel == conjStrictSuperset; i++ { + rel := rels[i] + remainingAll := allReplicaConstraints[rel.allIndex].remainingReplicas + neededVoterReplicas := conf.voterConstraints[rel.voterIndex].numReplicas - + voterConstraints[rel.voterIndex].numReplicas - + voterConstraints[rel.voterIndex].additionalReplicas + if neededVoterReplicas > 0 && remainingAll > 0 { + // Satisfy 1 replica. + voterConstraints[rel.voterIndex].additionalReplicas++ + allReplicaConstraints[rel.allIndex].remainingReplicas-- + newVoterIndex := allReplicaConstraints[rel.allIndex].newVoterIndex + if newVoterIndex == -1 { + // We haven't yet created a narrower voter constraint for this. + newVoterIndex = len(voterConstraints) + allReplicaConstraints[rel.allIndex].newVoterIndex = newVoterIndex + voterConstraints = append(voterConstraints, voterConstraintsAndAdditionalInfo{ + internedConstraintsConjunction: internedConstraintsConjunction{ + numReplicas: 0, + constraints: conf.constraints[rel.allIndex].constraints, + }, + additionalReplicas: 0, + }) + } + voterConstraints[newVoterIndex].numReplicas++ + added = true + } + } + if !added { + break + } + } + for i := range conf.voterConstraints { + neededReplicas := conf.voterConstraints[i].numReplicas + actualReplicas := voterConstraints[i].numReplicas + voterConstraints[i].additionalReplicas + if actualReplicas > neededReplicas { + panic("code bug") + } + if actualReplicas < neededReplicas { + err = errors.Errorf("could not satisfy all voter constraints due to " + + "non-intersecting conjunctions in voter and all replica constraints") + // Just force the satisfaction. + voterConstraints[i].numReplicas += neededReplicas - actualReplicas + } + } + n := len(voterConstraints) - 1 + if emptyVoterConstraintIndex >= 0 && emptyVoterConstraintIndex < n { + // Move it to the end, since it is the biggest set. + voterConstraints[emptyVoterConstraintIndex], voterConstraints[n] = + voterConstraints[n], voterConstraints[emptyVoterConstraintIndex] + } + var vc []internedConstraintsConjunction + for i := range voterConstraints { + if voterConstraints[i].numReplicas > 0 { + vc = append(vc, voterConstraints[i].internedConstraintsConjunction) + } + } + conf.voterConstraints = vc + return conf, err +} + type replicaKindIndex int32 const ( diff --git a/pkg/kv/kvserver/allocator/allocator2/constraint_test.go b/pkg/kv/kvserver/allocator/allocator2/constraint_test.go index b5192fa6e509..c97d32c7c626 100644 --- a/pkg/kv/kvserver/allocator/allocator2/constraint_test.go +++ b/pkg/kv/kvserver/allocator/allocator2/constraint_test.go @@ -10,10 +10,133 @@ package allocator2 +import ( + "fmt" + "strconv" + "strings" + "testing" + + "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/datadriven" + "github.com/stretchr/testify/require" +) + // TODO: tests for // - storeIDPostingList // - localityTierInterner // - localityTiers.diversityScore -// - makeNormalizedSpanConfig // - rangeAnalyzedConstraints initialization: pool and release; stateForInit, finishInit // - rangeAnalyzedConstraints read-only functions. + +func TestNormalizedSpanConfig(t *testing.T) { + interner := newStringInterner() + datadriven.RunTest(t, "testdata/normalize_config", + func(t *testing.T, d *datadriven.TestData) string { + parseConstraints := func(fields []string) []roachpb.Constraint { + var cc []roachpb.Constraint + for _, field := range fields { + var typ roachpb.Constraint_Type + switch field[0] { + case '+': + typ = roachpb.Constraint_REQUIRED + case '-': + typ = roachpb.Constraint_PROHIBITED + default: + t.Fatalf(fmt.Sprintf("unexpected start of field %s", field)) + } + kv := strings.Split(field[1:], "=") + if len(kv) != 2 { + t.Fatalf("unexpected field %s", field) + } + cc = append(cc, roachpb.Constraint{ + Type: typ, + Key: kv[0], + Value: kv[1], + }) + } + return cc + } + parseConstraintsConj := func(fields []string) roachpb.ConstraintsConjunction { + var cc roachpb.ConstraintsConjunction + if strings.HasPrefix(fields[0], "num-replicas=") { + val := strings.TrimPrefix(fields[0], "num-replicas=") + replicas, err := strconv.Atoi(val) + require.NoError(t, err) + cc.NumReplicas = int32(replicas) + fields = fields[1:] + } + cc.Constraints = parseConstraints(fields) + return cc + } + printSpanConf := func(b *strings.Builder, conf roachpb.SpanConfig) { + fmt.Fprintf(b, " num-replicas=%d num-voters=%d\n", conf.NumReplicas, conf.NumVoters) + if len(conf.Constraints) > 0 { + fmt.Fprintf(b, " constraints:\n") + for _, cc := range conf.Constraints { + fmt.Fprintf(b, " %s\n", cc.String()) + } + } + if len(conf.VoterConstraints) > 0 { + fmt.Fprintf(b, " voter-constraints:\n") + for _, cc := range conf.VoterConstraints { + fmt.Fprintf(b, " %s\n", cc.String()) + } + } + if len(conf.LeasePreferences) > 0 { + fmt.Fprintf(b, " lease-preferences:\n") + for _, lp := range conf.LeasePreferences { + fmt.Fprintf(b, " ") + for i, cons := range lp.Constraints { + if i > 0 { + b.WriteRune(',') + } + b.WriteString(cons.String()) + } + fmt.Fprintf(b, "\n") + } + } + } + switch d.Cmd { + case "normalize": + var numReplicas, numVoters int + var conf roachpb.SpanConfig + d.ScanArgs(t, "num-replicas", &numReplicas) + conf.NumReplicas = int32(numReplicas) + d.ScanArgs(t, "num-voters", &numVoters) + conf.NumVoters = int32(numVoters) + for _, line := range strings.Split(d.Input, "\n") { + parts := strings.Fields(line) + switch parts[0] { + case "constraint": + cc := parseConstraintsConj(parts[1:]) + conf.Constraints = append(conf.Constraints, cc) + case "voter-constraint": + cc := parseConstraintsConj(parts[1:]) + conf.VoterConstraints = append(conf.VoterConstraints, cc) + case "lease-preference": + cc := parseConstraints(parts[1:]) + conf.LeasePreferences = append(conf.LeasePreferences, roachpb.LeasePreference{ + Constraints: cc, + }) + default: + return fmt.Sprintf("unknown field: %s", parts[0]) + } + } + var b strings.Builder + fmt.Fprintf(&b, "input:\n") + printSpanConf(&b, conf) + nConf, err := makeNormalizedSpanConfig(&conf, interner) + if err != nil { + fmt.Fprintf(&b, "err=%s\n", err.Error()) + } + if nConf != nil { + fmt.Fprintf(&b, "output:\n") + printSpanConf(&b, nConf.uninternedConfig()) + } + return b.String() + + default: + return fmt.Sprintf("unknown command: %s", d.Cmd) + } + }) +} diff --git a/pkg/kv/kvserver/allocator/allocator2/testdata/normalize_config b/pkg/kv/kvserver/allocator/allocator2/testdata/normalize_config new file mode 100644 index 000000000000..08f67431fdc1 --- /dev/null +++ b/pkg/kv/kvserver/allocator/allocator2/testdata/normalize_config @@ -0,0 +1,363 @@ +normalize num-replicas=3 num-voters=3 +constraint +region=us-west -zone=us-west-b +constraint +zone=us-west-a +lease-preference +region=us-west +rack=10 +---- +input: + num-replicas=3 num-voters=3 + constraints: + +region=us-west,-zone=us-west-b + +zone=us-west-a + lease-preferences: + +region=us-west,+rack=10 +output: + num-replicas=3 num-voters=3 + constraints: + +region=us-west,+zone=us-west-a,-zone=us-west-b:3 + lease-preferences: + +region=us-west,+rack=10 + +normalize num-replicas=3 num-voters=3 +voter-constraint +region=us-west -zone=us-west-b +voter-constraint +zone=us-west-a +---- +input: + num-replicas=3 num-voters=3 + voter-constraints: + +region=us-west,-zone=us-west-b + +zone=us-west-a +output: + num-replicas=3 num-voters=3 + voter-constraints: + +region=us-west,+zone=us-west-a,-zone=us-west-b:3 + +normalize num-replicas=5 num-voters=5 +constraint num-replicas=1 -zone=us-west-b +constraint +zone=us-west-a +lease-preference +region=us-west +rack=10 +---- +input: + num-replicas=5 num-voters=5 + constraints: + -zone=us-west-b:1 + +zone=us-west-a + lease-preferences: + +region=us-west,+rack=10 +err=invalid mix of constraints + +normalize num-replicas=5 num-voters=3 +constraint num-replicas=1 -zone=us-west-b -zone=us-west-b +rack=10 +constraint num-replicas=2 +zone=us-west-a +voter-constraint num-replicas=2 +zone=us-west-a +---- +input: + num-replicas=5 num-voters=3 + constraints: + -zone=us-west-b,-zone=us-west-b,+rack=10:1 + +zone=us-west-a:2 + voter-constraints: + +zone=us-west-a:2 +output: + num-replicas=5 num-voters=3 + constraints: + +rack=10,-zone=us-west-b:1 + +zone=us-west-a:2 + :2 + voter-constraints: + +zone=us-west-a:2 + :1 + +# Multi-region config with all voters in one region, and one voter pinned to a +# zone. +normalize num-replicas=5 num-voters=3 +constraint num-replicas=1 +region=b +constraint num-replicas=1 +region=c +voter-constraint num-replicas=1 +region=a +zone=a1 +voter-constraint num-replicas=1 +region=a -zone=a1 +---- +input: + num-replicas=5 num-voters=3 + constraints: + +region=b:1 + +region=c:1 + voter-constraints: + +region=a,+zone=a1:1 + +region=a,-zone=a1:1 +output: + num-replicas=5 num-voters=3 + constraints: + +region=b:1 + +region=c:1 + :3 + voter-constraints: + +region=a,+zone=a1:1 + +region=a,-zone=a1:1 + :1 + +# Similar to previous, but with another constraint that does not intersect with the +# voter constraints. +normalize num-replicas=5 num-voters=3 +constraint num-replicas=1 +region=b +constraint num-replicas=1 +region=c +constraint num-replicas=1 +region=d +voter-constraint num-replicas=1 +region=a +zone=a1 +voter-constraint num-replicas=1 +region=a -zone=a1 +---- +input: + num-replicas=5 num-voters=3 + constraints: + +region=b:1 + +region=c:1 + +region=d:1 + voter-constraints: + +region=a,+zone=a1:1 + +region=a,-zone=a1:1 +output: + num-replicas=5 num-voters=3 + constraints: + +region=b:1 + +region=c:1 + +region=d:1 + :2 + voter-constraints: + +region=a,+zone=a1:1 + +region=a,-zone=a1:1 + +region=b:1 + +# Simple multi-region config with voters in multiple regions. +normalize num-replicas=5 num-voters=5 +constraint num-replicas=2 +region=a +constraint num-replicas=2 +region=b +constraint num-replicas=1 +region=c +voter-constraint num-replicas=2 +region=a +voter-constraint num-replicas=2 +region=b +voter-constraint num-replicas=1 +region=c +lease-preference +region=a +lease-preference +region=b +lease-preference +region=c +---- +input: + num-replicas=5 num-voters=5 + constraints: + +region=a:2 + +region=b:2 + +region=c:1 + voter-constraints: + +region=a:2 + +region=b:2 + +region=c:1 + lease-preferences: + +region=a + +region=b + +region=c +output: + num-replicas=5 num-voters=5 + constraints: + +region=a:2 + +region=b:2 + +region=c:1 + voter-constraints: + +region=a:2 + +region=b:2 + +region=c:1 + lease-preferences: + +region=a + +region=b + +region=c + +# Multi-region config with under-specified voter-constraint. +normalize num-replicas=9 num-voters=9 +constraint num-replicas=1 +region=a +constraint num-replicas=1 +region=b +constraint num-replicas=1 +region=c +constraint num-replicas=1 +region=d +constraint num-replicas=1 +region=e +voter-constraint num-replicas=2 +region=f +lease-preference +region=f +---- +input: + num-replicas=9 num-voters=9 + constraints: + +region=a:1 + +region=b:1 + +region=c:1 + +region=d:1 + +region=e:1 + voter-constraints: + +region=f:2 + lease-preferences: + +region=f +output: + num-replicas=9 num-voters=9 + constraints: + +region=a:1 + +region=b:1 + +region=c:1 + +region=d:1 + +region=e:1 + :4 + voter-constraints: + +region=f:2 + +region=e:1 + +region=a:1 + +region=b:1 + +region=c:1 + +region=d:1 + :2 + lease-preferences: + +region=f + +# Multi-region config with under-specified voter-constraint. +normalize num-replicas=9 num-voters=7 +constraint num-replicas=1 +region=a +constraint num-replicas=1 +region=b +constraint num-replicas=1 +region=c +constraint num-replicas=1 +region=d +constraint num-replicas=1 +region=e +voter-constraint num-replicas=2 +region=f +lease-preference +region=f +---- +input: + num-replicas=9 num-voters=7 + constraints: + +region=a:1 + +region=b:1 + +region=c:1 + +region=d:1 + +region=e:1 + voter-constraints: + +region=f:2 + lease-preferences: + +region=f +output: + num-replicas=9 num-voters=7 + constraints: + +region=a:1 + +region=b:1 + +region=c:1 + +region=d:1 + +region=e:1 + :4 + voter-constraints: + +region=f:2 + +region=c:1 + +region=a:1 + +region=b:1 + :2 + lease-preferences: + +region=f + +normalize num-replicas=5 num-voters=3 +constraint num-replicas=1 +region=b +zone=b2 +constraint num-replicas=1 +region=c +zone=c2 +voter-constraint num-replicas=1 +region=b +zone=b1 +voter-constraint num-replicas=1 +region=a -zone=a1 +---- +input: + num-replicas=5 num-voters=3 + constraints: + +region=b,+zone=b2:1 + +region=c,+zone=c2:1 + voter-constraints: + +region=b,+zone=b1:1 + +region=a,-zone=a1:1 +err=intersecting conjunctions in constraints and voter constraints +output: + num-replicas=5 num-voters=3 + constraints: + +region=b,+zone=b2:1 + +region=c,+zone=c2:1 + :3 + voter-constraints: + +region=b,+zone=b1:1 + +region=a,-zone=a1:1 + :1 + +# Multi-region with under-specified voter-constraint. In this case the +# under-specification is not the unconstrained set, but the conjunction +# "+region=b", which can be further narrowed. +normalize num-replicas=3 num-voters=3 +constraint num-replicas=1 +region=b +zone=b2 +constraint num-replicas=1 +region=c +zone=c2 +constraint num-replicas=1 +region=d +zone=d2 +voter-constraint num-replicas=2 +region=b +voter-constraint num-replicas=1 +region=c +zone=c2 +---- +input: + num-replicas=3 num-voters=3 + constraints: + +region=b,+zone=b2:1 + +region=c,+zone=c2:1 + +region=d,+zone=d2:1 + voter-constraints: + +region=b:2 + +region=c,+zone=c2:1 +err=could not satisfy all voter constraints due to non-intersecting conjunctions in voter and all replica constraints +output: + num-replicas=3 num-voters=3 + constraints: + +region=b,+zone=b2:1 + +region=c,+zone=c2:1 + +region=d,+zone=d2:1 + voter-constraints: + +region=b:1 + +region=c,+zone=c2:1 + +region=b,+zone=b2:1 + +# Similar to previous, but under-specified in a way that does not generate an +# error. +normalize num-replicas=4 num-voters=3 +constraint num-replicas=1 +region=b +zone=b2 +constraint num-replicas=1 +region=c +zone=c2 +constraint num-replicas=1 +region=d +zone=d2 +voter-constraint num-replicas=1 +region=e +voter-constraint num-replicas=1 +region=c +voter-constraint num-replicas=1 +region=d +---- +input: + num-replicas=4 num-voters=3 + constraints: + +region=b,+zone=b2:1 + +region=c,+zone=c2:1 + +region=d,+zone=d2:1 + voter-constraints: + +region=e:1 + +region=c:1 + +region=d:1 +output: + num-replicas=4 num-voters=3 + constraints: + +region=b,+zone=b2:1 + +region=c,+zone=c2:1 + +region=d,+zone=d2:1 + :1 + voter-constraints: + +region=e:1 + +region=c,+zone=c2:1 + +region=d,+zone=d2:1 + +# Single-region very under-specified. +normalize num-replicas=9 num-voters=5 +constraint num-replicas=3 +region=a +zone=a1 +constraint num-replicas=3 +region=a +zone=a2 +constraint num-replicas=3 +region=a +zone=a3 +voter-constraint num-replicas=5 +region=a +---- +input: + num-replicas=9 num-voters=5 + constraints: + +region=a,+zone=a1:3 + +region=a,+zone=a2:3 + +region=a,+zone=a3:3 + voter-constraints: + +region=a:5 +output: + num-replicas=9 num-voters=5 + constraints: + +region=a,+zone=a1:3 + +region=a,+zone=a2:3 + +region=a,+zone=a3:3 + voter-constraints: + +region=a,+zone=a1:2 + +region=a,+zone=a2:2 + +region=a,+zone=a3:1 diff --git a/pkg/roachpb/span_config.proto b/pkg/roachpb/span_config.proto index 3b0baa92d020..d3925f4db7bd 100644 --- a/pkg/roachpb/span_config.proto +++ b/pkg/roachpb/span_config.proto @@ -165,6 +165,57 @@ message SpanConfig { // on. This must be compatible with the Constraints field above, but not // necessarily a subset. It's compatible as long as there are no prohibitive // constraints above that are required here. + // + // As of v23.2, if both VoterConstraints and Constraints are specified, we + // require that VoterConstraints is stricter than Constraints. Strictness is + // defined as: if we satisfy VoterConstraints (by satisfying NumVoters and + // ConstraintsConjunction.NumReplicas for all ConstraintsConjunctions in + // VoterConstraints), then: + // + // - Any replica chosen to be a voter will satisfy some + // ConstraintsConjunction in Constraints. + // + // - If no ConstraintsConjunction in VoterConstraints is over-satisfied + // (exceeds the implicit or explicit NumReplicas), then voters by + // themselves will not over-satisfy any ConstraintsConjunction in + // Constraints. + // + // For existing clusters, these strictness requirements may not be met, so + // internally the system tries to do a normalization to meet the strictness + // requirement. + // + // Here is an example that does not satisfy strictness: + // num-replicas=9 num-voters=9 + // constraints: + // +region=a:1 + // +region=b:1 + // +region=c:1 + // +region=d:1 + // +region=e:1 + // voter-constraints: + // +region=f:2 + // + // This is because 7 replicas are unconstrained according to + // VoterConstraints, and 4 replicas are unconstrained according to + // Constraints. So (the difference) 3 replicas can satisfy VoterConstraints + // without any restriction and can over-satisfy some constraint in + // Constraints. + // + // The stricter version of this config has the following VoterConstraints: + // +region=a:1 + // +region=b:1 + // +region=c:1 + // +region=d:1 + // +region=e:1 + // +region=f:2 + // + // TODO(kvoli,sumeer): document how we will inform the operator when this + // normalization fails. + // + // It is also highly recommended that when there are multiple + // ConstraintsConjunctions in Constraints (or VoterConstraints), they are + // specified in a manner that the same store cannot satisfy multiple + // conjunctions. repeated ConstraintsConjunction voter_constraints = 8 [(gogoproto.nullable) = false]; // LeasePreference captures the preference for how range leases are to be diff --git a/pkg/sql/opt/props/ordering_choice.go b/pkg/sql/opt/props/ordering_choice.go index f4495a8c001f..83a676aa127f 100644 --- a/pkg/sql/opt/props/ordering_choice.go +++ b/pkg/sql/opt/props/ordering_choice.go @@ -657,12 +657,15 @@ func (oc *OrderingChoice) AppendCol(id opt.ColumnID, descending bool) { } // Copy returns a complete copy of this instance, with a private version of the -// ordering column array. +// ordering column slice. func (oc *OrderingChoice) Copy() OrderingChoice { var other OrderingChoice other.Optional = oc.Optional.Copy() other.Columns = make([]OrderingColumnChoice, len(oc.Columns)) copy(other.Columns, oc.Columns) + for i := range other.Columns { + other.Columns[i].Group = other.Columns[i].Group.Copy() + } return other } diff --git a/pkg/sql/opt/xform/optimizer.go b/pkg/sql/opt/xform/optimizer.go index 2a26cd4be31e..0cc4ea49db9f 100644 --- a/pkg/sql/opt/xform/optimizer.go +++ b/pkg/sql/opt/xform/optimizer.go @@ -703,7 +703,7 @@ func (o *Optimizer) enforceProps( } // optimizeEnforcer optimizes and costs the enforcer. getEnforcer is used to -// reset the enforcer after recusing in optimizeGroup, since the current group +// reset the enforcer after recursing in optimizeGroup, since the current group // and its children may use the same SortExpr to avoid allocations. func (o *Optimizer) optimizeEnforcer( state *groupState,