From bfe3a3d08e0fc99838553d1aa3556e077d8ae3dc Mon Sep 17 00:00:00 2001
From: Anthony Romano <anthony.romano@coreos.com>
Date: Fri, 1 Apr 2016 13:44:06 -0700
Subject: [PATCH] wal: fix tail corruption

On ReadAll, WAL seeks to the end of the last record in the tail. If the tail did not
end with preallocated space, the decoder would report 0 as the last offset and begin
writing at offset 0 of the tail.

Fixes #4903
---
 wal/decoder.go  | 17 +++++--------
 wal/repair.go   |  3 ---
 wal/wal.go      |  4 ---
 wal/wal_test.go | 66 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 72 insertions(+), 18 deletions(-)

diff --git a/wal/decoder.go b/wal/decoder.go
index e8585097c..0678ab74f 100644
--- a/wal/decoder.go
+++ b/wal/decoder.go
@@ -60,17 +60,8 @@ func (d *decoder) decodeRecord(rec *walpb.Record) error {
 	}
 
 	l, err := readInt64(d.brs[0])
-	if err == io.EOF {
-		d.brs = d.brs[1:]
-		d.lastValidOff = 0
-		return d.decodeRecord(rec)
-	}
-
-	if err != nil {
-		return err
-	}
-	if l == 0 {
-		// hit preallocated space
+	if err == io.EOF || (err == nil && l == 0) {
+		// hit end of file or preallocated space
 		d.brs = d.brs[1:]
 		if len(d.brs) == 0 {
 			return io.EOF
@@ -78,6 +69,10 @@ func (d *decoder) decodeRecord(rec *walpb.Record) error {
 		d.lastValidOff = 0
 		return d.decodeRecord(rec)
 	}
+	if err != nil {
+		return err
+	}
+
 	data := make([]byte, l)
 	if _, err = io.ReadFull(d.brs[0], data); err != nil {
 		// ReadFull returns io.EOF only if no bytes were read
diff --git a/wal/repair.go b/wal/repair.go
index d1eb975ff..eceebff3c 100644
--- a/wal/repair.go
+++ b/wal/repair.go
@@ -55,9 +55,6 @@ func Repair(dirpath string) bool {
 			continue
 		case io.EOF:
 			return true
-		case ErrZeroTrailer:
-			plog.Noticef("found zero trailer in %v", f.Name())
-			fallthrough
 		case io.ErrUnexpectedEOF:
 			plog.Noticef("repairing %v", f.Name())
 			bf, bferr := os.Create(f.Name() + ".broken")
diff --git a/wal/wal.go b/wal/wal.go
index 70c81e6ea..3e5db7232 100644
--- a/wal/wal.go
+++ b/wal/wal.go
@@ -57,7 +57,6 @@ var (
 	ErrCRCMismatch      = errors.New("wal: crc mismatch")
 	ErrSnapshotMismatch = errors.New("wal: snapshot mismatch")
 	ErrSnapshotNotFound = errors.New("wal: snapshot not found")
-	ErrZeroTrailer      = errors.New("wal: zero trailer")
 	crcTable            = crc32.MakeTable(crc32.Castagnoli)
 )
 
@@ -273,9 +272,6 @@ func (w *WAL) ReadAll() (metadata []byte, state raftpb.HardState, ents []raftpb.
 		}
 	}
 
-	if err == ErrZeroTrailer {
-		err = io.EOF
-	}
 	switch w.tail() {
 	case nil:
 		// We do not have to read out all entries in read mode.
diff --git a/wal/wal_test.go b/wal/wal_test.go
index 6bf4cb928..026babcf6 100644
--- a/wal/wal_test.go
+++ b/wal/wal_test.go
@@ -533,3 +533,69 @@ func TestReleaseLockTo(t *testing.T) {
 		t.Errorf("lockindex = %d, want %d", lockIndex, 10)
 	}
 }
+
+// TestTailWriteNoSlackSpace ensures that tail writes append if there's no preallocated space.
+func TestTailWriteNoSlackSpace(t *testing.T) {
+	p, err := ioutil.TempDir(os.TempDir(), "waltest")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(p)
+
+	// create initial WAL
+	w, err := Create(p, []byte("metadata"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	// write some entries
+	for i := 1; i <= 5; i++ {
+		es := []raftpb.Entry{{Index: uint64(i), Term: 1, Data: []byte{byte(i)}}}
+		if err = w.Save(raftpb.HardState{Term: 1}, es); err != nil {
+			t.Fatal(err)
+		}
+	}
+	// get rid of slack space by truncating file
+	off, serr := w.tail().Seek(0, os.SEEK_CUR)
+	if serr != nil {
+		t.Fatal(serr)
+	}
+	if terr := w.tail().Truncate(off); terr != nil {
+		t.Fatal(terr)
+	}
+	w.Close()
+
+	// open, write more
+	w, err = Open(p, walpb.Snapshot{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, _, ents, rerr := w.ReadAll()
+	if rerr != nil {
+		t.Fatal(rerr)
+	}
+	if len(ents) != 5 {
+		t.Fatalf("got entries %+v, expected 5 entries", ents)
+	}
+	// write more entries
+	for i := 6; i <= 10; i++ {
+		es := []raftpb.Entry{{Index: uint64(i), Term: 1, Data: []byte{byte(i)}}}
+		if err = w.Save(raftpb.HardState{Term: 1}, es); err != nil {
+			t.Fatal(err)
+		}
+	}
+	w.Close()
+
+	// confirm all writes
+	w, err = Open(p, walpb.Snapshot{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, _, ents, rerr = w.ReadAll()
+	if rerr != nil {
+		t.Fatal(rerr)
+	}
+	if len(ents) != 10 {
+		t.Fatalf("got entries %+v, expected 10 entries", ents)
+	}
+	w.Close()
+}