Skip to content

Commit 144b4a0

Browse files
committed
mmaintegration: address minor code review comments
This commit address minor code review comments.
1 parent f562450 commit 144b4a0

File tree

4 files changed

+8
-10
lines changed

4 files changed

+8
-10
lines changed

pkg/kv/kvserver/lease_queue.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,9 @@ func (lq *leaseQueue) process(
150150
// changes to store pool and inform mma.
151151
lq.as.PostApply(changeID, err == nil /*success*/)
152152
if err != nil {
153-
// TODO(wenyihu6): we need to call post apply with false when as is more meaningful
154153
return false, errors.Wrapf(err, "%s: unable to transfer lease to s%d", repl, transferOp.Target)
155154
}
156155
}
157-
158156
return true, nil
159157
}
160158

pkg/kv/kvserver/mmaintegration/allocator_op.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ type trackedAllocatorChange struct {
2222
changeIDs []mmaprototype.ChangeID
2323
// Usage is range load usage.
2424
usage allocator.RangeUsageInfo
25-
// Only one of the following two fields will be set.
25+
// Exactly one of the following two fields will be set.
2626
leaseTransferOp *leaseTransferOp
2727
changeReplicasOp *changeReplicasOp
2828
}

pkg/kv/kvserver/mmaintegration/allocator_sync.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@ type SyncChangeID uint64
3232
// has been disabled postapply or other components call into allocator sync when
3333
// mma is disabled.)
3434

35-
// AllocatorSync is a component that coordinates changes from external
36-
// components (e.g. replicate/lease queue) with mma. When mma is disabled,
37-
// its sole purpose is to track and apply changes to the store pool upon
38-
// success.
35+
// AllocatorSync is a component that coordinates changes from all components
36+
// (including mma/replicate/lease queue) with mma and store pool. When mma is
37+
// disabled, its sole purpose is to track and apply changes to the store pool
38+
// upon success.
3939
type AllocatorSync struct {
4040
sp *storepool.StorePool
4141
st *cluster.Settings
@@ -85,7 +85,7 @@ func mmaRangeLoad(rangeUsageInfo allocator.RangeUsageInfo) mmaprototype.RangeLoa
8585
func (as *AllocatorSync) addTrackedChange(change trackedAllocatorChange) SyncChangeID {
8686
as.mu.Lock()
8787
defer as.mu.Unlock()
88-
as.mu.changeSeqGen += 1
88+
as.mu.changeSeqGen++
8989
syncChangeID := as.mu.changeSeqGen
9090
as.mu.trackedChanges[syncChangeID] = change
9191
return syncChangeID

pkg/kv/kvserver/mmaintegration/mma_conversion.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ func convertLeaseTransferToMMA(
2323
transferFrom, transferTo roachpb.ReplicationTarget,
2424
) []mmaprototype.ReplicaChange {
2525
// TODO(wenyihu6): we are passing existing replicas to
26-
// mma.RegisterExternalChanges just to get the add and remove replica state.
27-
// See if things could be cleaned up.
26+
// mmaprototype.MakeLeaseTransferChanges just to get the add and remove
27+
// replica state. See if things could be cleaned up.
2828
existingReplicas := make([]mmaprototype.StoreIDAndReplicaState, len(desc.InternalReplicas))
2929
for i, replica := range desc.Replicas().Descriptors() {
3030
existingReplicas[i] = mmaprototype.StoreIDAndReplicaState{

0 commit comments

Comments
 (0)