Skip to content

Commit cf25e7d

Browse files
committed
kvserver: rm unnecessary Replica.mu locking
The destroyStatus can be read with only raftMu held, because it is always locked when this field is written. Epic: none Release note: none
1 parent ea93ddd commit cf25e7d

File tree

1 file changed

+10
-12
lines changed

1 file changed

+10
-12
lines changed

pkg/kv/kvserver/store_create_replica.go

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -90,20 +90,16 @@ func (s *Store) tryGetReplica(
9090
if !found {
9191
return nil, nil
9292
}
93-
9493
repl.raftMu.Lock() // not unlocked on success
95-
repl.mu.RLock()
9694

9795
// The current replica is removed, go back around.
9896
if repl.shMu.destroyStatus.Removed() {
99-
repl.mu.RUnlock()
10097
repl.raftMu.Unlock()
10198
return nil, errRetry
10299
}
103100

104101
// Drop messages from replicas we know to be too old.
105-
if fromReplicaIsTooOldRLocked(repl, creatingReplica) {
106-
repl.mu.RUnlock()
102+
if fromReplicaIsTooOldRaftMuLocked(repl, creatingReplica) {
107103
repl.raftMu.Unlock()
108104
return nil, kvpb.NewReplicaTooOldError(creatingReplica.ReplicaID)
109105
}
@@ -115,7 +111,6 @@ func (s *Store) tryGetReplica(
115111
id.ReplicaID, repl)
116112
}
117113

118-
repl.mu.RUnlock()
119114
if err := s.removeReplicaRaftMuLocked(
120115
ctx, repl, id.ReplicaID, "superseded by newer Replica",
121116
); err != nil {
@@ -124,7 +119,6 @@ func (s *Store) tryGetReplica(
124119
repl.raftMu.Unlock()
125120
return nil, errRetry
126121
}
127-
defer repl.mu.RUnlock()
128122

129123
if repl.replicaID > id.ReplicaID {
130124
// The sender is behind and is sending to an old replica.
@@ -224,11 +218,15 @@ func (s *Store) tryGetOrCreateReplica(
224218
return repl, true, nil
225219
}
226220

227-
// fromReplicaIsTooOldRLocked returns true if the creatingReplica is deemed to
228-
// be a member of the range which has been removed.
229-
// Assumes toReplica.mu is locked for (at least) reading.
230-
func fromReplicaIsTooOldRLocked(toReplica *Replica, fromReplica *roachpb.ReplicaDescriptor) bool {
231-
toReplica.mu.AssertRHeld()
221+
// fromReplicaIsTooOldRaftMuLocked returns true if the creatingReplica is deemed
222+
// to be a member of the range which has been removed.
223+
//
224+
// Assumes toReplica.raftMu is locked. This could be relaxed to Replica.mu
225+
// locked for reads, but the only user of it holds raftMu.
226+
func fromReplicaIsTooOldRaftMuLocked(
227+
toReplica *Replica, fromReplica *roachpb.ReplicaDescriptor,
228+
) bool {
229+
toReplica.raftMu.AssertHeld()
232230
if fromReplica == nil {
233231
return false
234232
}

0 commit comments

Comments
 (0)