Skip to content

Commit

Permalink
kvserver: rename StoreList.filter
Browse files Browse the repository at this point in the history
This commit renames `StoreList`'s `filter()` method to `excludeInvalid()` as
the existing name was ambiguous.

Release justification: Fixes high priority bug

Release note: None
  • Loading branch information
aayushshah15 committed Sep 8, 2021
1 parent 3efcecf commit 153fd6b
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 15 deletions.
8 changes: 4 additions & 4 deletions pkg/kv/kvserver/allocator.go
Original file line number Diff line number Diff line change
Expand Up @@ -1276,8 +1276,8 @@ func (a *Allocator) TransferLeaseTarget(
alwaysAllowDecisionWithoutStats bool,
) roachpb.ReplicaDescriptor {
sl, _, _ := a.storePool.getStoreList(storeFilterSuspect)
sl = sl.filter(conf.Constraints)
sl = sl.filter(conf.VoterConstraints)
sl = sl.excludeInvalid(conf.Constraints)
sl = sl.excludeInvalid(conf.VoterConstraints)
// The only thing we use the storeList for is for the lease mean across the
// eligible stores, make that explicit here.
candidateLeasesMean := sl.candidateLeases.mean
Expand Down Expand Up @@ -1445,8 +1445,8 @@ func (a *Allocator) ShouldTransferLease(
}

sl, _, _ := a.storePool.getStoreList(storeFilterSuspect)
sl = sl.filter(conf.Constraints)
sl = sl.filter(conf.VoterConstraints)
sl = sl.excludeInvalid(conf.Constraints)
sl = sl.excludeInvalid(conf.VoterConstraints)
log.VEventf(ctx, 3, "ShouldTransferLease (lease-holder=%d):\n%s", leaseStoreID, sl)

// Only consider live, non-draining, non-suspect replicas.
Expand Down
4 changes: 2 additions & 2 deletions pkg/kv/kvserver/allocator_scorer.go
Original file line number Diff line number Diff line change
Expand Up @@ -1365,9 +1365,9 @@ func containsStore(stores []roachpb.StoreID, target roachpb.StoreID) bool {
return false
}

// constraintsCheck returns true iff the provided store would be a valid in a
// isStoreValid returns true iff the provided store would be a valid in a
// range with the provided constraints.
func constraintsCheck(
func isStoreValid(
store roachpb.StoreDescriptor, constraints []roachpb.ConstraintsConjunction,
) bool {
if len(constraints) == 0 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/allocator_scorer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ func TestConstraintsCheck(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
for _, s := range testStores {
valid := constraintsCheck(s, tc.constraints)
valid := isStoreValid(s, tc.constraints)
ok := tc.expected[s.StoreID]
if valid != ok {
t.Errorf("expected store %d to be %t, but got %t", s.StoreID, ok, valid)
Expand Down
6 changes: 3 additions & 3 deletions pkg/kv/kvserver/store_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -793,16 +793,16 @@ func (sl StoreList) String() string {
return buf.String()
}

// filter takes a store list and removes stores that would be explicitly invalid
// excludeInvalid takes a store list and removes stores that would be explicitly invalid
// under the given set of constraints. It maintains the original order of the
// passed in store list.
func (sl StoreList) filter(constraints []roachpb.ConstraintsConjunction) StoreList {
func (sl StoreList) excludeInvalid(constraints []roachpb.ConstraintsConjunction) StoreList {
if len(constraints) == 0 {
return sl
}
var filteredDescs []roachpb.StoreDescriptor
for _, store := range sl.stores {
if ok := constraintsCheck(store, constraints); ok {
if ok := isStoreValid(store, constraints); ok {
filteredDescs = append(filteredDescs, store)
}
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/kv/kvserver/store_pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func verifyStoreList(
sl, aliveStoreCount, throttled = sp.getStoreListFromIDs(storeIDs, filter)
}
throttledStoreCount := len(throttled)
sl = sl.filter(constraints)
sl = sl.excludeInvalid(constraints)
if aliveStoreCount != expectedAliveStoreCount {
return errors.Errorf("expected AliveStoreCount %d does not match actual %d",
expectedAliveStoreCount, aliveStoreCount)
Expand Down Expand Up @@ -216,7 +216,7 @@ func TestStorePoolGetStoreList(t *testing.T) {
required := []string{"ssd", "dc"}
// Nothing yet.
sl, _, _ := sp.getStoreList(storeFilterNone)
sl = sl.filter(constraints)
sl = sl.excludeInvalid(constraints)
if len(sl.stores) != 0 {
t.Errorf("expected no stores, instead %+v", sl.stores)
}
Expand Down Expand Up @@ -485,7 +485,7 @@ func TestStoreListFilter(t *testing.T) {
}
}

filtered := sl.filter(constraints)
filtered := sl.excludeInvalid(constraints)
if !reflect.DeepEqual(expected, filtered.stores) {
t.Errorf("did not get expected stores %s", pretty.Diff(expected, filtered.stores))
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/kv/kvserver/store_rebalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -483,8 +483,8 @@ func (sr *StoreRebalancer) chooseLeaseToTransfer(
continue
}

filteredStoreList := storeList.filter(conf.Constraints)
filteredStoreList = storeList.filter(conf.VoterConstraints)
filteredStoreList := storeList.excludeInvalid(conf.Constraints)
filteredStoreList = storeList.excludeInvalid(conf.VoterConstraints)
if sr.rq.allocator.followTheWorkloadPrefersLocal(
ctx,
filteredStoreList,
Expand Down

0 comments on commit 153fd6b

Please sign in to comment.