Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
134585: rac2: clear scratchEvent after using r=sumeerbhola a=pav-kv

The `scratchEvent` holds a `LogSnapshot`. We do not want to hold it after it is used. Holding it longer may lead to raft entries outliving the corresponding proposals and increase memory footprint. The `LogSnapshot` is also unsafe to use outside a single `raftMu` scope.

Epic: none
Release note: none

Co-authored-by: Pavel Kalinnikov <[email protected]>
  • Loading branch information
craig[bot] and pav-kv committed Nov 8, 2024
2 parents c0eeaf2 + 4a9c6cc commit 4a6d2af
Showing 1 changed file with 4 additions and 0 deletions.
4 changes: 4 additions & 0 deletions pkg/kv/kvserver/kvflowcontrol/rac2/range_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1069,6 +1069,8 @@ func (rc *rangeController) HandleRaftEventRaftMuLocked(ctx context.Context, e Ra
var numOptionalForceFlushes [2]int
for r, rs := range rc.replicaMap {
info := e.ReplicasStateInfo[r]
// Defensive, since we already clear the scratchEvent later in this method.
// Intended invariant: scratchEvent is always empty except in this method.
rs.scratchEvent = raftEventForReplica{}
mode := e.MsgAppMode
if info.State == tracker.StateReplicate {
Expand Down Expand Up @@ -1168,6 +1170,8 @@ func (rc *rangeController) HandleRaftEventRaftMuLocked(ctx context.Context, e Ra
}
}
shouldWaitChange = rs.handleReadyEntriesRaftMuLocked(ctx, rs.scratchEvent, rd) || shouldWaitChange
// Clear scratchEvent, since it contains a reference to a raft.LogSnapshot.
rs.scratchEvent = raftEventForReplica{}
}

// If there was a quorum change, update the voter sets, triggering the
Expand Down

0 comments on commit 4a6d2af

Please sign in to comment.