kvserver: hold required locks on destroy status set

Epic: none
Release note: none

Author: pav-kv

pkg/kv/kvserver/replica_application_state_machine_test.go

@@ -149,6 +149,8 @@ func TestReplicaStateMachineChangeReplicas(t *testing.T) {
 		// should there be a command in the raft log (i.e. some errant lease request
 		// or whatnot) this will fire assertions because it will conflict with the
 		// log index that we pulled out of thin air above.
+		r.readOnlyCmdMu.Lock()
+		defer r.readOnlyCmdMu.Unlock()
 		r.mu.Lock()
 		defer r.mu.Unlock()
 		r.mu.destroyStatus.Set(errors.New("test done"), destroyReasonRemoved)
@@ -231,14 +233,16 @@ func TestReplicaStateMachineRaftLogTruncationStronglyCoupled(t *testing.T) {
 		_, err = sm.ApplySideEffects(checkedCmd.Ctx(), checkedCmd)
 		require.NoError(t, err)
 		func() {
-			r.mu.Lock()
-			defer r.mu.Unlock()
 			// Set a destroyStatus to make sure there won't be any raft processing once
 			// we release raftMu. We applied a command but not one from the raft log, so
 			// should there be a command in the raft log (i.e. some errant lease request
 			// or whatnot) this will fire assertions because it will conflict with the
 			// log index that we pulled out of thin air above.
+			r.readOnlyCmdMu.Lock()
+			r.mu.Lock()
+			defer r.mu.Unlock()
 			r.mu.destroyStatus.Set(errors.New("test done"), destroyReasonRemoved)
+			r.readOnlyCmdMu.Unlock()
 
 			require.Equal(t, raftAppliedIndex+1, r.shMu.state.RaftAppliedIndex)
 			require.Equal(t, truncatedIndex+1, ls.shMu.trunc.Index)
@@ -427,7 +431,13 @@ func TestReplicaStateMachineEphemeralAppBatchRejection(t *testing.T) {
 	defer r.raftMu.Unlock()
 	// Avoid additional raft processing after we're done with this replica because
 	// we've applied entries that aren't in the log.
-	defer r.mu.destroyStatus.Set(errors.New("boom"), destroyReasonRemoved)
+	defer func() {
+		r.readOnlyCmdMu.Lock()
+		defer r.readOnlyCmdMu.Unlock()
+		r.mu.Lock()
+		defer r.mu.Unlock()
+		r.mu.destroyStatus.Set(errors.New("boom"), destroyReasonRemoved)
+	}()
 
 	sm := r.getStateMachine()
 

pkg/kv/kvserver/replica_command_test.go

@@ -677,9 +677,13 @@ func TestWaitForLeaseAppliedIndex(t *testing.T) {
 	stopper.Stop(ctx)
 
 	destroyErr := errors.New("destroyed")
-	tc.repl.mu.Lock()
-	tc.repl.mu.destroyStatus.Set(destroyErr, destroyReasonRemoved)
-	tc.repl.mu.Unlock()
+	func() {
+		tc.repl.readOnlyCmdMu.Lock()
+		defer tc.repl.readOnlyCmdMu.Unlock()
+		tc.repl.mu.Lock()
+		defer tc.repl.mu.Unlock()
+		tc.repl.mu.destroyStatus.Set(destroyErr, destroyReasonRemoved)
+	}()
 
 	_, err = tc.repl.WaitForLeaseAppliedIndex(ctx, maxLAI)
 	require.Error(t, err)

pkg/kv/kvserver/replica_test.go

@@ -13781,9 +13781,13 @@ func TestAdminScatterDestroyedReplica(t *testing.T) {
 	tc.Start(ctx, t, stopper)
 
 	errBoom := errors.New("boom")
-	tc.repl.mu.Lock()
-	tc.repl.mu.destroyStatus.Set(errBoom, destroyReasonMergePending)
-	tc.repl.mu.Unlock()
+	func() {
+		tc.repl.readOnlyCmdMu.Lock()
+		defer tc.repl.readOnlyCmdMu.Unlock()
+		tc.repl.mu.Lock()
+		defer tc.repl.mu.Unlock()
+		tc.repl.mu.destroyStatus.Set(errBoom, destroyReasonMergePending)
+	}()
 
 	desc := tc.repl.Desc()
 	resp, err := tc.repl.adminScatter(ctx, kvpb.AdminScatterRequest{