Skip to content

Commit

Permalink
storage: acquire shared lock in sendRaftMessage, not exclusive lock
Browse files Browse the repository at this point in the history
This was expensive and unnecessary in order to accomplish what the code
wanted to accomplish. This reduces below Raft lock contention with above
Raft processing (which often holds a shared lock).

Release note: None
  • Loading branch information
nvanbenschoten committed Jun 26, 2019
1 parent faff108 commit a428a67
Showing 1 changed file with 10 additions and 7 deletions.
17 changes: 10 additions & 7 deletions pkg/storage/replica_raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,14 @@ func (r *Replica) tick(livenessMap IsLiveMap) (bool, error) {

r.maybeTransferRaftLeadershipLocked(ctx)

// For followers, we update lastUpdateTimes when we step a message from them
// into the local Raft group. The leader won't hit that path, so we update
// it whenever it ticks. In effect, this makes sure it always sees itself as
// alive.
if r.mu.replicaID == r.mu.leaderID {
r.mu.lastUpdateTimes.update(r.mu.replicaID, timeutil.Now())
}

r.mu.ticks++
r.mu.internalRaftGroup.Tick()

Expand Down Expand Up @@ -1034,19 +1042,14 @@ func (r *Replica) sendRaftMessages(ctx context.Context, messages []raftpb.Messag

// sendRaftMessage sends a Raft message.
func (r *Replica) sendRaftMessage(ctx context.Context, msg raftpb.Message) {
r.mu.Lock()
r.mu.RLock()
fromReplica, fromErr := r.getReplicaDescriptorByIDRLocked(roachpb.ReplicaID(msg.From), r.mu.lastToReplica)
toReplica, toErr := r.getReplicaDescriptorByIDRLocked(roachpb.ReplicaID(msg.To), r.mu.lastFromReplica)
var startKey roachpb.RKey
if msg.Type == raftpb.MsgHeartbeat {
if r.mu.replicaID == 0 {
log.Fatalf(ctx, "preemptive snapshot attempted to send a heartbeat: %+v", msg)
}
// For followers, we update lastUpdateTimes when we step a message from
// them into the local Raft group. The leader won't hit that path, so we
// update it whenever it sends a heartbeat. In effect, this makes sure
// it always sees itself as alive.
r.mu.lastUpdateTimes.update(r.mu.replicaID, timeutil.Now())
} else if msg.Type == raftpb.MsgApp && r.mu.internalRaftGroup != nil {
// When the follower is potentially an uninitialized replica waiting for
// a split trigger, send the replica's StartKey along. See the method
Expand All @@ -1064,7 +1067,7 @@ func (r *Replica) sendRaftMessage(ctx context.Context, msg raftpb.Message) {
}
})
}
r.mu.Unlock()
r.mu.RUnlock()

if fromErr != nil {
log.Warningf(ctx, "failed to look up sender replica %d in r%d while sending %s: %s",
Expand Down

0 comments on commit a428a67

Please sign in to comment.