Merge pull request #1082 from rqlite/clean-up-backup

Tighten up backup code

CHANGELOG.md

@@ -2,6 +2,7 @@
 ### Implementation changes and bug fixes
 - [PR #1079](https://github.com/rqlite/rqlite/pull/1079): Use a Protobuf model for Backup requests.
 - [PR #1078](https://github.com/rqlite/rqlite/pull/1078): Decrease bootstrap polling interval from 5 seconds to 2 seconds.
+- [PR #1082](https://github.com/rqlite/rqlite/pull/1082): Small refactor of backup code.
 
 ## 7.7.2 (October 14th 2022)
 ### Implementation changes and bug fixes

db/db.go

@@ -674,8 +674,8 @@ func (db *DB) Copy(dstDB *DB) error {
 // an ordinary on-disk database file, the serialization is just a copy of the
 // disk file. For an in-memory database or a "TEMP" database, the serialization
 // is the same sequence of bytes which would be written to disk if that database
-// were backed up to disk. This function must not be called while any transaction
+// were backed up to disk. This function must not be called while any writes
-// is in progress.
+// are happening to the database.
 func (db *DB) Serialize() ([]byte, error) {
 	if !db.memory {
 		// Simply read and return the SQLite file.

store/store.go

@@ -810,26 +810,47 @@ func (s *Store) Query(qr *command.QueryRequest) ([]*command.QueryRows, error) {
 // Backup writes a snapshot of the underlying database to dst
 //
 // If Leader is true for the request, this operation is performed with a read consistency
-// level equivalent to "weak". Otherwise, no guarantees are made about the read consistency level.
+// level equivalent to "weak". Otherwise, no guarantees are made about the read consistency
-func (s *Store) Backup(br *command.BackupRequest, dst io.Writer) error {
+// level. This function is safe to call while the database is being changed.
+func (s *Store) Backup(br *command.BackupRequest, dst io.Writer) (retErr error) {
+	startT := time.Now()
+	defer func() {
+		if retErr == nil {
+			stats.Add(numBackups, 1)
+			s.logger.Printf("database backed up in %s", time.Since(startT))
+		}
+	}()
+
 	if br.Leader && s.raft.State() != raft.Leader {
 		return ErrNotLeader
 	}
 
 	if br.Format == command.BackupRequest_BACKUP_REQUEST_FORMAT_BINARY {
-		if err := s.database(br.Leader, dst); err != nil {
+		f, err := ioutil.TempFile("", "rqlilte-snap-")
+		if err != nil {
 			return err
 		}
-	} else if br.Format == command.BackupRequest_BACKUP_REQUEST_FORMAT_SQL {
+		if err := f.Close(); err != nil {
-		if err := s.db.Dump(dst); err != nil {
 			return err
 		}
-	} else {
+		defer os.Remove(f.Name())
-		return ErrInvalidBackupFormat
+
+		if err := s.db.Backup(f.Name()); err != nil {
+			return err
 		}
 
-	stats.Add(numBackups, 1)
+		of, err := os.Open(f.Name())
-	return nil
+		if err != nil {
+			return err
+		}
+		defer of.Close()
+
+		_, err = io.Copy(dst, of)
+		return err
+	} else if br.Format == command.BackupRequest_BACKUP_REQUEST_FORMAT_SQL {
+		return s.db.Dump(dst)
+	}
+	return ErrInvalidBackupFormat
 }
 
 // Loads an entire SQLite file into the database, sending the request
@@ -1151,7 +1172,7 @@ func (s *Store) Apply(l *raft.Log) (e interface{}) {
 // about the read consistency level.
 //
 // http://sqlite.org/howtocorrupt.html states it is safe to do this
-// as long as no transaction is in progress.
+// as long as the database is not written to during the call.
 func (s *Store) Database(leader bool) ([]byte, error) {
 	if leader && s.raft.State() != raft.Leader {
 		return nil, ErrNotLeader
@@ -1168,7 +1189,7 @@ func (s *Store) Database(leader bool) ([]byte, error) {
 // However, queries that involve a transaction must be blocked.
 //
 // http://sqlite.org/howtocorrupt.html states it is safe to copy or serialize the
-// database as long as no transaction is in progress.
+// database as long as no writes to the database are in progress.
 func (s *Store) Snapshot() (raft.FSMSnapshot, error) {
 	defer func() {
 		s.numSnapshotsMu.Lock()
@@ -1290,34 +1311,6 @@ func (s *Store) logSize() (int64, error) {
 	return fi.Size(), nil
 }
 
-// Database copies contents of the underlying SQLite database to dst
-func (s *Store) database(leader bool, dst io.Writer) error {
-	if leader && s.raft.State() != raft.Leader {
-		return ErrNotLeader
-	}
-
-	f, err := ioutil.TempFile("", "rqlilte-snap-")
-	if err != nil {
-		return err
-	}
-	if err := f.Close(); err != nil {
-		return err
-	}
-
-	if err := s.db.Backup(f.Name()); err != nil {
-		return err
-	}
-
-	of, err := os.Open(f.Name())
-	if err != nil {
-		return err
-	}
-	defer of.Close()
-
-	_, err = io.Copy(dst, of)
-	return err
-}
-
 func (s *Store) databaseTypePretty() string {
 	if s.dbConf.Memory {
 		return "in-memory"

store/store_test.go

@@ -474,7 +474,7 @@ COMMIT;
 
 	f, err := ioutil.TempFile("", "rqlite-baktest-")
 	defer os.Remove(f.Name())
-	s.logger.Printf("backup file is %s", f.Name())
+	t.Logf("backup file is %s", f.Name())
 
 	if err := s.Backup(backupRequestBinary(true), f); err != nil {
 		t.Fatalf("Backup failed %s", err.Error())