From c8cd72b11623a006dc3a9f50f6d740e72e89440d Mon Sep 17 00:00:00 2001
From: Pavel Kalinnikov <pavel@cockroachlabs.com>
Date: Wed, 24 Jan 2024 09:50:02 +0000
Subject: [PATCH] raft: send commit index only if necessary

Signed-off-by: Pavel Kalinnikov <pavel@cockroachlabs.com>
---
 raft.go                                   |  9 ++--
 testdata/confchange_v2_replace_leader.txt |  4 --
 testdata/probe_and_replicate.txt          | 60 -----------------------
 3 files changed, 3 insertions(+), 70 deletions(-)

diff --git a/raft.go b/raft.go
index 39791b48..5e62ab10 100644
--- a/raft.go
+++ b/raft.go
@@ -1465,8 +1465,8 @@ func stepLeader(r *raft, m pb.Message) error {
 				r.sendAppend(m.From)
 			}
 		} else {
-			pr.UpdateCommit(m.Commit)
 			oldPaused := pr.IsPaused()
+			pr.UpdateCommit(m.Commit)
 			// We want to update our tracking if the response updates our
 			// matched index or if the response can move a probing peer back
 			// into StateReplicate (see heartbeat_rep_recovers_from_probing.txt
@@ -1500,11 +1500,8 @@ func stepLeader(r *raft, m pb.Message) error {
 					// to respond to pending read index requests
 					releasePendingReadIndexMessages(r)
 					r.bcastAppend()
-				} else if oldPaused {
-					// If we were paused before, this node may be missing the
-					// latest commit index, so send it.
-					// TODO(pav-kv): remove this branch, and decide on sending the commit
-					// index update based on pr.Commit.
+				} else if oldPaused && r.id != m.From && pr.Commit < r.raftLog.committed {
+					// The node is potentially missing the latest commit index. Send it.
 					r.sendAppend(m.From)
 				}
 				// We've updated flow control information above, which may
diff --git a/testdata/confchange_v2_replace_leader.txt b/testdata/confchange_v2_replace_leader.txt
index 4e2bf5cb..4ddde33c 100644
--- a/testdata/confchange_v2_replace_leader.txt
+++ b/testdata/confchange_v2_replace_leader.txt
@@ -282,12 +282,10 @@ stabilize
   CommittedEntries:
   2/5 EntryNormal ""
   Messages:
-  4->1 MsgApp Term:2 Log:2/5 Commit:4
   4->1 MsgApp Term:2 Log:2/5 Commit:5
   4->2 MsgApp Term:2 Log:2/5 Commit:5
   4->3 MsgApp Term:2 Log:2/5 Commit:5
 > 1 receiving messages
-  4->1 MsgApp Term:2 Log:2/5 Commit:4
   4->1 MsgApp Term:2 Log:2/5 Commit:5
 > 2 receiving messages
   4->2 MsgApp Term:2 Log:2/5 Commit:5
@@ -299,7 +297,6 @@ stabilize
   CommittedEntries:
   2/5 EntryNormal ""
   Messages:
-  1->4 MsgAppResp Term:2 Log:0/5 Commit:4
   1->4 MsgAppResp Term:2 Log:0/5 Commit:5
 > 2 handling Ready
   Ready MustSync=false:
@@ -316,7 +313,6 @@ stabilize
   Messages:
   3->4 MsgAppResp Term:2 Log:0/5 Commit:5
 > 4 receiving messages
-  1->4 MsgAppResp Term:2 Log:0/5 Commit:4
   1->4 MsgAppResp Term:2 Log:0/5 Commit:5
   2->4 MsgAppResp Term:2 Log:0/5 Commit:5
   3->4 MsgAppResp Term:2 Log:0/5 Commit:5
diff --git a/testdata/probe_and_replicate.txt b/testdata/probe_and_replicate.txt
index 715417ed..c75c4625 100644
--- a/testdata/probe_and_replicate.txt
+++ b/testdata/probe_and_replicate.txt
@@ -507,18 +507,6 @@ stabilize 1 2
   2->1 MsgAppResp Term:8 Log:0/21 Commit:18
 > 1 receiving messages
   2->1 MsgAppResp Term:8 Log:0/21 Commit:18
-> 1 handling Ready
-  Ready MustSync=false:
-  Messages:
-  1->2 MsgApp Term:8 Log:8/21 Commit:18
-> 2 receiving messages
-  1->2 MsgApp Term:8 Log:8/21 Commit:18
-> 2 handling Ready
-  Ready MustSync=false:
-  Messages:
-  2->1 MsgAppResp Term:8 Log:0/21 Commit:18
-> 1 receiving messages
-  2->1 MsgAppResp Term:8 Log:0/21 Commit:18
 
 stabilize 1 3
 ----
@@ -557,18 +545,6 @@ stabilize 1 3
   3->1 MsgAppResp Term:8 Log:0/21 Commit:18
 > 1 receiving messages
   3->1 MsgAppResp Term:8 Log:0/21 Commit:18
-> 1 handling Ready
-  Ready MustSync=false:
-  Messages:
-  1->3 MsgApp Term:8 Log:8/21 Commit:18
-> 3 receiving messages
-  1->3 MsgApp Term:8 Log:8/21 Commit:18
-> 3 handling Ready
-  Ready MustSync=false:
-  Messages:
-  3->1 MsgAppResp Term:8 Log:0/21 Commit:18
-> 1 receiving messages
-  3->1 MsgAppResp Term:8 Log:0/21 Commit:18
 
 stabilize 1 4
 ----
@@ -644,18 +620,6 @@ stabilize 1 5
   5->1 MsgAppResp Term:8 Log:0/21 Commit:21
 > 1 receiving messages
   5->1 MsgAppResp Term:8 Log:0/21 Commit:21
-> 1 handling Ready
-  Ready MustSync=false:
-  Messages:
-  1->5 MsgApp Term:8 Log:8/21 Commit:21
-> 5 receiving messages
-  1->5 MsgApp Term:8 Log:8/21 Commit:21
-> 5 handling Ready
-  Ready MustSync=false:
-  Messages:
-  5->1 MsgAppResp Term:8 Log:0/21 Commit:21
-> 1 receiving messages
-  5->1 MsgAppResp Term:8 Log:0/21 Commit:21
 
 stabilize 1 6
 ----
@@ -697,18 +661,6 @@ stabilize 1 6
   6->1 MsgAppResp Term:8 Log:0/21 Commit:21
 > 1 receiving messages
   6->1 MsgAppResp Term:8 Log:0/21 Commit:21
-> 1 handling Ready
-  Ready MustSync=false:
-  Messages:
-  1->6 MsgApp Term:8 Log:8/21 Commit:21
-> 6 receiving messages
-  1->6 MsgApp Term:8 Log:8/21 Commit:21
-> 6 handling Ready
-  Ready MustSync=false:
-  Messages:
-  6->1 MsgAppResp Term:8 Log:0/21 Commit:21
-> 1 receiving messages
-  6->1 MsgAppResp Term:8 Log:0/21 Commit:21
 
 stabilize 1 7
 ----
@@ -754,15 +706,3 @@ stabilize 1 7
   7->1 MsgAppResp Term:8 Log:0/21 Commit:21
 > 1 receiving messages
   7->1 MsgAppResp Term:8 Log:0/21 Commit:21
-> 1 handling Ready
-  Ready MustSync=false:
-  Messages:
-  1->7 MsgApp Term:8 Log:8/21 Commit:21
-> 7 receiving messages
-  1->7 MsgApp Term:8 Log:8/21 Commit:21
-> 7 handling Ready
-  Ready MustSync=false:
-  Messages:
-  7->1 MsgAppResp Term:8 Log:0/21 Commit:21
-> 1 receiving messages
-  7->1 MsgAppResp Term:8 Log:0/21 Commit:21