Skip to content

Commit e189ba7

Browse files
sumeerbholawenyihu6
authored andcommitted
mmaprototype: fix bug in sortTargetCandidateSetAndPick
The intention of the code was to be structured around sets representing equivalence classes, such that a later set is only considered if none of the earlier sets had a member that was discarded and had pending changes. Prior to this change, this criteria was arbitrarily applied in the middle of a set. So if two stores {s1, s2} were in the same set and s1 was discarded and had pending changes we would not consider s2. Now we will include s2 and stop when the next set starts. Epic: none Release note: none
1 parent 764f9a2 commit e189ba7

File tree

1 file changed

+17
-13
lines changed

1 file changed

+17
-13
lines changed

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

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1116,19 +1116,23 @@ func sortTargetCandidateSetAndPick(
11161116
// only reason we will consider a set later than the first one is if the
11171117
// earlier sets get fully discarded solely because of nls and have no
11181118
// pending changes, or because of ignoreHigherThanLoadThreshold.
1119-
lowestLoad := cands.candidates[0].sls
1119+
lowestLoadSet := cands.candidates[0].sls
1120+
currentLoadSet := lowestLoadSet
11201121
discardedCandsHadNoPendingChanges := true
11211122
for _, cand := range cands.candidates {
1122-
if cand.sls > lowestLoad {
1123+
if cand.sls > currentLoadSet {
11231124
if !discardedCandsHadNoPendingChanges {
1124-
// Never go beyond lowestLoad if we have discarded candidates that
1125-
// have pending changes. We will wait for those to have no pending
1126-
// changes before we consider candidates with load > lowestLoad.
1125+
// Never go to the next set if we have discarded candidates that have
1126+
// pending changes. We will wait for those to have no pending changes
1127+
// before we consider later sets.
11271128
break
11281129
}
1130+
currentLoadSet = cand.sls
1131+
}
1132+
if cand.sls > lowestLoadSet {
11291133
if j == 0 {
11301134
// This is the lowestLoad set being considered now.
1131-
lowestLoad = cand.sls
1135+
lowestLoadSet = cand.sls
11321136
} else if ignoreLevel < ignoreHigherThanLoadThreshold || overloadedDim == NumLoadDimensions {
11331137
// Past the lowestLoad set. We don't care about these.
11341138
break
@@ -1161,8 +1165,8 @@ func sortTargetCandidateSetAndPick(
11611165
log.VInfof(ctx, 2, "sortTargetCandidateSetAndPick: no candidates due to load")
11621166
return 0
11631167
}
1164-
lowestLoad = cands.candidates[0].sls
1165-
highestLoad := cands.candidates[j-1].sls
1168+
lowestLoadSet = cands.candidates[0].sls
1169+
highestLoadSet := cands.candidates[j-1].sls
11661170
cands.candidates = cands.candidates[:j]
11671171
// The set of candidates we will consider all have load <= loadThreshold.
11681172
// They may all be lowestLoad, or we may have allowed additional candidates
@@ -1184,11 +1188,11 @@ func sortTargetCandidateSetAndPick(
11841188
// conservative choice, since pending added work is slightly inflated in
11851189
// size, and we want to have a true picture of all of these potential
11861190
// candidates before we start using the ones with load >= loadNoChange.
1187-
if lowestLoad > loadThreshold {
1191+
if lowestLoadSet > loadThreshold {
11881192
panic("candidates should not have lowestLoad > loadThreshold")
11891193
}
11901194
// INVARIANT: lowestLoad <= loadThreshold.
1191-
if lowestLoad == loadThreshold && ignoreLevel < ignoreHigherThanLoadThreshold {
1195+
if lowestLoadSet == loadThreshold && ignoreLevel < ignoreHigherThanLoadThreshold {
11921196
log.VInfof(ctx, 2, "sortTargetCandidateSetAndPick: no candidates due to equal to loadThreshold")
11931197
return 0
11941198
}
@@ -1197,12 +1201,12 @@ func sortTargetCandidateSetAndPick(
11971201

11981202
// < loadNoChange is fine. We need to check whether the following cases can continue.
11991203
// [loadNoChange, loadThreshold), or loadThreshold && ignoreHigherThanLoadThreshold.
1200-
if lowestLoad >= loadNoChange &&
1204+
if lowestLoadSet >= loadNoChange &&
12011205
(!discardedCandsHadNoPendingChanges || ignoreLevel == ignoreLoadNoChangeAndHigher) {
12021206
log.VInfof(ctx, 2, "sortTargetCandidateSetAndPick: no candidates due to loadNoChange")
12031207
return 0
12041208
}
1205-
if lowestLoad != highestLoad {
1209+
if lowestLoadSet != highestLoadSet {
12061210
slices.SortFunc(cands.candidates, func(a, b candidateInfo) int {
12071211
return cmp.Or(
12081212
cmp.Compare(a.leasePreferenceIndex, b.leasePreferenceIndex),
@@ -1223,7 +1227,7 @@ func sortTargetCandidateSetAndPick(
12231227
return 0
12241228
}
12251229
cands.candidates = cands.candidates[:j]
1226-
if lowestLoad != highestLoad || (lowestLoad >= loadNoChange && overloadedDim != NumLoadDimensions) {
1230+
if lowestLoadSet != highestLoadSet || (lowestLoadSet >= loadNoChange && overloadedDim != NumLoadDimensions) {
12271231
// Sort candidates from lowest to highest along overloaded dimension. We
12281232
// limit when we do this, since this will further restrict the pool of
12291233
// candidates and in general we don't want to restrict the pool.

0 commit comments

Comments
 (0)