Merge pull request #24 from hashicorp/fix-append-after-restore

Fix a bug where the first log appended is not Idx=1

go.mod

@@ -4,19 +4,18 @@ go 1.18
 
 require (
 	github.com/HdrHistogram/hdrhistogram-go v1.1.0
-	github.com/armon/go-metrics v0.3.10
+	github.com/armon/go-metrics v0.4.1
 	github.com/benbjohnson/immutable v0.4.0
 	github.com/benmathews/bench v0.0.0-20210120214102-f7c75b9ef6e7
 	github.com/benmathews/hdrhistogram-writer v0.0.0-20210120211942-3cb1c7c33f95
 	github.com/coreos/etcd v3.3.27+incompatible
 	github.com/google/gofuzz v1.2.0
 	github.com/hashicorp/go-hclog v0.14.1
-	github.com/hashicorp/raft v1.3.10
+	github.com/hashicorp/raft v1.4.0
 	github.com/hashicorp/raft-boltdb v0.0.0-20220329195025-15018e9b97e0
 	github.com/hashicorp/raft-boltdb/v2 v2.2.2
-	github.com/ryboe/q v1.0.18
 	github.com/segmentio/fasthash v1.0.3
-	github.com/stretchr/testify v1.8.0
+	github.com/stretchr/testify v1.8.2
 	go.etcd.io/bbolt v1.3.6
 )
 
@@ -29,12 +28,9 @@ require (
 	github.com/hashicorp/go-immutable-radix v1.3.0 // indirect
 	github.com/hashicorp/go-msgpack v1.1.5 // indirect
 	github.com/hashicorp/golang-lru v0.5.4 // indirect
-	github.com/kr/pretty v0.3.1 // indirect
-	github.com/kr/text v0.2.0 // indirect
 	github.com/mattn/go-colorable v0.1.6 // indirect
 	github.com/mattn/go-isatty v0.0.12 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
-	github.com/rogpeppe/go-internal v1.9.0 // indirect
 	golang.org/x/exp v0.0.0-20220827204233-334a2380cb91 // indirect
 	golang.org/x/sys v0.0.0-20220728004956-3c1f35247d10 // indirect
 	golang.org/x/time v0.1.0 // indirect

go.sum

@@ -14,8 +14,8 @@ github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRF
 github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
 github.com/armon/go-metrics v0.0.0-20190430140413-ec5e00d3c878/go.mod h1:3AMJUQhVx52RsWOnlkpikZr01T/yAVN2gn0861vByNg=
 github.com/armon/go-metrics v0.3.8/go.mod h1:4O98XIr/9W0sxpJ8UaYkvjk10Iff7SnFrb4QAOwNTFc=
-github.com/armon/go-metrics v0.3.10 h1:FR+drcQStOe+32sYyJYyZ7FIdgoGGBnwLl+flodp8Uo=
-github.com/armon/go-metrics v0.3.10/go.mod h1:4O98XIr/9W0sxpJ8UaYkvjk10Iff7SnFrb4QAOwNTFc=
+github.com/armon/go-metrics v0.4.1 h1:hR91U9KYmb6bLBYLQjyM+3j+rcd/UhE+G78SFnF8gJA=
+github.com/armon/go-metrics v0.4.1/go.mod h1:E6amYzXo6aW1tqzoZGT755KkbgrJsSdpwZ+3JqfkOG4=
 github.com/benbjohnson/immutable v0.4.0 h1:CTqXbEerYso8YzVPxmWxh2gnoRQbbB9X1quUC8+vGZA=
 github.com/benbjohnson/immutable v0.4.0/go.mod h1:iAr8OjJGLnLmVUr9MZ/rz4PWUy6Ouc2JLYuMArmvAJM=
 github.com/benmathews/bench v0.0.0-20210120214102-f7c75b9ef6e7 h1:nYTgFk9sOL3rmNew6rR2anUWWCzmSYPMJiSmowV8Yls=
@@ -91,7 +91,6 @@ github.com/hashicorp/go-hclog v0.14.1/go.mod h1:whpDNt7SSdeAju8AWKIWsul05p54N/39
 github.com/hashicorp/go-immutable-radix v1.0.0/go.mod h1:0y9vanUI8NX6FsYoO3zeMjhV/C5i9g4Q3DwcSNZ4P60=
 github.com/hashicorp/go-immutable-radix v1.3.0 h1:8exGP7ego3OmkfksihtSouGMZ+hQrhxx+FVELeXpVPE=
 github.com/hashicorp/go-immutable-radix v1.3.0/go.mod h1:0y9vanUI8NX6FsYoO3zeMjhV/C5i9g4Q3DwcSNZ4P60=
-github.com/hashicorp/go-msgpack v0.5.5 h1:i9R9JSrqIz0QVLz3sz+i3YJdT7TTSLcfLLzJi9aZTuI=
 github.com/hashicorp/go-msgpack v0.5.5/go.mod h1:ahLV/dePpqEmjfWmKiqvPkv/twdG7iPBM1vqhUKIvfM=
 github.com/hashicorp/go-msgpack v1.1.5 h1:9byZdVjKTe5mce63pRVNP1L7UAmdHOTEMGehn6KvJWs=
 github.com/hashicorp/go-msgpack v1.1.5/go.mod h1:gWVc3sv/wbDmR3rQsj1CAktEZzoz1YNK9NfGLXJ69/4=
@@ -104,8 +103,8 @@ github.com/hashicorp/golang-lru v0.5.4 h1:YDjusn29QI/Das2iO9M0BHnIbxPeyuCHsjMW+l
 github.com/hashicorp/golang-lru v0.5.4/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4=
 github.com/hashicorp/raft v1.1.0/go.mod h1:4Ak7FSPnuvmb0GV6vgIAJ4vYT4bek9bb6Q+7HVbyzqM=
 github.com/hashicorp/raft v1.2.0/go.mod h1:vPAJM8Asw6u8LxC3eJCUZmRP/E4QmUGE1R7g7k8sG/8=
-github.com/hashicorp/raft v1.3.10 h1:LR5QZX1VQd0DFWZfeCwWawyeKfpS/Tm1yjnJIY5X4Tw=
-github.com/hashicorp/raft v1.3.10/go.mod h1:J8naEwc6XaaCfts7+28whSeRvCqTd6e20BlCU3LtEO4=
+github.com/hashicorp/raft v1.4.0 h1:tn28S/AWv0BtRQgwZv/1NELu8sCvI0FixqL8C8MYKeY=
+github.com/hashicorp/raft v1.4.0/go.mod h1:nz64BIjXphDLATfKGG5RzHtNUPioLeKFsXEm88yTVew=
 github.com/hashicorp/raft-boltdb v0.0.0-20171010151810-6e5ba93211ea/go.mod h1:pNv7Wc3ycL6F5oOWn+tPGo2gWD4a5X+yp/ntwdKLjRk=
 github.com/hashicorp/raft-boltdb v0.0.0-20210409134258-03c10cc3d4ea/go.mod h1:qRd6nFJYYS6Iqnc/8HcUmko2/2Gw8qTFEmxDLii6W5I=
 github.com/hashicorp/raft-boltdb v0.0.0-20220329195025-15018e9b97e0 h1:CO8dBMLH6dvE1jTn/30ZZw3iuPsNfajshWoJTnVc5cc=
@@ -124,8 +123,6 @@ github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxv
 github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc=
 github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
 github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
-github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
-github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
 github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
@@ -160,7 +157,6 @@ github.com/nsqio/go-nsq v1.0.8/go.mod h1:vKq36oyeVXgsS5Q8YEO7WghqidAVXQlcFxzQbQT
 github.com/pascaldekloe/goe v0.1.0 h1:cBOtyMzM9HTpWjXfbbunk26uA6nG3a8n06Wieeh0MwY=
 github.com/pascaldekloe/goe v0.1.0/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
 github.com/pierrec/lz4 v2.4.1+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi+IEE17M5jbnwPHcY=
-github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA=
 github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
@@ -181,10 +177,6 @@ github.com/prometheus/procfs v0.0.2/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsT
 github.com/prometheus/procfs v0.0.8/go.mod h1:7Qr8sr6344vo1JqZ6HhLceV9o3AJ1Ff+GxbHq6oeK9A=
 github.com/prometheus/procfs v0.2.0/go.mod h1:lV6e/gmhEcM9IjHGsFOCxxuZ+z1YqCvr4OA4YeYWdaU=
 github.com/rcrowley/go-metrics v0.0.0-20190826022208-cac0b30c2563/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4=
-github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8=
-github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
-github.com/ryboe/q v1.0.18 h1:uTonPt1eZjy7GSpB0XpYpsCvX+Yf9f+M4CUKuH2r+vg=
-github.com/ryboe/q v1.0.18/go.mod h1:elqvVf/GBuZHvZ9gvHv4MKM6NZAMz2rFajnTgQZ46wU=
 github.com/segmentio/fasthash v1.0.3 h1:EI9+KE1EwvMLBWwjpRDc+fEM+prwxDYbslddQGtrmhM=
 github.com/segmentio/fasthash v1.0.3/go.mod h1:waKX8l2N8yckOgmSsXJi7x1ZfdKZ4x7KRMzBtS3oedY=
 github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo=
@@ -193,14 +185,16 @@ github.com/streadway/amqp v0.0.0-20200108173154-1c71cc93ed71/go.mod h1:AZpEONHx3
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
+github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
-github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk=
 github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
+github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8=
+github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
 github.com/tv42/httpunix v0.0.0-20150427012821-b75d8614f926/go.mod h1:9ESjWnEqriFuLhtthL60Sar/7RFoluCcXsuvEwTV5KM=
 github.com/xdg/scram v0.0.0-20180814205039-7eeb5667e42c/go.mod h1:lB8K/P019DLNhemzwFU4jHLhdvlE6uDZjXFejJXr49I=
 github.com/xdg/stringprep v1.0.0/go.mod h1:Jhud4/sHMO4oL310DaZAKk9ZaJ08SJfe+sJh0HrGL1Y=

segment/filer_test.go

@@ -37,6 +37,9 @@ func TestSegmentBasics(t *testing.T) {
 	// being created and written to).
 	require.False(t, w.(*Writer).wf.(*testWritableFile).dirty)
 
+	// Try to write a log that is not the base index
+	err = w.Append([]types.LogEntry{{Index: 2, Data: []byte("two")}})
+	require.ErrorContains(t, err, "non-monotonic append to segment with BaseIndex=1. Entry index 2, expected 1")
 	// Append to writer
 	err = w.Append([]types.LogEntry{{Index: 1, Data: []byte("one")}})
 	require.NoError(t, err)
@@ -57,6 +60,10 @@ func TestSegmentBasics(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, []byte("one"), got.Bs)
 
+	// Try to write a log that is not the expected next index (which would be 2)
+	err = w.Append([]types.LogEntry{{Index: 10, Data: []byte("ten")}})
+	require.ErrorContains(t, err, "non-monotonic append to segment with BaseIndex=1. Entry index 10, expected 2")
+
 	expectVals := append([]string{}, "one")
 	// OK, now write some more.
 	batch := make([]types.LogEntry, 0, 10)

segment/writer.go

@@ -363,19 +363,48 @@ func (w *Writer) OffsetForFrame(idx uint64) (uint32, error) {
 }
 
 func (w *Writer) appendEntry(e types.LogEntry) error {
+	offsets := w.getOffsets()
+
+	// Check the invariant that this entry is the next one we expect otherwise our
+	// index logic is incorrect and will result in panics on read.
+	if e.Index != w.info.BaseIndex+uint64(len(offsets)) {
+		return fmt.Errorf("non-monotonic append to segment with BaseIndex=%d. Entry index %d, expected %d",
+			w.info.BaseIndex, e.Index, w.info.BaseIndex+uint64(len(offsets)))
+	}
+
 	fh := frameHeader{
 		typ: FrameEntry,
 		len: uint32(len(e.Data)),
 	}
-	return w.appendFrame(fh, e.Data)
+	bufOffset, err := w.appendFrame(fh, e.Data)
+	if err != nil {
+		return err
+	}
+	// Update the offsets index
+
+	// Add the index entry. Note this is safe despite mutating the same backing
+	// array as tail because it's beyond the limit current readers will access
+	// until we do the atomic update below. Even if append re-allocates the
+	// backing array, it will only read the indexes smaller than numEntries from
+	// the old array to copy them into the new one and we are not mutating the
+	// same memory locations. Old readers might still be looking at the old
+	// array (lower than numEntries) through the current tail.offsets slice but
+	// we are not touching that at least below numEntries.
+	offsets = append(offsets, w.writer.writeOffset+uint32(bufOffset))
+
+	// Now we can make it available to readers. Note that readers still
+	// shouldn't read it until we actually commit to disk (and increment
+	// commitIdx) but it's race free for them to now!
+	w.offsets.Store(offsets)
+	return nil
 }
 
 func (w *Writer) appendCommit() error {
 	fh := frameHeader{
 		typ: FrameCommit,
 		crc: w.writer.crc,
 	}
-	if err := w.appendFrame(fh, nil); err != nil {
+	if _, err := w.appendFrame(fh, nil); err != nil {
 		return err
 	}
 
@@ -430,41 +459,21 @@ func (w *Writer) appendIndex() error {
 
 // appendFrame appends the given frame to the current block. The frame must fit
 // already otherwise an error will be returned.
-func (w *Writer) appendFrame(fh frameHeader, data []byte) error {
+func (w *Writer) appendFrame(fh frameHeader, data []byte) (int, error) {
 	// Encode frame header into current block buffer
 	l := encodedFrameSize(len(data))
 	w.ensureBufCap(l)
 
 	bufOffset := len(w.writer.commitBuf)
 	if err := writeFrame(w.writer.commitBuf[bufOffset:bufOffset+l], fh, data); err != nil {
-		return err
+		return 0, err
 	}
 	// Update len of commitBuf since we resliced it for the write
 	w.writer.commitBuf = w.writer.commitBuf[:bufOffset+l]
 
 	// Update the CRC
 	w.writer.crc = crc32.Update(w.writer.crc, castagnoliTable, w.writer.commitBuf[bufOffset:bufOffset+l])
-
-	// If frame is an entry, update the index
-	if fh.typ == FrameEntry {
-		offsets := w.getOffsets()
-
-		// Add the index entry. Note this is safe despite mutating the same backing
-		// array as tail because it's beyond the limit current readers will access
-		// until we do the atomic update below. Even if append re-allocates the
-		// backing array, it will only read the indexes smaller than numEntries from
-		// the old array to copy them into the new one and we are not mutating the
-		// same memory locations. Old readers might still be looking at the old
-		// array (lower than numEntries) through the current tail.offsets slice but
-		// we are not touching that at least below numEntries.
-		offsets = append(offsets, w.writer.writeOffset+uint32(bufOffset))
-
-		// Now we can make it available to readers. Note that readers still
-		// shouldn't read it until we actually commit to disk (and increment
-		// commitIdx) but it's race free for them to now!
-		w.offsets.Store(offsets)
-	}
-	return nil
+	return bufOffset, nil
 }
 
 func (w *Writer) flush() error {

state.go

@@ -28,6 +28,10 @@ type state struct {
 	finalizer atomic.Value // func()
 
 	nextSegmentID uint64
+
+	// nextBaseIndex is used to signal which baseIndex to use next if there are no
+	// segments or current tail.
+	nextBaseIndex uint64
 	segments      *immutable.SortedMap[uint64, segmentState]
 	tail          types.SegmentWriter
 }
@@ -151,8 +155,23 @@ func (s *state) lastIndex() uint64 {
 	if tailIdx > 0 {
 		return tailIdx
 	}
-	// Current tail is empty, so the largest log is the MaxIndex of the previous
-	// segment which must be the same as the tail's BaseIndex - 1 or zero.
+	// Current tail is empty. Check there are previous sealed segments.
+	it := s.segments.Iterator()
+	it.Last()
+	_, _, ok := it.Prev()
+	if !ok {
+		// No tail! shouldn't be possible but means no logs yet
+		return 0
+	}
+	// Go back to the segment before the tail
+	_, _, ok = it.Prev()
+	if !ok {
+		// No previous segment so the whole log is empty
+		return 0
+	}
+
+	// There was a previous segment so it's MaxIndex will be one less than the
+	// tail's BaseIndex.
 	tailSeg := s.getTailInfo()
 	if tailSeg == nil || tailSeg.BaseIndex == 0 {
 		return 0

types/types.go

@@ -3,10 +3,18 @@
 
 package types
 
-import "errors"
+import (
+	"errors"
+
+	"github.com/hashicorp/raft"
+)
 
 var (
-	ErrNotFound = errors.New("log entry not found")
+	// ErrNotFound is our own version of raft's not found error. It's important
+	// it's exactly the same because the raft lib checks for equality with it's
+	// own type as a crucial part of replication processing (detecting end of logs
+	// and that a snapshot is needed for a follower).
+	ErrNotFound = raft.ErrLogNotFound
 	ErrCorrupt  = errors.New("WAL is corrupt")
 	ErrSealed   = errors.New("segment is sealed")
 	ErrClosed   = errors.New("closed")

verifier/store.go

@@ -15,6 +15,9 @@ import (
 	"github.com/hashicorp/raft-wal/metrics"
 )
 
+var _ raft.LogStore = &LogStore{}
+var _ raft.MonotonicLogStore = &LogStore{}
+
 // LogStore is a raft.LogStore that acts as middleware around an underlying
 // persistent store. It provides support for periodically verifying that ranges
 // of logs read back from the LogStore match the values written, and the values
@@ -239,3 +242,12 @@ func (s *LogStore) Close() error {
 	}
 	return nil
 }
+
+// IsMonotonic implements the raft.MonotonicLogStore interface. This is a shim
+// to expose the underlying store as monotonically indexed or not.
+func (s *LogStore) IsMonotonic() bool {
+	if store, ok := s.s.(raft.MonotonicLogStore); ok {
+		return store.IsMonotonic()
+	}
+	return false
+}