From 66ee394527ed22d1155c74bd95ffa2d861b334d4 Mon Sep 17 00:00:00 2001 From: Peter Mattis Date: Tue, 18 Sep 2018 23:17:51 -0400 Subject: [PATCH] raft: fix Ready.MustSync logic The previous logic was erroneously setting Ready.MustSync to true when the hard state had not changed because we were comparing an empty hard state to the previous hard state. In combination with another misfeature in CockroachDB (unnecessary writing of empty batches), this was causing a steady stream of synchronous writes to disk. --- raft/node.go | 2 +- raft/rawnode_test.go | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/raft/node.go b/raft/node.go index 7c5f329e4..f9053da09 100644 --- a/raft/node.go +++ b/raft/node.go @@ -575,7 +575,7 @@ func newReady(r *raft, prevSoftSt *SoftState, prevHardSt pb.HardState) Ready { if len(r.readStates) != 0 { rd.ReadStates = r.readStates } - rd.MustSync = MustSync(rd.HardState, prevHardSt, len(rd.Entries)) + rd.MustSync = MustSync(r.hardState(), prevHardSt, len(rd.Entries)) return rd } diff --git a/raft/rawnode_test.go b/raft/rawnode_test.go index 8b8ccf5d3..4c794a763 100644 --- a/raft/rawnode_test.go +++ b/raft/rawnode_test.go @@ -57,6 +57,10 @@ func TestRawNodeProposeAndConfChange(t *testing.T) { s.Append(rd.Entries) rawNode.Advance(rd) + if d := rawNode.Ready(); d.MustSync || !IsEmptyHardState(d.HardState) || len(d.Entries) > 0 { + t.Fatalf("expected empty hard state with must-sync=false: %#v", d) + } + rawNode.Campaign() proposed := false var ( @@ -329,7 +333,7 @@ func TestRawNodeRestart(t *testing.T) { HardState: emptyState, // commit up to commit index in st CommittedEntries: entries[:st.Commit], - MustSync: true, + MustSync: false, } storage := NewMemoryStorage() @@ -366,7 +370,7 @@ func TestRawNodeRestartFromSnapshot(t *testing.T) { HardState: emptyState, // commit up to commit index in st CommittedEntries: entries, - MustSync: true, + MustSync: false, } s := NewMemoryStorage()