Skip to content

Commit

Permalink
kv: fix lock ordering in Replica.applySnapshot
Browse files Browse the repository at this point in the history
Fixes #60479.

ad78116 was a nice improvement that avoided a number of tricky questions
about exposing an inconsistent in-memory replica state during a
snapshot. However, it appears to have introduced a deadlock due to lock
ordering issues (see referenced issue).

This commit fixes that issue by locking the Store mutex before the
Replica mutex in `Replica.applySnapshot`'s combined critical section.

Release note: None
  • Loading branch information
nvanbenschoten committed Feb 11, 2021
1 parent b69a09d commit 2e71a2b
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion pkg/kv/kvserver/replica_raftstorage.go
Original file line number Diff line number Diff line change
Expand Up @@ -969,15 +969,18 @@ func (r *Replica) applySnapshot(
// consume the SSTs above, meaning that at this point the in-memory state lags
// the on-disk state.

r.mu.Lock()
r.store.mu.Lock()
r.mu.Lock()
if r.store.removePlaceholderLocked(ctx, r.RangeID) {
atomic.AddInt32(&r.store.counts.filledPlaceholders, 1)
}
r.setDescLockedRaftMuLocked(ctx, s.Desc)
if err := r.store.maybeMarkReplicaInitializedLockedReplLocked(ctx, r); err != nil {
log.Fatalf(ctx, "unable to mark replica initialized while applying snapshot: %+v", err)
}
// NOTE: even though we acquired the store mutex first (according to the
// lock ordering rules described on Store.mu), it is safe to drop it first
// without risking a lock-ordering deadlock.
r.store.mu.Unlock()

// Invoke the leasePostApply method to ensure we properly initialize the
Expand Down

0 comments on commit 2e71a2b

Please sign in to comment.