Merge pull request #6371 from useblacksmith/fix-history-db-corruption

Add corruption recovery to history.db

Author: tonistiigi

cmd/buildkitd/main.go

@@ -828,7 +828,7 @@ func newController(ctx context.Context, c *cli.Context, cfg *config.Config) (*co
 	}
 	cacheStoreForDebug = cacheStorage
 
-	historyDB, err := boltutil.Open(filepath.Join(cfg.Root, "history.db"), 0600, nil)
+	historyDB, err := boltutil.SafeOpen(filepath.Join(cfg.Root, "history.db"), 0600, nil)
 	if err != nil {
 		return nil, err
 	}

solver/bboltcachestorage/storage.go

@@ -4,11 +4,8 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"os"
 
-	"github.com/moby/buildkit/identity"
 	"github.com/moby/buildkit/solver"
-	"github.com/moby/buildkit/util/bklog"
 	"github.com/moby/buildkit/util/db"
 	"github.com/moby/buildkit/util/db/boltutil"
 	digest "github.com/opencontainers/go-digest"
@@ -28,7 +25,7 @@ type Store struct {
 }
 
 func NewStore(dbPath string) (*Store, error) {
-	db, err := safeOpenDB(dbPath, &bolt.Options{
+	db, err := boltutil.SafeOpen(dbPath, 0600, &bolt.Options{
 		NoSync: true,
 	})
 	if err != nil {
@@ -517,51 +514,3 @@ func isEmptyBucket(b *bolt.Bucket) bool {
 	k, _ := b.Cursor().First()
 	return k == nil
 }
-
-// safeOpenDB opens a bolt database and recovers from panic that
-// can be caused by a corrupted database file.
-func safeOpenDB(dbPath string, opts *bolt.Options) (db db.DB, err error) {
-	defer func() {
-		if r := recover(); r != nil {
-			err = errors.Errorf("%v", r)
-		}
-
-		// If we get an error when opening the database, but we have
-		// access to the file and the file looks like it has content,
-		// then fallback to resetting the database since the database
-		// may be corrupt.
-		if err != nil && fileHasContent(dbPath) {
-			db, err = fallbackOpenDB(dbPath, opts, err)
-		}
-	}()
-	return openDB(dbPath, opts)
-}
-
-// fallbackOpenDB performs database recovery and opens the new database
-// file when the database fails to open. Called after the first database
-// open fails.
-func fallbackOpenDB(dbPath string, opts *bolt.Options, openErr error) (db.DB, error) {
-	backupPath := dbPath + "." + identity.NewID() + ".bak"
-	bklog.L.Errorf("failed to open database file %s, resetting to empty. Old database is backed up to %s. "+
-		"This error signifies that buildkitd likely crashed or was sigkilled abrubtly, leaving the database corrupted. "+
-		"If you see logs from a previous panic then please report in the issue tracker at https://github.com/moby/buildkit . %+v", dbPath, backupPath, openErr)
-	if err := os.Rename(dbPath, backupPath); err != nil {
-		return nil, errors.Wrapf(err, "failed to rename database file %s to %s", dbPath, backupPath)
-	}
-
-	// Attempt to open the database again. This should be a new database.
-	// If this fails, it is a permanent error.
-	return openDB(dbPath, opts)
-}
-
-// openDB opens a bolt database in user-only read/write mode.
-func openDB(dbPath string, opts *bolt.Options) (db.DB, error) {
-	return boltutil.Open(dbPath, 0600, opts)
-}
-
-// fileHasContent checks if we have access to the file with appropriate
-// permissions and the file has a non-zero size.
-func fileHasContent(dbPath string) bool {
-	st, err := os.Stat(dbPath)
-	return err == nil && st.Size() > 0
-}

util/db/boltutil/safe_open.go

@@ -0,0 +1,56 @@
+package boltutil
+
+import (
+	"os"
+
+	"github.com/moby/buildkit/identity"
+	"github.com/moby/buildkit/util/bklog"
+	"github.com/moby/buildkit/util/db"
+	"github.com/pkg/errors"
+	bolt "go.etcd.io/bbolt"
+)
+
+// SafeOpen opens a bolt database with automatic recovery from corruption.
+// If the database file is corrupted, it backs up the corrupted file and creates
+// a new empty database. This is useful for disposable databases like cache or
+// history where data loss is acceptable but startup failure is not.
+func SafeOpen(dbPath string, mode os.FileMode, opts *bolt.Options) (db db.DB, err error) {
+	defer func() {
+		if r := recover(); r != nil {
+			err = errors.Errorf("%v", r)
+		}
+
+		// If we get an error when opening the database, but we have
+		// access to the file and the file looks like it has content,
+		// then fallback to resetting the database since the database
+		// may be corrupt.
+		if err != nil && fileHasContent(dbPath) {
+			db, err = fallbackOpen(dbPath, mode, opts, err)
+		}
+	}()
+	return Open(dbPath, mode, opts)
+}
+
+// fallbackOpen performs database recovery and opens a new database
+// file when the database fails to open. Called after the first database
+// open fails.
+func fallbackOpen(dbPath string, mode os.FileMode, opts *bolt.Options, openErr error) (db.DB, error) {
+	backupPath := dbPath + "." + identity.NewID() + ".bak"
+	bklog.L.Errorf("failed to open database file %s, resetting to empty. Old database is backed up to %s. "+
+		"This error signifies that buildkitd likely crashed or was sigkilled abruptly, leaving the database corrupted. "+
+		"If you see logs from a previous panic then please report in the issue tracker at https://github.com/moby/buildkit . %+v", dbPath, backupPath, openErr)
+	if err := os.Rename(dbPath, backupPath); err != nil {
+		return nil, errors.Wrapf(err, "failed to rename database file %s to %s", dbPath, backupPath)
+	}
+
+	// Attempt to open the database again. This should be a new database.
+	// If this fails, it is a permanent error.
+	return Open(dbPath, mode, opts)
+}
+
+// fileHasContent checks if we have access to the file with appropriate
+// permissions and the file has a non-zero size.
+func fileHasContent(dbPath string) bool {
+	st, err := os.Stat(dbPath)
+	return err == nil && st.Size() > 0
+}