From 503101668e771ce5821477ba99eea2f233ee733e Mon Sep 17 00:00:00 2001 From: wenyihu6 Date: Wed, 19 Nov 2025 23:47:45 -0500 Subject: [PATCH] mmaprototype: fix doneFunc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, we were incorrectly including both voterIndex and nonVoterIndex in satisfiedByReplica for voter constraints's doneFunc, which didn’t seem right. I was initially surprised that nonVoterIndex showed up at all for satisfiedByReplica of voter constraints. It might be accounting for cases where a promotion or demotion might be needed. So satisfiedByReplica[nonVoterIndex] for a voter constraint may legitimately be non-zero. TODO: Putting this up for visibility and discussion — I still need to understand the code here better to see whether this fix is right; something feels off. More tests are needed as well. --- pkg/kv/kvserver/allocator/mmaprototype/constraint.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/kv/kvserver/allocator/mmaprototype/constraint.go b/pkg/kv/kvserver/allocator/mmaprototype/constraint.go index e879ff4834c0..33c93ef3d86a 100644 --- a/pkg/kv/kvserver/allocator/mmaprototype/constraint.go +++ b/pkg/kv/kvserver/allocator/mmaprototype/constraint.go @@ -840,7 +840,7 @@ func (rac *rangeAnalyzedConstraints) finishInit( rac.numNeededReplicas[nonVoterIndex] = spanConfig.numReplicas - spanConfig.numVoters rac.replicas = rac.buf.replicas - analyzeFunc := func(ac *analyzedConstraints) { + analyzeFunc := func(ac *analyzedConstraints, isVoterConstraints bool) { if len(ac.constraints) == 0 { // Nothing to do. return @@ -880,6 +880,11 @@ func (rac *rangeAnalyzedConstraints) finishInit( // oversatisfying. for j := range ac.constraints { doneFunc := func() bool { + if isVoterConstraints { + // For voter constraints, only voters can satisfy them. + return len(ac.satisfiedByReplica[voterIndex][j]) >= int(ac.constraints[j].numReplicas) + } + // For all-replica constraints, both voters and non-voters count. return len(ac.satisfiedByReplica[voterIndex][j])+ len(ac.satisfiedByReplica[nonVoterIndex][j]) >= int(ac.constraints[j].numReplicas) } @@ -930,11 +935,11 @@ func (rac *rangeAnalyzedConstraints) finishInit( } if spanConfig.constraints != nil { rac.constraints.constraints = spanConfig.constraints - analyzeFunc(&rac.constraints) + analyzeFunc(&rac.constraints, false /* isVoterConstraints */) } if spanConfig.voterConstraints != nil { rac.voterConstraints.constraints = spanConfig.voterConstraints - analyzeFunc(&rac.voterConstraints) + analyzeFunc(&rac.voterConstraints, true /* isVoterConstraints */) } rac.leaseholderID = leaseholder