Handle snapshots for empty WALs

db/db.go

@@ -106,6 +106,7 @@ type PoolStats struct {
 }
 
 // IsValidSQLiteFile checks that the supplied path looks like a SQLite file.
+// A non-existent file is considered invalid.
 func IsValidSQLiteFile(path string) bool {
 	f, err := os.Open(path)
 	if err != nil {
@@ -122,13 +123,14 @@ func IsValidSQLiteFile(path string) bool {
 }
 
 // IsValidSQLiteData checks that the supplied data looks like a SQLite data.
-// See https://www.sqlite.org/fileformat.html
+// See https://www.sqlite.org/fileformat.html.
 func IsValidSQLiteData(b []byte) bool {
 	return len(b) > 13 && string(b[0:13]) == "SQLite format"
 }
 
 // IsValidSQLiteWALFile checks that the supplied path looks like a SQLite
-// WAL file. See https://www.sqlite.org/fileformat2.html#walformat
+// WAL file. See https://www.sqlite.org/fileformat2.html#walformat. A
+// non-existent file is considered invalid.
 func IsValidSQLiteWALFile(path string) bool {
 	f, err := os.Open(path)
 	if err != nil {

db/wal/compacting_scanner_test.go

@@ -7,6 +7,13 @@ import (
 	"testing"
 )
 
+func Test_CompactingScanner_Scan_Empty(t *testing.T) {
+	_, err := NewCompactingScanner(bytes.NewReader([]byte{}), true)
+	if err != io.EOF {
+		t.Fatal(err)
+	}
+}
+
 func Test_CompactingScanner_Scan(t *testing.T) {
 	b, err := os.ReadFile("testdata/wal-reader/ok/wal")
 	if err != nil {

snapshot/sink.go

@@ -77,7 +77,9 @@ func (s *Sink) Cancel() error {
 }
 
 // Close closes the sink, and finalizes creation of the snapshot. It is critical
-// that Close is called, or the snapshot will not be in place.
+// that Close is called, or the snapshot will not be in place. It is OK to call
+// Close without every calling Write. In that case the Snapshot will be finalized
+// as usual, but will effectively be the same as the previously created snapshot.
 func (s *Sink) Close() error {
 	if !s.opened {
 		return nil
@@ -132,28 +134,38 @@ func (s *Sink) processSnapshotData() (retErr error) {
 		return err
 	}
 
-	if db.IsValidSQLiteFile(s.dataFD.Name()) {
-		if err := os.Rename(s.dataFD.Name(), filepath.Join(s.str.Dir(), s.meta.ID+".db")); err != nil {
-			return err
-		}
-	} else if db.IsValidSQLiteWALFile(s.dataFD.Name()) {
-		if len(snapshots) == 0 {
-			// We are trying to create our first snapshot from a WAL file, which is invalid.
-			return fmt.Errorf("data for first snapshot is a WAL file")
-		} else {
-			// We have at least one previous snapshot. That means we should have a valid SQLite file
-			// for the previous snapshot.
+	if len(snapshots) == 0 && !db.IsValidSQLiteFile(s.dataFD.Name()) {
+		// We have no snapshots yet, so the incomding data must be a valid SQLite file.
+		return fmt.Errorf("data for first snapshot must be a valid SQLite file")
+	}
+
+	dataSz, err := fileSize(s.dataFD.Name())
+	if err != nil {
+		return err
+	}
+
+	// Writing zero data for a snapshot is acceptable, and indicates the snapshot
+	// is empty. This could happen if lots of entries were written to the Raft log,
+	// which would trigger a Raft snapshot, but those entries didn't actually change
+	// the database. Otherwise, the data must be a valid SQLite file or WAL file.
+	if dataSz != 0 {
+		if db.IsValidSQLiteFile(s.dataFD.Name()) {
+			if err := os.Rename(s.dataFD.Name(), filepath.Join(s.str.Dir(), s.meta.ID+".db")); err != nil {
+				return err
+			}
+		} else if db.IsValidSQLiteWALFile(s.dataFD.Name()) {
+			// With WAL data incoming, then we must have a valid SQLite file from the previous snapshot.
 			snapPrev := snapshots[len(snapshots)-1]
 			snapPrevDB := filepath.Join(s.str.Dir(), snapPrev.ID+".db")
 			if !db.IsValidSQLiteFile(snapPrevDB) {
 				return fmt.Errorf("previous snapshot data is not a SQLite file: %s", snapPrevDB)
 			}
+			if err := os.Rename(s.dataFD.Name(), filepath.Join(s.str.Dir(), s.meta.ID+".db-wal")); err != nil {
+				return err
+			}
+		} else {
+			return fmt.Errorf("invalid snapshot data file: %s", s.dataFD.Name())
 		}
-		if err := os.Rename(s.dataFD.Name(), filepath.Join(s.str.Dir(), s.meta.ID+".db-wal")); err != nil {
-			return err
-		}
-	} else {
-		return fmt.Errorf("invalid snapshot data file: %s", s.dataFD.Name())
 	}
 
 	// Indicate snapshot data been successfully persisted to disk by renaming
@@ -178,12 +190,15 @@ func (s *Sink) processSnapshotData() (retErr error) {
 		snapNewDB := filepath.Join(s.str.Dir(), snapNew.ID+".db")
 		snapNewWAL := filepath.Join(s.str.Dir(), snapNew.ID+".db-wal")
 
-		if db.IsValidSQLiteWALFile(snapNewWAL) {
-			// The most recent snapshot was created from a WAL file, so we need to replay
-			// that WAL file into the previous SQLite file.
+		if db.IsValidSQLiteWALFile(snapNewWAL) || dataSz == 0 {
+			// One of two things have happened. Either the snapshot data is empty, in which
+			// case we can just make the existing SQLite file the new snapshot, or the snapshot
+			// data is a valid WAL file, in which case we need to replay it into the existing
+			// SQLite file.
 			if err := os.Rename(snapPrevDB, snapNewDB); err != nil {
 				return err
 			}
+			// An open-close cycle checkpoints and removes any WAL file.
 			if err := openCloseDB(snapNewDB); err != nil {
 				return err
 			}
@@ -200,3 +215,11 @@ func (s *Sink) processSnapshotData() (retErr error) {
 func (s *Sink) writeMeta(dir string) error {
 	return writeMeta(dir, s.meta)
 }
+
+func fileSize(path string) (int64, error) {
+	stat, err := os.Stat(path)
+	if err != nil {
+		return 0, err
+	}
+	return stat.Size(), nil
+}

snapshot/sink_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/hashicorp/raft"
@@ -269,12 +270,16 @@ func Test_SinkCreateFullThenWALSnapshots(t *testing.T) {
 	createSnapshot("snap-3456", 5, 4, 3, "testdata/db-and-wals/wal-01")
 	createSnapshot("snap-4567", 6, 5, 4, "testdata/db-and-wals/wal-02")
 	createSnapshot("snap-5678", 7, 6, 5, "testdata/db-and-wals/wal-03")
+	createSnapshot("snap-9abc", 8, 7, 6, "testdata/db-and-wals/empty-file")
 
 	// Check the database state inside the Store.
 	dbPath, err := store.getDBPath()
 	if err != nil {
 		t.Fatalf("Failed to get DB path: %v", err)
 	}
+	if filepath.Base(dbPath) != "snap-9abc.db" {
+		t.Fatalf("Unexpected DB file name: %s", dbPath)
+	}
 	checkDB, err := db.Open(dbPath, false, true)
 	if err != nil {
 		t.Fatalf("failed to open database at %s: %s", dbPath, err)

snapshot/store.go

@@ -326,7 +326,6 @@ func (s *Store) getSnapshots() ([]*raft.SnapshotMeta, error) {
 }
 
 // getDBPath returns the path to the database file for the most recent snapshot.
-// It is mostly useful for testing.
 func (s *Store) getDBPath() (string, error) {
 	snapshots, err := s.getSnapshots()
 	if err != nil {