mmaprototype: add more tests for err in TestNormalizedVoterAllRelatio…

pkg/kv/kvserver/allocator/mmaprototype/constraint.go

@@ -209,59 +209,102 @@ 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
 )
 
+// 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 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.
 			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
 }
 
@@ -581,13 +624,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

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,7 +129,83 @@ table:
 	relationships:
 	[idx=0][voter=+region=us-east:3] [all=+region=us-west:5] rel=nonIntersecting
 
-# Test error case: multiple empty voter constraints.
+# 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
 voter-constraint num-replicas=1
 ----
@@ -159,7 +236,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 +262,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

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: