Merge #143736

143736: kvserver: move a couple of fields to shMu r=tbg a=pav-kv

This allows reading the fields outside `Replica.mu` sections.

Related to #140235

Co-authored-by: Pavel Kalinnikov <[email protected]>

Author: craig[bot]

pkg/kv/kvserver/replica.go

@@ -558,6 +558,14 @@ type Replica struct {
 		// log was checked for truncation or at the time of the last Raft log
 		// truncation.
 		raftLogLastCheckSize int64
+
+		// leaderID is the ID of the leader replica within the Raft group.
+		// NB: this is updated in a separate critical section from the Raft group,
+		// and can therefore briefly be out of sync with the Raft status.
+		leaderID roachpb.ReplicaID
+		// currentRACv2Mode is always in-sync with RawNode.
+		// MsgAppPull <=> LazyReplication.
+		currentRACv2Mode rac2.RaftMsgAppMode
 	}
 
 	mu struct {
@@ -859,10 +867,6 @@ type Replica struct {
 		// TODO(erikgrinaker): make this never be nil.
 		internalRaftGroup *raft.RawNode
 
-		// The ID of the leader replica within the Raft group. NB: this is updated
-		// in a separate critical section from the Raft group, and can therefore
-		// briefly be out of sync with the Raft status.
-		leaderID roachpb.ReplicaID
 		// The most recently added replica for the range and when it was added.
 		// Used to determine whether a replica is new enough that we shouldn't
 		// penalize it for being slightly behind. These field gets cleared out once
@@ -990,11 +994,6 @@ type Replica struct {
 		// implementation.
 		replicaFlowControlIntegration replicaFlowControlIntegration
 
-		// The currentRACv2Mode is always in-sync with RawNode.
-		// MsgAppPull <=> LazyReplication.
-		// Updated with both raftMu and mu held.
-		currentRACv2Mode rac2.RaftMsgAppMode
-
 		// raftTracer is used to trace raft messages that are sent with a
 		// tracing context.
 		raftTracer rafttrace.RaftTracer

pkg/kv/kvserver/replica_init.go

@@ -246,7 +246,7 @@ func newUninitializedReplicaWithoutRaftGroup(
 		store.cfg.Settings, store.stopper, r.AmbientContext, r, onTrip, onReset,
 	)
 	r.LeaderlessWatcher = NewLeaderlessWatcher(r)
-	r.mu.currentRACv2Mode = r.replicationAdmissionControlModeToUse(context.TODO())
+	r.shMu.currentRACv2Mode = r.replicationAdmissionControlModeToUse(context.TODO())
 	r.raftMu.flowControlLevel = kvflowcontrol.GetV2EnabledWhenLeaderLevel(
 		r.raftCtx, store.ClusterSettings(), store.TestingKnobs().FlowControlTestingKnobs)
 	if r.raftMu.flowControlLevel > kvflowcontrol.V2NotEnabledWhenLeader {
@@ -373,7 +373,7 @@ func (r *Replica) initRaftGroupRaftMuLockedReplicaMuLocked() error {
 		raftpb.PeerID(r.replicaID),
 		r.shMu.state.RaftAppliedIndex,
 		r.store.cfg,
-		r.mu.currentRACv2Mode == rac2.MsgAppPull,
+		r.shMu.currentRACv2Mode == rac2.MsgAppPull,
 		&raftLogger{ctx: ctx},
 		(*replicaRLockedStoreLiveness)(r),
 		r.store.raftMetrics,

pkg/kv/kvserver/replica_proposal_quota.go

@@ -145,8 +145,8 @@ func (r *Replica) updateProposalQuotaRaftMuLocked(
 	// need for a setting change callback, as handleRaftReady (the caller), will
 	// be called at least at the tick interval for a non-quiescent range.
 	shouldInitQuotaPool := false
-	if r.mu.leaderID != lastLeaderID {
-		if r.replicaID == r.mu.leaderID {
+	if r.shMu.leaderID != lastLeaderID {
+		if r.replicaID == r.shMu.leaderID {
 			r.mu.lastUpdateTimes = make(map[roachpb.ReplicaID]time.Time)
 			r.mu.lastUpdateTimes.updateOnBecomeLeader(r.shMu.state.Desc.Replicas().Descriptors(), now)
 			r.mu.replicaFlowControlIntegration.onBecameLeader(ctx)
@@ -176,10 +176,10 @@ func (r *Replica) updateProposalQuotaRaftMuLocked(
 			}
 			return
 		}
-	} else if r.replicaID == r.mu.leaderID && r.mu.proposalQuota == nil && enabled {
+	} else if r.replicaID == r.shMu.leaderID && r.mu.proposalQuota == nil && enabled {
 		r.mu.lastProposalAtTicks = r.mu.ticks // delay imminent quiescence
 		shouldInitQuotaPool = true
-	} else if r.replicaID != r.mu.leaderID {
+	} else if r.replicaID != r.shMu.leaderID {
 		// We're a follower and have been since the last update. Nothing to do.
 		return
 	}

pkg/kv/kvserver/replica_raft.go

@@ -651,7 +651,7 @@ func (r *Replica) ticksSinceLastMessageRLocked() int64 {
 // status where necessary.
 func (r *Replica) isRaftLeaderRLocked() bool {
 	// Defensively check replicaID != 0.
-	return r.replicaID != 0 && r.replicaID == r.mu.leaderID
+	return r.replicaID != 0 && r.replicaID == r.shMu.leaderID
 }
 
 var errRemoved = errors.New("replica removed")
@@ -955,10 +955,11 @@ func (r *Replica) handleRaftReadyRaftMuLocked(
 	var raftNodeBasicState replica_rac2.RaftNodeBasicState
 	var logSnapshot raft.LogSnapshot
 
-	r.mu.Lock()
-	rac2ModeForReady := r.mu.currentRACv2Mode
-	leaderID := r.mu.leaderID
+	rac2ModeForReady := r.shMu.currentRACv2Mode
+	leaderID := r.shMu.leaderID
 	lastLeaderID := leaderID
+
+	r.mu.Lock()
 	err := r.withRaftGroupLocked(func(raftGroup *raft.RawNode) (bool, error) {
 		r.deliverLocalRaftMsgsRaftMuLockedReplicaMuLocked(ctx, raftGroup)
 
@@ -976,7 +977,7 @@ func (r *Replica) handleRaftReadyRaftMuLocked(
 				// Ready, and switch to pull mode after that.
 				switchToPullModeAfterReady = true
 			}
-			r.mu.currentRACv2Mode = rac2ModeToUse
+			r.shMu.currentRACv2Mode = rac2ModeToUse
 		}
 		logSnapshot = raftGroup.LogSnapshot()
 		if hasReady = raftGroup.HasReady(); hasReady {
@@ -1256,11 +1257,11 @@ func (r *Replica) handleRaftReadyRaftMuLocked(
 	r.mu.Lock()
 	r.asLogStorage().updateStateRaftMuLockedMuLocked(state)
 	var becameLeader bool
-	if r.mu.leaderID != leaderID {
-		r.mu.leaderID = leaderID
+	if r.shMu.leaderID != leaderID {
+		r.shMu.leaderID = leaderID
 		// Clear the remote proposal set. Would have been nil already if not
 		// previously the leader.
-		becameLeader = r.mu.leaderID == r.replicaID
+		becameLeader = leaderID == r.replicaID
 	}
 	r.mu.Unlock()
 
@@ -1570,9 +1571,7 @@ func (r *Replica) processRACv2PiggybackedAdmitted(ctx context.Context) {
 func (r *Replica) processRACv2RangeController(ctx context.Context) {
 	r.raftMu.Lock()
 	defer r.raftMu.Unlock()
-	// Can read Replica.mu.currentRACv2Mode since updates require both raftMu
-	// and Replica.mu.
-	mode := r.mu.currentRACv2Mode
+	mode := r.shMu.currentRACv2Mode
 	var logSnapshot raft.LogSnapshot
 	if mode == rac2.MsgAppPull {
 		err := r.withRaftGroup(func(raftGroup *raft.RawNode) (bool, error) {
@@ -1585,7 +1584,7 @@ func (r *Replica) processRACv2RangeController(ctx context.Context) {
 		}
 	}
 	r.flowControlV2.ProcessSchedulerEventRaftMuLocked(
-		ctx, r.mu.currentRACv2Mode, logSnapshot)
+		ctx, r.shMu.currentRACv2Mode, logSnapshot)
 }
 
 // SendMsgApp implements rac2.MsgAppSender.

pkg/kv/kvserver/store_raft.go

@@ -852,7 +852,7 @@ func (s *Store) supportWithdrawnCallback(supportWithdrawnForStoreIDs map[roachpb
 			if !r.mu.asleep {
 				return false
 			}
-			leader, err := r.getReplicaDescriptorByIDRLocked(r.mu.leaderID, roachpb.ReplicaDescriptor{})
+			leader, err := r.getReplicaDescriptorByIDRLocked(r.shMu.leaderID, roachpb.ReplicaDescriptor{})
 			// If we found the replica's leader's store, and it doesn't match any of
 			// the stores in supportWithdrawnForStoreIDs, the replica shouldn't wake
 			// up. In all other cases, the replica should wake up.