From ec1f2a2e218fa896b0796dd406962936f92450ec Mon Sep 17 00:00:00 2001 From: Lidor Carmel Date: Mon, 15 Aug 2022 10:59:31 -0700 Subject: [PATCH] allocator: rename candidate selection function In preparation for adding a new selection function for a good enough candidate, rename the existing "good" to "best". Release note: None --- .../allocator/allocatorimpl/allocator.go | 2 +- .../allocatorimpl/allocator_scorer.go | 8 +- .../allocatorimpl/allocator_scorer_test.go | 120 +++++++++--------- 3 files changed, 65 insertions(+), 65 deletions(-) diff --git a/pkg/kv/kvserver/allocator/allocatorimpl/allocator.go b/pkg/kv/kvserver/allocator/allocatorimpl/allocator.go index 502d299e85e8..80b6ecfd3640 100644 --- a/pkg/kv/kvserver/allocator/allocatorimpl/allocator.go +++ b/pkg/kv/kvserver/allocator/allocatorimpl/allocator.go @@ -1102,7 +1102,7 @@ func (a Allocator) RemoveTarget( ) log.VEventf(ctx, 3, "remove %s: %s", targetType, rankedCandidates) - if bad := rankedCandidates.selectBad(a.randGen); bad != nil { + if bad := rankedCandidates.selectWorst(a.randGen); bad != nil { for _, exist := range existingReplicas { if exist.StoreID == bad.store.StoreID { log.VEventf(ctx, 3, "remove target: %s", bad) diff --git a/pkg/kv/kvserver/allocator/allocatorimpl/allocator_scorer.go b/pkg/kv/kvserver/allocator/allocatorimpl/allocator_scorer.go index cb79b1017424..2a30899403d7 100644 --- a/pkg/kv/kvserver/allocator/allocatorimpl/allocator_scorer.go +++ b/pkg/kv/kvserver/allocator/allocatorimpl/allocator_scorer.go @@ -836,9 +836,9 @@ func (cl candidateList) betterThan(c candidate) candidateList { return cl } -// selectGood randomly chooses a good candidate store from a sorted (by score -// reversed) candidate list using the provided random generator. -func (cl candidateList) selectGood(randGen allocatorRand) *candidate { +// selectBest randomly chooses one of the best candidate stores from a sorted +// (by score reversed) candidate list using the provided random generator. +func (cl candidateList) selectBest(randGen allocatorRand) *candidate { cl = cl.best() if len(cl) == 0 { return nil @@ -1570,7 +1570,7 @@ func bestRebalanceTarget( if len(option.candidates) == 0 { continue } - target := option.candidates.selectGood(randGen) + target := option.candidates.selectBest(randGen) if target == nil { continue } diff --git a/pkg/kv/kvserver/allocator/allocatorimpl/allocator_scorer_test.go b/pkg/kv/kvserver/allocator/allocatorimpl/allocator_scorer_test.go index f6d6e1111ab9..a1f243ce0422 100644 --- a/pkg/kv/kvserver/allocator/allocatorimpl/allocator_scorer_test.go +++ b/pkg/kv/kvserver/allocator/allocatorimpl/allocator_scorer_test.go @@ -95,9 +95,9 @@ func TestOnlyValidAndHealthyDisk(t *testing.T) { } } -// TestSelectGoodPanic is a basic regression test against a former panic in -// selectGood when called with just invalid/full stores. -func TestSelectGoodPanic(t *testing.T) { +// TestSelectBestPanic is a basic regression test against a former panic in +// selectBest when called with just invalid/full stores. +func TestSelectBestPanic(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) @@ -107,12 +107,12 @@ func TestSelectGoodPanic(t *testing.T) { }, } allocRand := makeAllocatorRand(rand.NewSource(0)) - if good := cl.selectGood(allocRand); good != nil { - t.Errorf("cl.selectGood() got %v, want nil", good) + if good := cl.selectBest(allocRand); good != nil { + t.Errorf("cl.selectBest() got %v, want nil", good) } } -// TestCandidateSelection tests select{good,bad} and {best,worst}constraints. +// TestCandidateSelection tests select{best,worst} and {best,worst}constraints. func TestCandidateSelection(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) @@ -153,60 +153,60 @@ func TestCandidateSelection(t *testing.T) { } testCases := []struct { - candidates []scoreTuple - best []scoreTuple - worst []scoreTuple - good scoreTuple - bad scoreTuple + candidates []scoreTuple + best []scoreTuple + worst []scoreTuple + bestChosen scoreTuple + worstChosen scoreTuple }{ { - candidates: []scoreTuple{{0, 0}}, - best: []scoreTuple{{0, 0}}, - worst: []scoreTuple{{0, 0}}, - good: scoreTuple{0, 0}, - bad: scoreTuple{0, 0}, + candidates: []scoreTuple{{0, 0}}, + best: []scoreTuple{{0, 0}}, + worst: []scoreTuple{{0, 0}}, + bestChosen: scoreTuple{0, 0}, + worstChosen: scoreTuple{0, 0}, }, { - candidates: []scoreTuple{{0, 0}, {0, 1}}, - best: []scoreTuple{{0, 0}, {0, 1}}, - worst: []scoreTuple{{0, 0}, {0, 1}}, - good: scoreTuple{0, 0}, - bad: scoreTuple{0, 1}, + candidates: []scoreTuple{{0, 0}, {0, 1}}, + best: []scoreTuple{{0, 0}, {0, 1}}, + worst: []scoreTuple{{0, 0}, {0, 1}}, + bestChosen: scoreTuple{0, 0}, + worstChosen: scoreTuple{0, 1}, }, { - candidates: []scoreTuple{{0, 0}, {0, 1}, {0, 2}}, - best: []scoreTuple{{0, 0}, {0, 1}, {0, 2}}, - worst: []scoreTuple{{0, 0}, {0, 1}, {0, 2}}, - good: scoreTuple{0, 1}, - bad: scoreTuple{0, 2}, + candidates: []scoreTuple{{0, 0}, {0, 1}, {0, 2}}, + best: []scoreTuple{{0, 0}, {0, 1}, {0, 2}}, + worst: []scoreTuple{{0, 0}, {0, 1}, {0, 2}}, + bestChosen: scoreTuple{0, 1}, + worstChosen: scoreTuple{0, 2}, }, { - candidates: []scoreTuple{{1, 0}, {0, 1}}, - best: []scoreTuple{{1, 0}}, - worst: []scoreTuple{{0, 1}}, - good: scoreTuple{1, 0}, - bad: scoreTuple{0, 1}, + candidates: []scoreTuple{{1, 0}, {0, 1}}, + best: []scoreTuple{{1, 0}}, + worst: []scoreTuple{{0, 1}}, + bestChosen: scoreTuple{1, 0}, + worstChosen: scoreTuple{0, 1}, }, { - candidates: []scoreTuple{{1, 0}, {0, 1}, {0, 2}}, - best: []scoreTuple{{1, 0}}, - worst: []scoreTuple{{0, 1}, {0, 2}}, - good: scoreTuple{1, 0}, - bad: scoreTuple{0, 2}, + candidates: []scoreTuple{{1, 0}, {0, 1}, {0, 2}}, + best: []scoreTuple{{1, 0}}, + worst: []scoreTuple{{0, 1}, {0, 2}}, + bestChosen: scoreTuple{1, 0}, + worstChosen: scoreTuple{0, 2}, }, { - candidates: []scoreTuple{{1, 0}, {1, 1}, {0, 2}}, - best: []scoreTuple{{1, 0}, {1, 1}}, - worst: []scoreTuple{{0, 2}}, - good: scoreTuple{1, 0}, - bad: scoreTuple{0, 2}, + candidates: []scoreTuple{{1, 0}, {1, 1}, {0, 2}}, + best: []scoreTuple{{1, 0}, {1, 1}}, + worst: []scoreTuple{{0, 2}}, + bestChosen: scoreTuple{1, 0}, + worstChosen: scoreTuple{0, 2}, }, { - candidates: []scoreTuple{{1, 0}, {1, 1}, {0, 2}, {0, 3}}, - best: []scoreTuple{{1, 0}, {1, 1}}, - worst: []scoreTuple{{0, 2}, {0, 3}}, - good: scoreTuple{1, 0}, - bad: scoreTuple{0, 3}, + candidates: []scoreTuple{{1, 0}, {1, 1}, {0, 2}, {0, 3}}, + best: []scoreTuple{{1, 0}, {1, 1}}, + worst: []scoreTuple{{0, 2}, {0, 3}}, + bestChosen: scoreTuple{1, 0}, + worstChosen: scoreTuple{0, 3}, }, } @@ -227,24 +227,24 @@ func TestCandidateSelection(t *testing.T) { t.Errorf("expected:%s actual:%s diff:%v", formatter(e), formatter(a), pretty.Diff(e, a)) } }) - t.Run(fmt.Sprintf("good-%s", formatter(cl)), func(t *testing.T) { - good := cl.selectGood(allocRand) - if good == nil { - t.Fatalf("no good candidate found") + t.Run(fmt.Sprintf("select-best-%s", formatter(cl)), func(t *testing.T) { + best := cl.selectBest(allocRand) + if best == nil { + t.Fatalf("no 'best' candidate found") } - actual := scoreTuple{int(good.diversityScore + 0.5), good.rangeCount} - if actual != tc.good { - t.Errorf("expected:%v actual:%v", tc.good, actual) + actual := scoreTuple{int(best.diversityScore + 0.5), best.rangeCount} + if actual != tc.bestChosen { + t.Errorf("expected:%v actual:%v", tc.bestChosen, actual) } }) - t.Run(fmt.Sprintf("bad-%s", formatter(cl)), func(t *testing.T) { - bad := cl.selectBad(allocRand) - if bad == nil { - t.Fatalf("no bad candidate found") + t.Run(fmt.Sprintf("select-worst-%s", formatter(cl)), func(t *testing.T) { + worst := cl.selectWorst(allocRand) + if worst == nil { + t.Fatalf("no 'worst' candidate found") } - actual := scoreTuple{int(bad.diversityScore + 0.5), bad.rangeCount} - if actual != tc.bad { - t.Errorf("expected:%v actual:%v", tc.bad, actual) + actual := scoreTuple{int(worst.diversityScore + 0.5), worst.rangeCount} + if actual != tc.worstChosen { + t.Errorf("expected:%v actual:%v", tc.worstChosen, actual) } }) }