Skip to content

Commit

Permalink
kvserver: refine est impact for lease transfers
Browse files Browse the repository at this point in the history
Previously, when estimating the impact a lease transfer would have, we
would not differentiate between rebalance/transfer. This commit adds a
utility method `TransferImpact` to `RangeUsageInfo` which is now used
when making estimations about lease transers.

Currently, this is identical to `Load`, which was previously used
instead for transfers.

Release note: None
  • Loading branch information
kvoli committed Feb 7, 2023
1 parent 43ab658 commit 17fbc4d
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 5 deletions.
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/allocator/allocatorimpl/allocator.go
Original file line number Diff line number Diff line change
Expand Up @@ -2186,7 +2186,7 @@ func (a *Allocator) TransferLeaseTarget(
return candidates[a.randGen.Intn(len(candidates))]

case allocator.LoadConvergence:
leaseReplLoad := usageInfo.Load()
leaseReplLoad := usageInfo.TransferImpact()
candidates := make([]roachpb.StoreID, 0, len(existing)-1)
for _, repl := range existing {
if repl.StoreID != leaseRepl.StoreID() {
Expand Down
11 changes: 11 additions & 0 deletions pkg/kv/kvserver/allocator/range_usage_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,14 @@ func (r RangeUsageInfo) Load() load.Load {
dims[load.Queries] = r.QueriesPerSecond
return dims
}

// TransferImpact returns the impact of transferring the lease for the range,
// given the usage information. The impact is assumed to be symmetric, e.g. the
// receiving store of the transfer will have load = prev_load(recv) + impact
// after the transfer, whilst the sending side will have load =
// prev_load(sender) - impact after the transfer.
func (r RangeUsageInfo) TransferImpact() load.Load {
dims := load.Vector{}
dims[load.Queries] = r.QueriesPerSecond
return dims
}
8 changes: 4 additions & 4 deletions pkg/kv/kvserver/store_rebalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -715,17 +715,17 @@ func (sr *StoreRebalancer) chooseLeaseToTransfer(
// store's load. It's just unnecessary churn with no benefit to move leases
// responsible for, for example, 1 load unit on a store with 5000 load units.
const minLoadFraction = .001
if candidateReplica.RangeUsageInfo().Load().Dim(rctx.loadDimension) <
if candidateReplica.RangeUsageInfo().TransferImpact().Dim(rctx.loadDimension) <
rctx.LocalDesc.Capacity.Load().Dim(rctx.loadDimension)*minLoadFraction {
log.KvDistribution.VEventf(ctx, 3, "r%d's %s load is too little to matter relative to s%d's %s total load",
candidateReplica.GetRangeID(), candidateReplica.RangeUsageInfo().Load(),
candidateReplica.GetRangeID(), candidateReplica.RangeUsageInfo().TransferImpact(),
rctx.LocalDesc.StoreID, rctx.LocalDesc.Capacity.Load())
continue
}

desc, conf := candidateReplica.DescAndSpanConfig()
log.KvDistribution.VEventf(ctx, 3, "considering lease transfer for r%d with %s load",
desc.RangeID, candidateReplica.RangeUsageInfo().Load())
desc.RangeID, candidateReplica.RangeUsageInfo().TransferImpact())

// Check all the other voting replicas in order of increasing load.
// Learners or non-voters aren't allowed to become leaseholders or raft
Expand Down Expand Up @@ -787,7 +787,7 @@ func (sr *StoreRebalancer) chooseLeaseToTransfer(
1,
"transferring lease for r%d load=%s to store s%d load=%s from local store s%d load=%s",
desc.RangeID,
candidateReplica.RangeUsageInfo().Load(),
candidateReplica.RangeUsageInfo().TransferImpact(),
targetStore.StoreID,
targetStore.Capacity.Load(),
rctx.LocalDesc.StoreID,
Expand Down

0 comments on commit 17fbc4d

Please sign in to comment.