Skip to content

Commit

Permalink
replica_rac2: remove unlocked pending updates read
Browse files Browse the repository at this point in the history
In `ProcessPiggybackedAdmittedAtLeaderRaftMuLocked`, we swap the
pending updates map with another one before releasing the mutex, in
order to minimize the time holding a mutex. Subsequently, the method
would then check if the updates map was empty by calling `len()`,
performing a read.

When the updates map was not empty, the read was against a map which was
now only referenced in this function, because a new map was swapped in
under the lock.

When the updates map was empty, the map isn't swapped out and is
referenced by the receiver struct instance, reading a ref without
locking.

Return whether the updates map was empty, to avoid racing when reading
the length of the shared ref empty map.

Part of: #130187
Release note: None
  • Loading branch information
kvoli committed Sep 20, 2024
1 parent a4c6966 commit 8b17cd7
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions pkg/kv/kvserver/kvflowcontrol/replica_rac2/processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -1022,15 +1022,16 @@ func (p *processorImpl) ProcessPiggybackedAdmittedAtLeaderRaftMuLocked(ctx conte
var updates map[roachpb.ReplicaID]rac2.AdmittedVector
// Swap the updates map with the empty scratch. This is an optimization to
// minimize the time we hold the pendingAdmittedMu lock.
func() {
if updatesEmpty := func() bool {
p.leader.pendingAdmittedMu.Lock()
defer p.leader.pendingAdmittedMu.Unlock()
if updates = p.leader.pendingAdmittedMu.updates; len(updates) != 0 {
p.leader.pendingAdmittedMu.updates = p.leader.scratch
p.leader.scratch = updates
return false
}
}()
if len(updates) == 0 {
return true
}(); updatesEmpty {
return
}
for replicaID, state := range updates {
Expand Down

0 comments on commit 8b17cd7

Please sign in to comment.