diff --git a/pkg/kv/kvserver/allocator/allocatorimpl/allocator.go b/pkg/kv/kvserver/allocator/allocatorimpl/allocator.go index 86b1902c1382..9aa1eb43131a 100644 --- a/pkg/kv/kvserver/allocator/allocatorimpl/allocator.go +++ b/pkg/kv/kvserver/allocator/allocatorimpl/allocator.go @@ -366,10 +366,10 @@ func (s ReplicaStatus) String() string { // SafeValue implements the redact.SafeValue interface. func (t ReplicaStatus) SafeValue() {} -type transferDecision int +type accessLocalityTransferDecision int const ( - _ transferDecision = iota + _ accessLocalityTransferDecision = iota shouldTransfer shouldNotTransfer decideWithoutStats @@ -2523,6 +2523,100 @@ func getLoadDelta( return maxCandidateLoad - minCandidateLoad } +// TransferLeaseDecision indicates whether a range lease should be transferred +// and if so, for what reason. +type TransferLeaseDecision int + +const ( + _ TransferLeaseDecision = iota + // DontTransferLeaseCountBalanced indicates the load/lease counts of the + // valid replica stores are balanced within the target threshold and + // therefore the lease should not be transferred. + DontTransferLeaseBalanced + // DontTransferLeaseNoValidTargets indicates the lease should no be + // transferred from the current leaseholder because there are no valid + // leaseholder targets. + DontTransferLeaseNoValidTargets + // DontTransferLeaseNoStoreDescriptor indicates the lease should not be + // transferred because the current leaseholder's store descriptor cannot be + // found. This can occur on startup, before gossip has propogated the local + // descriptor. + DontTransferLeaseNoStoreDescriptor + // TransferLeaseForCountBalance indicates the lease should be transferred to + // better balance lease counts. + TransferLeaseForCountBalance + // TransferLeaseForAccessLocality indicates the lease should be transferred + // for better access locality. + TransferLeaseForAccessLocality + // TransferLeaseForIOOverload indicates the lease should be transferred + // because the current leaseholder's store is IO overloaded. + TransferLeaseForIOOverload + // TransferLeaseForPreferences indicates the lease should be transferred + // because there is a more preferred leaseholder according the applied range + // lease preferences. + TransferLeaseForPreferences +) + +// ShouldTransfer returns true when the lease should be transferred, false +// otherwise. +func (t TransferLeaseDecision) ShouldTransfer() bool { + switch t { + case TransferLeaseForCountBalance, TransferLeaseForAccessLocality, + TransferLeaseForIOOverload, TransferLeaseForPreferences: + return true + case DontTransferLeaseBalanced, DontTransferLeaseNoValidTargets, + DontTransferLeaseNoStoreDescriptor: + return false + default: + panic(fmt.Sprintf("unknown transfer lease decision %d", t)) + } +} + +// Priority returns the relative urgency of the lease transfer decision. The +// priority may be used to determine the ordering of lease transfers when +// multiple should occur. +func (t TransferLeaseDecision) Priority() float64 { + switch t { + case TransferLeaseForPreferences: + return 300 + case TransferLeaseForIOOverload: + return 200 + case TransferLeaseForAccessLocality: + return 100 + case TransferLeaseForCountBalance: + return 0 + case DontTransferLeaseBalanced, DontTransferLeaseNoValidTargets, + DontTransferLeaseNoStoreDescriptor: + return 0 + default: + panic(fmt.Sprintf("unknown transfer lease decision %d", t)) + } +} + +// SafeValue implements the redact.SafeValue interface. +func (t TransferLeaseDecision) SafeValue() {} + +func (t TransferLeaseDecision) String() string { + switch t { + case TransferLeaseForCountBalance: + return "transfer(lease count)" + case TransferLeaseForAccessLocality: + return "transfer(access locality)" + case TransferLeaseForIOOverload: + return "transfer(io-overload)" + case TransferLeaseForPreferences: + return "transfer(preferences)" + case DontTransferLeaseBalanced: + return "no-transfer(balanced)" + case DontTransferLeaseNoStoreDescriptor: + return "no-transfer(missing store descriptor)" + case DontTransferLeaseNoValidTargets: + return "no-transfer(no valid targets)" + default: + panic(fmt.Sprintf("unknown transfer lease decision %d", t)) + } +} + // ShouldTransferLease returns true if the specified store is overfull in terms // of leases with respect to the other stores matching the specified // attributes. @@ -2538,14 +2632,14 @@ func (a *Allocator) ShouldTransferLease( GetFirstIndex() kvpb.RaftIndex }, usageInfo allocator.RangeUsageInfo, -) bool { +) TransferLeaseDecision { if a.leaseholderShouldMoveDueToPreferences(ctx, storePool, conf, leaseRepl, existing) { - return true + return TransferLeaseForPreferences } if a.leaseholderShouldMoveDueToIOOverload( ctx, storePool, existing, leaseRepl.StoreID(), a.IOOverloadOptions()) { - return true + return TransferLeaseForIOOverload } existing = a.ValidLeaseTargets( @@ -2560,11 +2654,11 @@ func (a *Allocator) ShouldTransferLease( // Short-circuit if there are no valid targets out there. if len(existing) == 0 || (len(existing) == 1 && existing[0].StoreID == leaseRepl.StoreID()) { - return false + return DontTransferLeaseNoValidTargets } source, ok := storePool.GetStoreDescriptor(leaseRepl.StoreID()) if !ok { - return false + return DontTransferLeaseNoStoreDescriptor } sl, _, _ := storePool.GetStoreList(storepool.StoreFilterSuspect) @@ -2581,20 +2675,23 @@ func (a *Allocator) ShouldTransferLease( nil, sl.CandidateLeases.Mean, ) - var result bool + var result TransferLeaseDecision switch transferDec { case shouldNotTransfer: - result = false + result = DontTransferLeaseBalanced case shouldTransfer: - result = true + result = TransferLeaseForAccessLocality case decideWithoutStats: - result = a.shouldTransferLeaseForLeaseCountConvergence(ctx, storePool, sl, source, existing) + if a.shouldTransferLeaseForLeaseCountConvergence(ctx, storePool, sl, source, existing) { + result = TransferLeaseForCountBalance + } else { + result = DontTransferLeaseBalanced + } default: log.KvDistribution.Fatalf(ctx, "unexpected transfer decision %d", transferDec) } - log.KvDistribution.VEventf( - ctx, 3, "ShouldTransferLease decision (lease-holder=s%d): %t", leaseRepl.StoreID(), result, + ctx, 3, "ShouldTransferLease decision (lease-holder=s%d): %v", leaseRepl.StoreID(), result, ) return result } @@ -2634,7 +2731,7 @@ func (a Allocator) shouldTransferLeaseForAccessLocality( usageInfo allocator.RangeUsageInfo, rebalanceAdjustments map[roachpb.StoreID]float64, candidateLeasesMean float64, -) (transferDecision, roachpb.ReplicaDescriptor) { +) (accessLocalityTransferDecision, roachpb.ReplicaDescriptor) { // Only use load-based rebalancing if it's enabled and we have both // stats and locality information to base our decision on. if usageInfo.RequestLocality == nil || diff --git a/pkg/kv/kvserver/allocator/allocatorimpl/allocator_test.go b/pkg/kv/kvserver/allocator/allocatorimpl/allocator_test.go index 5541b894e361..3a3a7a3c7c0c 100644 --- a/pkg/kv/kvserver/allocator/allocatorimpl/allocator_test.go +++ b/pkg/kv/kvserver/allocator/allocatorimpl/allocator_test.go @@ -2682,21 +2682,21 @@ func TestAllocatorShouldTransferLease(t *testing.T) { testCases := []struct { leaseholder roachpb.StoreID existing []roachpb.ReplicaDescriptor - expected bool + expected TransferLeaseDecision }{ - {leaseholder: 1, existing: nil, expected: false}, - {leaseholder: 2, existing: nil, expected: false}, - {leaseholder: 3, existing: nil, expected: false}, - {leaseholder: 4, existing: nil, expected: false}, - {leaseholder: 3, existing: replicas(1), expected: true}, - {leaseholder: 3, existing: replicas(1, 2), expected: true}, - {leaseholder: 3, existing: replicas(2), expected: false}, - {leaseholder: 3, existing: replicas(3), expected: false}, - {leaseholder: 3, existing: replicas(4), expected: false}, - {leaseholder: 4, existing: replicas(1), expected: true}, - {leaseholder: 4, existing: replicas(2), expected: true}, - {leaseholder: 4, existing: replicas(3), expected: true}, - {leaseholder: 4, existing: replicas(1, 2, 3), expected: true}, + {leaseholder: 1, existing: nil, expected: DontTransferLeaseNoValidTargets}, + {leaseholder: 2, existing: nil, expected: DontTransferLeaseNoValidTargets}, + {leaseholder: 3, existing: nil, expected: DontTransferLeaseNoValidTargets}, + {leaseholder: 4, existing: nil, expected: DontTransferLeaseNoValidTargets}, + {leaseholder: 3, existing: replicas(1), expected: TransferLeaseForCountBalance}, + {leaseholder: 3, existing: replicas(1, 2), expected: TransferLeaseForCountBalance}, + {leaseholder: 3, existing: replicas(2), expected: DontTransferLeaseBalanced}, + {leaseholder: 3, existing: replicas(3), expected: DontTransferLeaseNoValidTargets}, + {leaseholder: 3, existing: replicas(4), expected: DontTransferLeaseBalanced}, + {leaseholder: 4, existing: replicas(1), expected: TransferLeaseForCountBalance}, + {leaseholder: 4, existing: replicas(2), expected: TransferLeaseForCountBalance}, + {leaseholder: 4, existing: replicas(3), expected: TransferLeaseForCountBalance}, + {leaseholder: 4, existing: replicas(1, 2, 3), expected: TransferLeaseForCountBalance}, } for _, c := range testCases { t.Run("", func(t *testing.T) { @@ -2752,20 +2752,20 @@ func TestAllocatorShouldTransferLeaseDraining(t *testing.T) { testCases := []struct { leaseholder roachpb.StoreID existing []roachpb.ReplicaDescriptor - expected bool + expected TransferLeaseDecision }{ - {leaseholder: 1, existing: nil, expected: false}, - {leaseholder: 2, existing: nil, expected: false}, - {leaseholder: 3, existing: nil, expected: false}, - {leaseholder: 4, existing: nil, expected: false}, - {leaseholder: 2, existing: replicas(1), expected: false}, - {leaseholder: 3, existing: replicas(1), expected: false}, - {leaseholder: 3, existing: replicas(1, 2), expected: false}, - {leaseholder: 3, existing: replicas(1, 2, 4), expected: false}, - {leaseholder: 4, existing: replicas(1), expected: false}, - {leaseholder: 4, existing: replicas(1, 2), expected: true}, - {leaseholder: 4, existing: replicas(1, 3), expected: true}, - {leaseholder: 4, existing: replicas(1, 2, 3), expected: true}, + {leaseholder: 1, existing: nil, expected: DontTransferLeaseNoValidTargets}, + {leaseholder: 2, existing: nil, expected: DontTransferLeaseNoValidTargets}, + {leaseholder: 3, existing: nil, expected: DontTransferLeaseNoValidTargets}, + {leaseholder: 4, existing: nil, expected: DontTransferLeaseNoValidTargets}, + {leaseholder: 2, existing: replicas(1), expected: DontTransferLeaseNoValidTargets}, + {leaseholder: 3, existing: replicas(1), expected: DontTransferLeaseNoValidTargets}, + {leaseholder: 3, existing: replicas(1, 2), expected: DontTransferLeaseBalanced}, + {leaseholder: 3, existing: replicas(1, 2, 4), expected: DontTransferLeaseBalanced}, + {leaseholder: 4, existing: replicas(1), expected: DontTransferLeaseNoValidTargets}, + {leaseholder: 4, existing: replicas(1, 2), expected: TransferLeaseForCountBalance}, + {leaseholder: 4, existing: replicas(1, 3), expected: TransferLeaseForCountBalance}, + {leaseholder: 4, existing: replicas(1, 2, 3), expected: TransferLeaseForCountBalance}, } for _, c := range testCases { t.Run("", func(t *testing.T) { @@ -2826,7 +2826,7 @@ func TestAllocatorShouldTransferSuspected(t *testing.T) { &mockRepl{storeID: 2, replicationFactor: 3}, allocator.RangeUsageInfo{}, ) - require.Equal(t, expected, result) + require.Equal(t, expected, result.ShouldTransfer()) } timeAfterNodeSuspect := liveness.TimeAfterNodeSuspect.Get(&a.st.SV) // Based on capacity node 1 is desirable. @@ -2861,7 +2861,7 @@ func TestAllocatorShouldTransferLeaseIOOverload(t *testing.T) { leaseCounts, IOScores []float64 leaseholder roachpb.StoreID excludeLeaseRepl bool - expected bool + expected TransferLeaseDecision enforcement IOOverloadEnforcementLevel }{ { @@ -2869,7 +2869,7 @@ func TestAllocatorShouldTransferLeaseIOOverload(t *testing.T) { leaseCounts: floats(100, 100, 100, 100, 100), IOScores: floats(2.5, 1.5, 0.5, 0, 0), leaseholder: 1, - expected: false, + expected: DontTransferLeaseBalanced, enforcement: IOOverloadThresholdBlockTransfers, }, { @@ -2879,7 +2879,7 @@ func TestAllocatorShouldTransferLeaseIOOverload(t *testing.T) { leaseholder: 1, // Store 3 is above the threshold (1.0 > 0.8), but equal to the avg (1.0), so // it is still considered a non-IO-overloaded candidate. - expected: true, + expected: TransferLeaseForIOOverload, enforcement: IOOverloadThresholdShed, }, { @@ -2887,7 +2887,7 @@ func TestAllocatorShouldTransferLeaseIOOverload(t *testing.T) { leaseCounts: floats(0, 100, 100, 400, 400), IOScores: floats(2.5, 1.5, 0.5, 0, 0), leaseholder: 5, - expected: true, + expected: TransferLeaseForCountBalance, enforcement: IOOverloadThresholdIgnore, }, { @@ -2895,7 +2895,7 @@ func TestAllocatorShouldTransferLeaseIOOverload(t *testing.T) { leaseCounts: floats(0, 0, 0, 0, 0), IOScores: floats(0.89, 0, 0, 0, 0), leaseholder: 1, - expected: false, + expected: DontTransferLeaseBalanced, enforcement: IOOverloadThresholdShed, }, } @@ -3006,54 +3006,55 @@ func TestAllocatorLeasePreferences(t *testing.T) { preferences []roachpb.LeasePreference expectAllowLeaseRepl roachpb.StoreID /* excludeLeaseRepl = false */ expectExcludeLeaseRepl roachpb.StoreID /* excludeLeaseRepl = true */ + expectShouldTransfer TransferLeaseDecision }{ - {1, nil, preferDC1, 0, 0}, - {1, replicas(1, 2, 3, 4), preferDC1, 0, 2}, - {1, replicas(2, 3, 4), preferDC1, 0, 2}, - {2, replicas(1, 2, 3, 4), preferDC1, 1, 1}, - {2, replicas(2, 3, 4), preferDC1, 0, 3}, - {4, replicas(2, 3, 4), preferDC1, 2, 2}, - {1, nil, preferDC4Then3Then2, 0, 0}, - {1, replicas(1, 2, 3, 4), preferDC4Then3Then2, 4, 4}, - {1, replicas(1, 2, 3), preferDC4Then3Then2, 3, 3}, - {1, replicas(1, 2), preferDC4Then3Then2, 2, 2}, - {3, replicas(1, 2, 3, 4), preferDC4Then3Then2, 4, 4}, - {3, replicas(1, 2, 3), preferDC4Then3Then2, 0, 2}, - {3, replicas(1, 3), preferDC4Then3Then2, 0, 1}, - {4, replicas(1, 2, 3, 4), preferDC4Then3Then2, 0, 3}, - {4, replicas(1, 2, 4), preferDC4Then3Then2, 0, 2}, - {4, replicas(1, 4), preferDC4Then3Then2, 0, 1}, - {1, replicas(1, 2, 3, 4), preferN2ThenS3, 2, 2}, - {1, replicas(1, 3, 4), preferN2ThenS3, 3, 3}, - {1, replicas(1, 4), preferN2ThenS3, 0, 4}, - {2, replicas(1, 2, 3, 4), preferN2ThenS3, 0, 3}, - {2, replicas(1, 2, 4), preferN2ThenS3, 0, 1}, - {3, replicas(1, 2, 3, 4), preferN2ThenS3, 2, 2}, - {3, replicas(1, 3, 4), preferN2ThenS3, 0, 1}, - {4, replicas(1, 4), preferN2ThenS3, 1, 1}, - {1, replicas(1, 2, 3, 4), preferNotS1ThenNotN2, 2, 2}, - {1, replicas(1, 3, 4), preferNotS1ThenNotN2, 3, 3}, - {1, replicas(1, 2), preferNotS1ThenNotN2, 2, 2}, - {1, replicas(1), preferNotS1ThenNotN2, 0, 0}, - {2, replicas(1, 2, 3, 4), preferNotS1ThenNotN2, 0, 3}, - {2, replicas(2, 3, 4), preferNotS1ThenNotN2, 0, 3}, - {2, replicas(1, 2, 3), preferNotS1ThenNotN2, 0, 3}, - {2, replicas(1, 2, 4), preferNotS1ThenNotN2, 0, 4}, - {4, replicas(1, 2, 3, 4), preferNotS1ThenNotN2, 2, 2}, - {4, replicas(1, 4), preferNotS1ThenNotN2, 0, 1}, - {1, replicas(1, 2, 3, 4), preferNotS1AndNotN2, 3, 3}, - {1, replicas(1, 2), preferNotS1AndNotN2, 0, 2}, - {2, replicas(1, 2, 3, 4), preferNotS1AndNotN2, 3, 3}, - {2, replicas(2, 3, 4), preferNotS1AndNotN2, 3, 3}, - {2, replicas(1, 2, 3), preferNotS1AndNotN2, 3, 3}, - {2, replicas(1, 2, 4), preferNotS1AndNotN2, 4, 4}, - {3, replicas(1, 3), preferNotS1AndNotN2, 0, 1}, - {4, replicas(1, 4), preferNotS1AndNotN2, 0, 1}, - {1, replicas(1, 2, 3, 4), preferMatchesNothing, 0, 2}, - {2, replicas(1, 2, 3, 4), preferMatchesNothing, 0, 1}, - {3, replicas(1, 3, 4), preferMatchesNothing, 1, 1}, - {4, replicas(1, 3, 4), preferMatchesNothing, 1, 1}, - {4, replicas(2, 3, 4), preferMatchesNothing, 2, 2}, + {1, nil, preferDC1, 0, 0, DontTransferLeaseNoValidTargets}, + {1, replicas(1, 2, 3, 4), preferDC1, 0, 2, DontTransferLeaseNoValidTargets}, + {1, replicas(2, 3, 4), preferDC1, 0, 2, DontTransferLeaseBalanced}, + {2, replicas(1, 2, 3, 4), preferDC1, 1, 1, TransferLeaseForPreferences}, + {2, replicas(2, 3, 4), preferDC1, 0, 3, DontTransferLeaseBalanced}, + {4, replicas(2, 3, 4), preferDC1, 2, 2, TransferLeaseForCountBalance}, + {1, nil, preferDC4Then3Then2, 0, 0, DontTransferLeaseNoValidTargets}, + {1, replicas(1, 2, 3, 4), preferDC4Then3Then2, 4, 4, TransferLeaseForPreferences}, + {1, replicas(1, 2, 3), preferDC4Then3Then2, 3, 3, TransferLeaseForPreferences}, + {1, replicas(1, 2), preferDC4Then3Then2, 2, 2, TransferLeaseForPreferences}, + {3, replicas(1, 2, 3, 4), preferDC4Then3Then2, 4, 4, TransferLeaseForPreferences}, + {3, replicas(1, 2, 3), preferDC4Then3Then2, 0, 2, DontTransferLeaseNoValidTargets}, + {3, replicas(1, 3), preferDC4Then3Then2, 0, 1, DontTransferLeaseNoValidTargets}, + {4, replicas(1, 2, 3, 4), preferDC4Then3Then2, 0, 3, DontTransferLeaseNoValidTargets}, + {4, replicas(1, 2, 4), preferDC4Then3Then2, 0, 2, DontTransferLeaseNoValidTargets}, + {4, replicas(1, 4), preferDC4Then3Then2, 0, 1, DontTransferLeaseNoValidTargets}, + {1, replicas(1, 2, 3, 4), preferN2ThenS3, 2, 2, TransferLeaseForPreferences}, + {1, replicas(1, 3, 4), preferN2ThenS3, 3, 3, TransferLeaseForPreferences}, + {1, replicas(1, 4), preferN2ThenS3, 0, 4, DontTransferLeaseBalanced}, + {2, replicas(1, 2, 3, 4), preferN2ThenS3, 0, 3, DontTransferLeaseNoValidTargets}, + {2, replicas(1, 2, 4), preferN2ThenS3, 0, 1, DontTransferLeaseNoValidTargets}, + {3, replicas(1, 2, 3, 4), preferN2ThenS3, 2, 2, TransferLeaseForPreferences}, + {3, replicas(1, 3, 4), preferN2ThenS3, 0, 1, DontTransferLeaseNoValidTargets}, + {4, replicas(1, 4), preferN2ThenS3, 1, 1, TransferLeaseForCountBalance}, + {1, replicas(1, 2, 3, 4), preferNotS1ThenNotN2, 2, 2, TransferLeaseForPreferences}, + {1, replicas(1, 3, 4), preferNotS1ThenNotN2, 3, 3, TransferLeaseForPreferences}, + {1, replicas(1, 2), preferNotS1ThenNotN2, 2, 2, TransferLeaseForPreferences}, + {1, replicas(1), preferNotS1ThenNotN2, 0, 0, DontTransferLeaseNoValidTargets}, + {2, replicas(1, 2, 3, 4), preferNotS1ThenNotN2, 0, 3, DontTransferLeaseBalanced}, + {2, replicas(2, 3, 4), preferNotS1ThenNotN2, 0, 3, DontTransferLeaseBalanced}, + {2, replicas(1, 2, 3), preferNotS1ThenNotN2, 0, 3, DontTransferLeaseBalanced}, + {2, replicas(1, 2, 4), preferNotS1ThenNotN2, 0, 4, DontTransferLeaseBalanced}, + {4, replicas(1, 2, 3, 4), preferNotS1ThenNotN2, 2, 2, TransferLeaseForCountBalance}, + {4, replicas(1, 4), preferNotS1ThenNotN2, 0, 1, DontTransferLeaseNoValidTargets}, + {1, replicas(1, 2, 3, 4), preferNotS1AndNotN2, 3, 3, TransferLeaseForPreferences}, + {1, replicas(1, 2), preferNotS1AndNotN2, 0, 2, DontTransferLeaseBalanced}, + {2, replicas(1, 2, 3, 4), preferNotS1AndNotN2, 3, 3, TransferLeaseForPreferences}, + {2, replicas(2, 3, 4), preferNotS1AndNotN2, 3, 3, TransferLeaseForPreferences}, + {2, replicas(1, 2, 3), preferNotS1AndNotN2, 3, 3, TransferLeaseForPreferences}, + {2, replicas(1, 2, 4), preferNotS1AndNotN2, 4, 4, TransferLeaseForPreferences}, + {3, replicas(1, 3), preferNotS1AndNotN2, 0, 1, DontTransferLeaseNoValidTargets}, + {4, replicas(1, 4), preferNotS1AndNotN2, 0, 1, DontTransferLeaseNoValidTargets}, + {1, replicas(1, 2, 3, 4), preferMatchesNothing, 0, 2, DontTransferLeaseBalanced}, + {2, replicas(1, 2, 3, 4), preferMatchesNothing, 0, 1, DontTransferLeaseBalanced}, + {3, replicas(1, 3, 4), preferMatchesNothing, 1, 1, TransferLeaseForCountBalance}, + {4, replicas(1, 3, 4), preferMatchesNothing, 1, 1, TransferLeaseForCountBalance}, + {4, replicas(2, 3, 4), preferMatchesNothing, 2, 2, TransferLeaseForCountBalance}, } for _, c := range testCases { @@ -3071,9 +3072,8 @@ func TestAllocatorLeasePreferences(t *testing.T) { }, allocator.RangeUsageInfo{}, ) - expectTransfer := c.expectAllowLeaseRepl != 0 - if expectTransfer != result { - t.Errorf("expected %v, but found %v", expectTransfer, result) + if c.expectShouldTransfer != result { + t.Errorf("expected %v, but found %v", c.expectShouldTransfer, result) } target := a.TransferLeaseTarget( ctx, diff --git a/pkg/kv/kvserver/allocator/plan/replicate.go b/pkg/kv/kvserver/allocator/plan/replicate.go index d9acac3218c5..c623c2a0aa6b 100644 --- a/pkg/kv/kvserver/allocator/plan/replicate.go +++ b/pkg/kv/kvserver/allocator/plan/replicate.go @@ -204,8 +204,8 @@ func (rp ReplicaPlanner) ShouldPlanChange( } // If the lease is valid, check to see if we should transfer it. - if canTransferLeaseFrom(ctx, repl, conf) && - rp.allocator.ShouldTransferLease( + if canTransferLeaseFrom(ctx, repl, conf) { + decision := rp.allocator.ShouldTransferLease( ctx, rp.storePool, desc, @@ -213,9 +213,12 @@ func (rp ReplicaPlanner) ShouldPlanChange( voterReplicas, repl, repl.RangeUsageInfo(), - ) { - log.KvDistribution.VEventf(ctx, 2, "lease transfer needed, enqueuing") - return true, 0 + ) + if decision.ShouldTransfer() { + log.KvDistribution.VEventf(ctx, + 2, "lease transfer needed %v, enqueuing", decision) + return true, 0 + } } leaseStatus := repl.LeaseStatusAt(ctx, now) diff --git a/pkg/testutils/lint/passes/redactcheck/redactcheck.go b/pkg/testutils/lint/passes/redactcheck/redactcheck.go index 3f5c0605ce30..b0b4716e3fa3 100644 --- a/pkg/testutils/lint/passes/redactcheck/redactcheck.go +++ b/pkg/testutils/lint/passes/redactcheck/redactcheck.go @@ -93,9 +93,10 @@ func runAnalyzer(pass *analysis.Pass) (interface{}, error) { "LeaseTransferOutcome": {}, }, "github.com/cockroachdb/cockroach/pkg/kv/kvserver/allocator/allocatorimpl": { - "AllocatorAction": {}, - "TargetReplicaType": {}, - "ReplicaStatus": {}, + "AllocatorAction": {}, + "TargetReplicaType": {}, + "ReplicaStatus": {}, + "TransferLeaseDecision": {}, }, "github.com/cockroachdb/cockroach/pkg/kv/kvserver/allocator/load": { "Dimension": {},