Merge cockroachdb#137455

137455: rac2: ignore MsgApps when constructing RaftEvent in MsgAppPull mode r=pav-kv a=sumeerbhola

There can be stale MsgApps even when the replica is in StateReplicate, and these are not relevant and serve only to confuse the downstream code.

Fixes cockroachdb#136322

Epic: CRDB-37515

Release note: None

Co-authored-by: sumeerbhola <[email protected]>

pkg/kv/kvserver/kvflowcontrol/rac2/range_controller.go

@@ -368,7 +368,8 @@ type RaftEvent struct {
 	// Entries contains the log entries to be written to storage.
 	Entries []raftpb.Entry
 	// MsgApps to followers. Only populated on the leader, when operating in
-	// MsgAppPush mode. This is informational, for bookkeeping in the callee.
+	// MsgAppPush mode. This is informational, for bookkeeping in the callee,
+	// which only looks at MsgApps with non-empty Entries.
 	//
 	// These MsgApps can be for entries in Entries, or for earlier ones.
 	// Typically, the MsgApps are ordered by entry index, and are a sequence of
@@ -436,7 +437,13 @@ func RaftEventFromMsgStorageAppendAndMsgApps(
 		event.Snap = appendMsg.Snapshot
 		event.Entries = appendMsg.Entries
 	}
-	if len(outboundMsgs) == 0 {
+	if len(outboundMsgs) == 0 || mode == MsgAppPull {
+		// MsgAppPull mode can have MsgApps with entries under some cases: (a)
+		// when the replica is in StateProbe, (b) stale MsgApps queued up inside
+		// Raft from when the replica was in StateProbe, even though it is now in
+		// StateReplicate. We ignore those in the RaftEvent created for the
+		// RangeController. They will get sent, but that is not the concern of the
+		// RACv2 code.
 		return event
 	}
 	// Clear the slices, to reuse slice allocations.

pkg/kv/kvserver/kvflowcontrol/rac2/range_controller_test.go

@@ -1853,6 +1853,15 @@ func TestRaftEventFromMsgStorageAppendAndMsgAppsBasic(t *testing.T) {
 		})
 		require.Equal(t, []raftpb.Message{outboundMsgs[0], outboundMsgs[3], outboundMsgs[2]}, msgApps)
 		checkSnapAndMap(event)
+		// Outbound msgs contains MsgApps for followers, but they are ignored
+		// since in pull mode.
+		event = RaftEventFromMsgStorageAppendAndMsgApps(
+			MsgAppPull, 19, appendMsg, outboundMsgs, logSnap, msgAppScratch, infoMap)
+		require.Equal(t, uint64(10), event.Term)
+		require.Equal(t, appendMsg.Snapshot, event.Snap)
+		require.Equal(t, appendMsg.Entries, event.Entries)
+		require.Nil(t, event.MsgApps)
+		checkSnapAndMap(event)
 	}
 }