From 1dbcaa7daf3418e8d6692a16bc1acdac536515a4 Mon Sep 17 00:00:00 2001 From: wenyihu6 Date: Tue, 2 Dec 2025 22:09:03 -0500 Subject: [PATCH 1/5] mmaprototype: add more tests for err in TestNormalizedVoterAllRelationships --- .../TestNormalizedVoterAllRelationships | 57 ++++++++++++++++++- 1 file changed, 55 insertions(+), 2 deletions(-) diff --git a/pkg/kv/kvserver/allocator/mmaprototype/testdata/TestNormalizedVoterAllRelationships b/pkg/kv/kvserver/allocator/mmaprototype/testdata/TestNormalizedVoterAllRelationships index ed5d31e99544..d5f7d373b978 100644 --- a/pkg/kv/kvserver/allocator/mmaprototype/testdata/TestNormalizedVoterAllRelationships +++ b/pkg/kv/kvserver/allocator/mmaprototype/testdata/TestNormalizedVoterAllRelationships @@ -128,7 +128,8 @@ table: relationships: [idx=0][voter=+region=us-east:3] [all=+region=us-west:5] rel=nonIntersecting -# Test error case: multiple empty voter constraints. +# Test error case: multiple empty voter constraints (via normalization adding +# empty constraint). normalized-voter-all-rels num-replicas=5 num-voters=3 voter-constraint num-replicas=1 ---- @@ -159,7 +160,8 @@ input: +region=us-west:2 normalization error: constraint replicas add up to more than configured replicas -# Test error case: multiple empty constraints. +# Test error case: multiple empty constraints (via normalization adding empty +# constraint). normalized-voter-all-rels num-replicas=8 num-voters=5 constraint num-replicas=2 voter-constraint num-replicas=1 +region=us-west @@ -184,6 +186,57 @@ table: relationships: err: multiple empty constraints: {2 []} and {6 []} +# Test error case: multiple explicit empty constraints. +normalized-voter-all-rels num-replicas=5 num-voters=3 +constraint num-replicas=2 +constraint num-replicas=3 +voter-constraint num-replicas=1 +region=us-west +---- +input: + num-replicas=5 num-voters=3 + constraints: + :2 + :3 + voter-constraints: + +region=us-west:1 +normalized: + num-replicas=5 num-voters=3 + constraints: + :2 + :3 + voter-constraints: + +region=us-west:1 + :2 +table: + emptyConstraintIndex: -1 + emptyVoterConstraintIndex: -1 + relationships: + err: multiple empty constraints: {2 []} and {3 []} + +# Test error case: voter constraint replicas add up to more than configured voters. +normalized-voter-all-rels num-replicas=5 num-voters=1 +voter-constraint num-replicas=2 +region=us-west +---- +input: + num-replicas=5 num-voters=1 + voter-constraints: + +region=us-west:2 +normalization error: constraint replicas add up to more than configured replicas + +# Test error case: invalid mix of constraints (NumReplicas=0 and NumReplicas>0 together). +# A constraint without num-replicas= defaults to 0 (applies to all), mixed with one that +# specifies num-replicas>0 is invalid. +normalized-voter-all-rels num-replicas=5 num-voters=3 +constraint +region=us-west +constraint num-replicas=2 +region=us-east +---- +input: + num-replicas=5 num-voters=3 + constraints: + +region=us-west + +region=us-east:2 +normalization error: invalid mix of constraints + # Test with multiple relationships. normalized-voter-all-rels num-replicas=8 num-voters=5 constraint num-replicas=2 +region=us-west From bc43132799b44c8aa2491e81b0b7e5c34281f5dc Mon Sep 17 00:00:00 2001 From: wenyihu6 Date: Tue, 2 Dec 2025 20:17:12 -0500 Subject: [PATCH 2/5] mmaprototype: improve comment on relationship --- .../allocator/mmaprototype/constraint.go | 40 ++++++++++++++++--- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/pkg/kv/kvserver/allocator/mmaprototype/constraint.go b/pkg/kv/kvserver/allocator/mmaprototype/constraint.go index 2a312e1dddfb..508182f247bf 100644 --- a/pkg/kv/kvserver/allocator/mmaprototype/constraint.go +++ b/pkg/kv/kvserver/allocator/mmaprototype/constraint.go @@ -212,56 +212,86 @@ const ( conjNonIntersecting ) +// relationship returns the logical relationship between two sets of constraints +// (represented as sorted, de-duplicated conjunctions). func (cc constraintsConj) relationship(b constraintsConj) conjunctionRelationship { + // n is the number of conjuncts in cc. n := len(cc) + // m is the number of conjuncts in b. m := len(b) + // extraInCC is the number of conjuncts present in cc but not in b. extraInCC := 0 + // extraInB is the number of conjuncts present in b but not in cc. extraInB := 0 + // inBoth is the number of conjuncts that are present in both. inBoth := 0 + + // Example: cc = [A, C, E] and b = [B, D, E]: + // + // | Step | i | j | cc[i] | b[j] | Comparison | Action | extraInCC | extraInB | inBoth | + // |------|---|---|-------|------|------------|-----------------------------|-----------|----------|--------| + // | 1 | 0 | 0 | A | B | A < B | extraInCC++, i++ | 1 | 0 | 0 | + // | 2 | 1 | 0 | C | B | C > B | extraInB++, j++ | 1 | 1 | 0 | + // | 3 | 1 | 1 | C | D | C < D | extraInCC++, i++ | 2 | 1 | 0 | + // | 4 | 2 | 1 | E | D | E > D | extraInB++, j++ | 2 | 2 | 0 | + // | 5 | 2 | 2 | E | E | E == E | inBoth++, i++, j++ | 2 | 2 | 1 | + // + // i and j are indices into cc and b respectively for i, j := 0, 0; i < n || j < m; { + // If we've reached the end of cc, remaining items are only in b. if i >= n { extraInB++ j++ continue } + // If we've reached the end of b, remaining items are only in cc. if j >= m { extraInCC++ i++ continue } + // If both conjuncts are identical, increment inBoth. if cc[i] == b[j] { inBoth++ i++ j++ continue } + + // If cc[i] < b[j], we've found a conjunct unique to cc. if cc[i].less(b[j]) { - // Found a conjunct that is not in b. extraInCC++ i++ continue } else { + // Otherwise, found a conjunct unique to b. extraInB++ j++ continue } } + + // There are four possibilities: + // 1. extraInCC > 0 and extraInB == 0: cc is a strict subset of b. if extraInCC > 0 && extraInB == 0 { return conjStrictSubset } + // 2. extraInB > 0 and extraInCC == 0: cc is a strict superset of b. if extraInB > 0 && extraInCC == 0 { return conjStrictSuperset } - // (extraInCC == 0 || extraInB > 0) && (extraInB == 0 || extraInCC > 0) - // => - // (extraInCC == 0 && extraInB == 0) || (extraInB > 0 && extraInCC > 0) + + // 3. extraInCC == 0 and extraInB == 0: sets are equal. if extraInCC == 0 && extraInB == 0 { return conjEqualSet } - // (extraInB > 0 && extraInCC > 0) + + // 4. extraInCC > 0 and extraInB > 0: + // a) if inBoth > 0, sets may possibly intersect. if inBoth > 0 { return conjPossiblyIntersecting } + // b) if inBoth == 0, sets are disjoint. return conjNonIntersecting } From f2c1b5c1a715be7070655aa39c6ee4a53042325b Mon Sep 17 00:00:00 2001 From: sumeerbhola Date: Wed, 26 Nov 2025 10:42:00 -0500 Subject: [PATCH 3/5] mma: additional case for non-intersecting conjunctions A=[+region=a, +zone=a1], B=[+region=a, +zone=a2] is now classified as non-intersecting. This fixes an existing todo. --- .../allocator/mmaprototype/constraint.go | 19 ++++++++++++------- .../mmaprototype/testdata/normalize_config | 1 - 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/kv/kvserver/allocator/mmaprototype/constraint.go b/pkg/kv/kvserver/allocator/mmaprototype/constraint.go index 508182f247bf..c7a0e0459e87 100644 --- a/pkg/kv/kvserver/allocator/mmaprototype/constraint.go +++ b/pkg/kv/kvserver/allocator/mmaprototype/constraint.go @@ -209,6 +209,8 @@ const ( // Example: A=[+region=a], B=[+zone=a1] // If zone=a1 happens to be in region=a, then the disjoint result is // not correct. + // Example: A=[+region=a, +zone=a1], B=[+region=a, +zone=a2] + // Since a store cannot be in both zones, the sets are disjoint. conjNonIntersecting ) @@ -260,6 +262,15 @@ func (cc constraintsConj) relationship(b constraintsConj) conjunctionRelationshi // If cc[i] < b[j], we've found a conjunct unique to cc. if cc[i].less(b[j]) { + // Found a conjunct that is not in b. + if cc[i].typ == b[j].typ && cc[i].key == b[j].key { + // For example, +zone=a1, +zone=a2. + return conjNonIntersecting + // NB: +zone=a1 and -zone=a1 are also non-intersecting, but we will + // not detect this case. Finding this case requires searching through + // b, and not simply walking in order, since the typ field is the + // first in the sort order and differs between these two conjuncts. + } extraInCC++ i++ continue @@ -611,13 +622,7 @@ func doStructuralNormalization(conf *normalizedSpanConfig) error { // non-intersecting. When the conjunctions are intersecting, we cannot // promote from one to the other to fill out the set of conjunctions. // - // Example 1: +region=a,+zone=a1 and +region=a,+zone=a2 are classified as - // conjPossiblyIntersecting, but we could do better in knowing that the - // conjunctions are non-intersecting since zone=a1 and zone=a2 are disjoint. - // - // TODO(sumeer): improve the case of example 1. - // - // Example 2: +region=a,+zone=a1 and +region=a,-zone=a2 are classified as + // Example: +region=a,+zone=a1 and +region=a,-zone=a2 are classified as // conjPossiblyIntersecting. And if there happens to be a zone=a3 in the // region, they are actually intersecting. We cannot do better since we // don't know the semantics of regions, zones or the universe of possible diff --git a/pkg/kv/kvserver/allocator/mmaprototype/testdata/normalize_config b/pkg/kv/kvserver/allocator/mmaprototype/testdata/normalize_config index 7431761915f6..4c3602ae84a5 100644 --- a/pkg/kv/kvserver/allocator/mmaprototype/testdata/normalize_config +++ b/pkg/kv/kvserver/allocator/mmaprototype/testdata/normalize_config @@ -314,7 +314,6 @@ input: 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: From 3b65f238ec4863fb8d01fc0b0dd19d446515d47b Mon Sep 17 00:00:00 2001 From: wenyihu6 Date: Tue, 2 Dec 2025 22:22:59 -0500 Subject: [PATCH 4/5] mmaprototype: fix conjNonIntersecting Previously, we started returning conjNonIntersecting when we encounter same type, same key, different values. However, the fix only returned early if the differing value is encountered from cc. This commit changed it so that we also check and return early for b. --- .../allocator/mmaprototype/constraint.go | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/pkg/kv/kvserver/allocator/mmaprototype/constraint.go b/pkg/kv/kvserver/allocator/mmaprototype/constraint.go index c7a0e0459e87..be48a2977af0 100644 --- a/pkg/kv/kvserver/allocator/mmaprototype/constraint.go +++ b/pkg/kv/kvserver/allocator/mmaprototype/constraint.go @@ -259,18 +259,20 @@ func (cc constraintsConj) relationship(b constraintsConj) conjunctionRelationshi j++ continue } - + // If the type and key are the same but value differs, + // then these constraints are non-intersecting. + // Example: +zone=a1, +zone=a2 (disjoint) + // + if cc[i].typ == b[j].typ && cc[i].key == b[j].key { + // For example, +zone=a1, +zone=a2. + return conjNonIntersecting + // NB: +zone=a1 and -zone=a1 are also non-intersecting, but we will + // not detect this case. Finding this case requires searching through + // b, and not simply walking in order, since the typ field is the + // first in the sort order and differs between these two conjuncts. + } // If cc[i] < b[j], we've found a conjunct unique to cc. if cc[i].less(b[j]) { - // Found a conjunct that is not in b. - if cc[i].typ == b[j].typ && cc[i].key == b[j].key { - // For example, +zone=a1, +zone=a2. - return conjNonIntersecting - // NB: +zone=a1 and -zone=a1 are also non-intersecting, but we will - // not detect this case. Finding this case requires searching through - // b, and not simply walking in order, since the typ field is the - // first in the sort order and differs between these two conjuncts. - } extraInCC++ i++ continue From 277506e7d56fa4f347c46395a9580078a405adb2 Mon Sep 17 00:00:00 2001 From: wenyihu6 Date: Tue, 2 Dec 2025 22:28:44 -0500 Subject: [PATCH 5/5] mmaprototype: add test cases for conjNonIntersecting --- .../TestNormalizedVoterAllRelationships | 80 ++++++++++++++++++- 1 file changed, 78 insertions(+), 2 deletions(-) diff --git a/pkg/kv/kvserver/allocator/mmaprototype/testdata/TestNormalizedVoterAllRelationships b/pkg/kv/kvserver/allocator/mmaprototype/testdata/TestNormalizedVoterAllRelationships index d5f7d373b978..31f1428c1008 100644 --- a/pkg/kv/kvserver/allocator/mmaprototype/testdata/TestNormalizedVoterAllRelationships +++ b/pkg/kv/kvserver/allocator/mmaprototype/testdata/TestNormalizedVoterAllRelationships @@ -13,7 +13,8 @@ table: emptyVoterConstraintIndex: -1 relationships: -# Test with intersecting voter and all constraints. +# Test non-intersecting: same region but different zones (same type, same key, +# different values). normalized-voter-all-rels num-replicas=3 num-voters=3 constraint +zone=us-west-a +region=us-west voter-constraint +zone=us-west-b +region=us-west @@ -34,7 +35,7 @@ table: emptyConstraintIndex: -1 emptyVoterConstraintIndex: -1 relationships: - [idx=0][voter=+zone=us-west-b,+region=us-west:3] [all=+zone=us-west-a,+region=us-west:3] rel=possiblyIntersecting + [idx=0][voter=+zone=us-west-b,+region=us-west:3] [all=+zone=us-west-a,+region=us-west:3] rel=nonIntersecting # Test with equal voter and all constraints. normalized-voter-all-rels num-replicas=3 num-voters=3 @@ -128,6 +129,81 @@ table: relationships: [idx=0][voter=+region=us-east:3] [all=+region=us-west:5] rel=nonIntersecting +# Test with non-intersecting constraints (same type, different key, different +# values). +normalized-voter-all-rels num-replicas=5 num-voters=3 +constraint +region=us-west +voter-constraint +dc=dc-1 +---- +input: + num-replicas=5 num-voters=3 + constraints: + +region=us-west + voter-constraints: + +dc=dc-1 +normalized: + num-replicas=5 num-voters=3 + constraints: + +region=us-west:5 + voter-constraints: + +dc=dc-1:3 +table: + emptyConstraintIndex: -1 + emptyVoterConstraintIndex: -1 + relationships: + [idx=0][voter=+dc=dc-1:3] [all=+region=us-west:5] rel=nonIntersecting + +# Test non-intersecting: same type, same key, different values. During the +# sorted walk, all-replica constraint has smaller zone value (us-west-a < +# us-west-b), so the differing value is encountered from all-replica constraints +# first. +normalized-voter-all-rels num-replicas=5 num-voters=3 +constraint +region=us-west +zone=us-west-a +voter-constraint +region=us-west +zone=us-west-b +---- +input: + num-replicas=5 num-voters=3 + constraints: + +region=us-west,+zone=us-west-a + voter-constraints: + +region=us-west,+zone=us-west-b +normalized: + num-replicas=5 num-voters=3 + constraints: + +zone=us-west-a,+region=us-west:5 + voter-constraints: + +zone=us-west-b,+region=us-west:3 +table: + emptyConstraintIndex: -1 + emptyVoterConstraintIndex: -1 + relationships: + [idx=0][voter=+zone=us-west-b,+region=us-west:3] [all=+zone=us-west-a,+region=us-west:5] rel=nonIntersecting + +# Test non-intersecting: same type, same key, different values. During the +# sorted walk, voter constraint has smaller zone value (us-west-a < us-west-b), +# so the differing value is encountered from voter constraints first. +normalized-voter-all-rels num-replicas=5 num-voters=3 +constraint +region=us-west +zone=us-west-b +voter-constraint +region=us-west +zone=us-west-a +---- +input: + num-replicas=5 num-voters=3 + constraints: + +region=us-west,+zone=us-west-b + voter-constraints: + +region=us-west,+zone=us-west-a +normalized: + num-replicas=5 num-voters=3 + constraints: + +zone=us-west-b,+region=us-west:5 + voter-constraints: + +zone=us-west-a,+region=us-west:3 +table: + emptyConstraintIndex: -1 + emptyVoterConstraintIndex: -1 + relationships: + [idx=0][voter=+zone=us-west-a,+region=us-west:3] [all=+zone=us-west-b,+region=us-west:5] rel=nonIntersecting + # Test error case: multiple empty voter constraints (via normalization adding # empty constraint). normalized-voter-all-rels num-replicas=5 num-voters=3