Browse Source

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.
Peter Mattis 7 years ago
parent
commit
66ee394527
2 changed files with 7 additions and 3 deletions
  1. 1 1
      raft/node.go
  2. 6 2
      raft/rawnode_test.go

+ 1 - 1
raft/node.go

@@ -575,7 +575,7 @@ func newReady(r *raft, prevSoftSt *SoftState, prevHardSt pb.HardState) Ready {
 	if len(r.readStates) != 0 {
 	if len(r.readStates) != 0 {
 		rd.ReadStates = r.readStates
 		rd.ReadStates = r.readStates
 	}
 	}
-	rd.MustSync = MustSync(rd.HardState, prevHardSt, len(rd.Entries))
+	rd.MustSync = MustSync(r.hardState(), prevHardSt, len(rd.Entries))
 	return rd
 	return rd
 }
 }
 
 

+ 6 - 2
raft/rawnode_test.go

@@ -57,6 +57,10 @@ func TestRawNodeProposeAndConfChange(t *testing.T) {
 	s.Append(rd.Entries)
 	s.Append(rd.Entries)
 	rawNode.Advance(rd)
 	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()
 	rawNode.Campaign()
 	proposed := false
 	proposed := false
 	var (
 	var (
@@ -329,7 +333,7 @@ func TestRawNodeRestart(t *testing.T) {
 		HardState: emptyState,
 		HardState: emptyState,
 		// commit up to commit index in st
 		// commit up to commit index in st
 		CommittedEntries: entries[:st.Commit],
 		CommittedEntries: entries[:st.Commit],
-		MustSync:         true,
+		MustSync:         false,
 	}
 	}
 
 
 	storage := NewMemoryStorage()
 	storage := NewMemoryStorage()
@@ -366,7 +370,7 @@ func TestRawNodeRestartFromSnapshot(t *testing.T) {
 		HardState: emptyState,
 		HardState: emptyState,
 		// commit up to commit index in st
 		// commit up to commit index in st
 		CommittedEntries: entries,
 		CommittedEntries: entries,
-		MustSync:         true,
+		MustSync:         false,
 	}
 	}
 
 
 	s := NewMemoryStorage()
 	s := NewMemoryStorage()