Skip to content

Commit 5031016

Browse files
committed
mmaprototype: fix doneFunc
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.
1 parent ef4e9e4 commit 5031016

File tree

1 file changed

+8
-3
lines changed

1 file changed

+8
-3
lines changed

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -840,7 +840,7 @@ func (rac *rangeAnalyzedConstraints) finishInit(
840840
rac.numNeededReplicas[nonVoterIndex] = spanConfig.numReplicas - spanConfig.numVoters
841841
rac.replicas = rac.buf.replicas
842842

843-
analyzeFunc := func(ac *analyzedConstraints) {
843+
analyzeFunc := func(ac *analyzedConstraints, isVoterConstraints bool) {
844844
if len(ac.constraints) == 0 {
845845
// Nothing to do.
846846
return
@@ -880,6 +880,11 @@ func (rac *rangeAnalyzedConstraints) finishInit(
880880
// oversatisfying.
881881
for j := range ac.constraints {
882882
doneFunc := func() bool {
883+
if isVoterConstraints {
884+
// For voter constraints, only voters can satisfy them.
885+
return len(ac.satisfiedByReplica[voterIndex][j]) >= int(ac.constraints[j].numReplicas)
886+
}
887+
// For all-replica constraints, both voters and non-voters count.
883888
return len(ac.satisfiedByReplica[voterIndex][j])+
884889
len(ac.satisfiedByReplica[nonVoterIndex][j]) >= int(ac.constraints[j].numReplicas)
885890
}
@@ -930,11 +935,11 @@ func (rac *rangeAnalyzedConstraints) finishInit(
930935
}
931936
if spanConfig.constraints != nil {
932937
rac.constraints.constraints = spanConfig.constraints
933-
analyzeFunc(&rac.constraints)
938+
analyzeFunc(&rac.constraints, false /* isVoterConstraints */)
934939
}
935940
if spanConfig.voterConstraints != nil {
936941
rac.voterConstraints.constraints = spanConfig.voterConstraints
937-
analyzeFunc(&rac.voterConstraints)
942+
analyzeFunc(&rac.voterConstraints, true /* isVoterConstraints */)
938943
}
939944

940945
rac.leaseholderID = leaseholder

0 commit comments

Comments
 (0)