Skip to content

Commit a1eab56

Browse files
committed
kvserver: update store pool for leasequeue.process
Previously, the lease queue didn’t update the local store pool correctly because `AllocationTransferLeaseOp.ApplyImpact` was unimplemented. This wasn’t an issue for the replicate queue, as it updates the local store pool directly through `rq.TransferLease` by calling `UpdateLocalStoresAfterLeaseTransfer`. This patch addresses the issue by calling `UpdateLocalStoresAfterLeaseTransfer` after `replica.AdminTransferLease` inside the lease queue. This fixes a bug where the allocator may make rebalancing decisions based on stale store pool data, failing to account for recent lease transfers not yet reflected in gossip. This state should eventually be corrected when real store capacity is measured and gossiped, and store pools are refreshed. This bug was found when reading the code. Epic: none Release note (bug fixes): Fixed a bug present since v24.1 where the allocator could make rebalancing decisions based on stale data, failing to account for recent local lease transfers not yet reflected in store capacity or gossip.
1 parent 9d40033 commit a1eab56

File tree

1 file changed

+9
-1
lines changed

1 file changed

+9
-1
lines changed

pkg/kv/kvserver/lease_queue.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,15 @@ func (lq *leaseQueue) process(
139139
if err := repl.AdminTransferLease(ctx, transferOp.Target.StoreID, false /* bypassSafetyChecks */); err != nil {
140140
return false, errors.Wrapf(err, "%s: unable to transfer lease to s%d", repl, transferOp.Target)
141141
}
142-
change.Op.ApplyImpact(lq.storePool)
142+
143+
// TODO(wenyihu6): Initially, change.Op.ApplyImpact was used here. This was
144+
// a problem since AllocationTransferLeaseOp.ApplyImpact was left
145+
// unimplemented. We should either implement
146+
// AllocationTransferLeaseOp.ApplyImpact correctly or remove the use of
147+
// ApplyImpact entirely. The replicate queue does not have this issue since
148+
// it uses rq.TransferLease, which updates the local store pool directly.
149+
lq.storePool.UpdateLocalStoresAfterLeaseTransfer(
150+
transferOp.Source.StoreID, transferOp.Target.StoreID, transferOp.Usage)
143151
}
144152

145153
return true, nil

0 commit comments

Comments
 (0)