From 32824e053ce265dc15d5090d1a1eb2d1a81d9e04 Mon Sep 17 00:00:00 2001
From: Ben Darnell <bdarnell@squareup.com>
Date: Thu, 13 Nov 2014 14:57:01 -0500
Subject: [PATCH] raft: Only call stableTo when we have ready entries or a
 snapshot.

The first Ready after RestartNode (with no snapshot) will have no
unstable entries, so we don't have the correct prevLastUnstablei
when Advance is called. This would cause raftLog.unstable to move
backwards and previously-stable entries would be returned to
the application again.

This should have been caught by the "unexpected Ready" portion of
TestNodeRestart, but it went unnoticed because the Node's goroutine
takes some time to read from advancec and prepare the write to read to
readyc. Added a small (1ms) delay to all such tests to ensure that the
goroutine has time to enter its select wait.
---
 raft/node.go      | 8 +++++++-
 raft/node_test.go | 8 ++++----
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/raft/node.go b/raft/node.go
index 35750e733..db2b710f3 100644
--- a/raft/node.go
+++ b/raft/node.go
@@ -226,6 +226,7 @@ func (n *node) run(r *raft) {
 	var readyc chan Ready
 	var advancec chan struct{}
 	var prevLastUnstablei uint64
+	var havePrevLastUnstablei bool
 	var rd Ready
 
 	lead := None
@@ -294,6 +295,7 @@ func (n *node) run(r *raft) {
 			}
 			if len(rd.Entries) > 0 {
 				prevLastUnstablei = rd.Entries[len(rd.Entries)-1].Index
+				havePrevLastUnstablei = true
 			}
 			if !IsEmptyHardState(rd.HardState) {
 				prevHardSt = rd.HardState
@@ -302,6 +304,7 @@ func (n *node) run(r *raft) {
 				prevSnapi = rd.Snapshot.Index
 				if prevSnapi > prevLastUnstablei {
 					prevLastUnstablei = prevSnapi
+					havePrevLastUnstablei = true
 				}
 			}
 			r.msgs = nil
@@ -310,7 +313,10 @@ func (n *node) run(r *raft) {
 			if prevHardSt.Commit != 0 {
 				r.raftLog.appliedTo(prevHardSt.Commit)
 			}
-			r.raftLog.stableTo(prevLastUnstablei)
+			if havePrevLastUnstablei {
+				r.raftLog.stableTo(prevLastUnstablei)
+				havePrevLastUnstablei = false
+			}
 			advancec = nil
 		case <-n.stop:
 			close(n.done)
diff --git a/raft/node_test.go b/raft/node_test.go
index ea41cb9fe..77675a56c 100644
--- a/raft/node_test.go
+++ b/raft/node_test.go
@@ -336,7 +336,7 @@ func TestNodeStart(t *testing.T) {
 	select {
 	case rd := <-n.Ready():
 		t.Errorf("unexpected Ready: %+v", rd)
-	default:
+	case <-time.After(time.Millisecond):
 	}
 }
 
@@ -364,7 +364,7 @@ func TestNodeRestart(t *testing.T) {
 	select {
 	case rd := <-n.Ready():
 		t.Errorf("unexpected Ready: %+v", rd)
-	default:
+	case <-time.After(time.Millisecond):
 	}
 }
 
@@ -432,12 +432,12 @@ func TestNodeAdvance(t *testing.T) {
 	select {
 	case rd := <-n.Ready():
 		t.Fatalf("unexpected Ready before Advance: %+v", rd)
-	default:
+	case <-time.After(time.Millisecond):
 	}
 	n.Advance()
 	select {
 	case <-n.Ready():
-	default:
+	case <-time.After(time.Millisecond):
 		t.Errorf("expect Ready after Advance, but there is no Ready available")
 	}
 }