From 372323118c4bd46c14c9c5dffac53476fc8e5f8f Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Wed, 24 Jan 2024 09:50:02 +0000 Subject: [PATCH] raft: send commit index only if necessary This commit fixes one case of unnecessary MsgApp sends. The leader now checks that the follower's commit index is behind, and only then sends a commit index update. Signed-off-by: Pavel Kalinnikov --- raft.go | 7 +-- raft_paper_test.go | 2 +- testdata/confchange_v2_replace_leader.txt | 4 -- testdata/probe_and_replicate.txt | 60 ----------------------- 4 files changed, 3 insertions(+), 70 deletions(-) diff --git a/raft.go b/raft.go index 8a13fbf2..0f9dc7de 100644 --- a/raft.go +++ b/raft.go @@ -1519,11 +1519,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/raft_paper_test.go b/raft_paper_test.go index e54f7a5a..cf5fdab8 100644 --- a/raft_paper_test.go +++ b/raft_paper_test.go @@ -633,7 +633,7 @@ func TestFollowerCheckMsgApp(t *testing.T) { msgs := r.readMessages() wmsgs := []pb.Message{ - {From: 1, To: 2, Type: pb.MsgAppResp, Term: 2, Index: tt.windex, Reject: tt.wreject, RejectHint: tt.wrejectHint, LogTerm: tt.wlogterm}, + {From: 1, To: 2, Type: pb.MsgAppResp, Term: 2, Index: tt.windex, Commit: tt.wcommit, Reject: tt.wreject, RejectHint: tt.wrejectHint, LogTerm: tt.wlogterm}, } if !reflect.DeepEqual(msgs, wmsgs) { t.Errorf("#%d: msgs = %+v, want %+v", i, msgs, wmsgs) diff --git a/testdata/confchange_v2_replace_leader.txt b/testdata/confchange_v2_replace_leader.txt index 366ac969..26fca5c3 100644 --- a/testdata/confchange_v2_replace_leader.txt +++ b/testdata/confchange_v2_replace_leader.txt @@ -284,12 +284,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 @@ -301,7 +299,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: @@ -318,7 +315,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 76e1391b..05f17a13 100644 --- a/testdata/probe_and_replicate.txt +++ b/testdata/probe_and_replicate.txt @@ -513,18 +513,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 ---- @@ -579,18 +567,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 ---- @@ -674,18 +650,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 ---- @@ -741,18 +705,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 ---- @@ -816,15 +768,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