kvserver: remove ApplyImpact frim AllocationOp

This commit removes `ApplyImpact` from the `AllocationOp` interface entirely,
since it adds an unnecessary layer of indirection without providing much value.

Author: wenyihu6

pkg/kv/kvserver/allocator/plan/op.go

@@ -8,7 +8,6 @@ package plan
 import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/allocator"
-	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/allocator/storepool"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 )
@@ -18,9 +17,6 @@ import (
 //
 // TODO(kvoli): Add AllocationRelocateRangeOp.
 type AllocationOp interface {
-	// ApplyImpact updates the given storepool to reflect the result of
-	// applying this operation.
-	ApplyImpact(storepool storepool.AllocatorStorePool)
 	// LHBeingRemoved returns true when the leaseholder is will be removed if
 	// this operation succeeds, otherwise false.
 	LHBeingRemoved() bool
@@ -43,12 +39,6 @@ func (o AllocationTransferLeaseOp) LHBeingRemoved() bool {
 	return true
 }
 
-func (o AllocationTransferLeaseOp) ApplyImpact(storepool storepool.AllocatorStorePool) {
-	// TODO(kvoli): Currently the local storepool is updated directly in the
-	// lease transfer call, rather than in this function. Move the storepool
-	// tracking from rq.TransferLease to this function once #89771 is merged.
-}
-
 // AllocationChangeReplicasOp represents an operation to execute a change
 // replicas txn.
 type AllocationChangeReplicasOp struct {
@@ -73,14 +63,6 @@ func (o AllocationChangeReplicasOp) LHBeingRemoved() bool {
 	return false
 }
 
-// applyEstimatedImpact updates the given storepool to reflect the result
-// of applying this operation.
-func (o AllocationChangeReplicasOp) ApplyImpact(storepool storepool.AllocatorStorePool) {
-	for _, chg := range o.Chgs {
-		storepool.UpdateLocalStoreAfterRebalance(chg.Target.StoreID, o.Usage, chg.ChangeType)
-	}
-}
-
 // AllocationFinalizeAtomicReplicationOp represents an operation to finalize an
 // atomic change replicas operation and remove any remaining learners.
 type AllocationFinalizeAtomicReplicationOp struct{}
@@ -89,13 +71,11 @@ var _ AllocationOp = &AllocationFinalizeAtomicReplicationOp{}
 
 // TODO(kvoli): This always returns false, however it is possible that the LH
 // may have been removed here.
-func (o AllocationFinalizeAtomicReplicationOp) LHBeingRemoved() bool                               { return false }
-func (o AllocationFinalizeAtomicReplicationOp) ApplyImpact(storepool storepool.AllocatorStorePool) {}
+func (o AllocationFinalizeAtomicReplicationOp) LHBeingRemoved() bool { return false }
 
 // AllocationNoop represents no operation.
 type AllocationNoop struct{}
 
 var _ AllocationOp = &AllocationNoop{}
 
-func (o AllocationNoop) LHBeingRemoved() bool                               { return false }
-func (o AllocationNoop) ApplyImpact(storepool storepool.AllocatorStorePool) {}
+func (o AllocationNoop) LHBeingRemoved() bool { return false }

pkg/kv/kvserver/lease_queue.go

@@ -140,12 +140,6 @@ func (lq *leaseQueue) process(
 			return false, errors.Wrapf(err, "%s: unable to transfer lease to s%d", repl, transferOp.Target)
 		}
 
-		// TODO(wenyihu6): Initially, change.Op.ApplyImpact was used here. This was
-		// a problem since AllocationTransferLeaseOp.ApplyImpact was left
-		// unimplemented. We should either implement
-		// AllocationTransferLeaseOp.ApplyImpact correctly or remove the use of
-		// ApplyImpact entirely. The replicate queue does not have this issue since
-		// it uses rq.TransferLease, which updates the local store pool directly.
 		lq.storePool.UpdateLocalStoresAfterLeaseTransfer(
 			transferOp.Source.StoreID, transferOp.Target.StoreID, transferOp.Usage)
 	}