From ef2f3abff749f9f152350ac775407986d7192870 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 20 Apr 2016 07:53:57 +1000 Subject: db: fix recovery from unsynced metadata Bolt stores the two latest transactions' metadata, but previously did not recover from validation failures in the latest by using the second latest. Fix this by correctly handling validation failures in db.go, as well as returning the metadata with highest txid which is also valid in DB.meta(). Signed-off-by: Aleksa Sarai --- db.go | 49 ++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/db.go b/db.go index 8503665..68e5554 100644 --- a/db.go +++ b/db.go @@ -200,9 +200,15 @@ func Open(path string, mode os.FileMode, options *Options) (*DB, error) { if _, err := db.file.ReadAt(buf[:], 0); err == nil { m := db.pageInBuffer(buf[:], 0).meta() if err := m.validate(); err != nil { - return nil, err + // If we can't read the page size, we can assume it's the same + // as the OS -- since that's how the page size was chosen in the + // first place. + // XXX: Does this cause issues with opening a database on a + // different OS than the one it was created on? + db.pageSize = os.Getpagesize() + } else { + db.pageSize = int(m.pageSize) } - db.pageSize = int(m.pageSize) } } @@ -262,12 +268,13 @@ func (db *DB) mmap(minsz int) error { db.meta0 = db.page(0).meta() db.meta1 = db.page(1).meta() - // Validate the meta pages. - if err := db.meta0.validate(); err != nil { - return err - } - if err := db.meta1.validate(); err != nil { - return err + // Validate the meta pages. We only return an error if both meta pages fail + // validation, since meta0 failing validation means that it wasn't saved + // properly -- but we can recover using meta1. And vice-versa. + err0 := db.meta0.validate() + err1 := db.meta1.validate() + if err0 != nil && err1 != nil { + return fmt.Errorf("meta0(%v) meta1(%v)", err0, err1) } return nil @@ -778,10 +785,30 @@ func (db *DB) pageInBuffer(b []byte, id pgid) *page { // meta retrieves the current meta page reference. func (db *DB) meta() *meta { - if db.meta0.txid > db.meta1.txid { - return db.meta0 + // We have to return the meta with the highest txid which doesn't fail + // validation. Otherwise, we can cause errors when in fact the database is + // in a consistent state. metaA is the one with the higher txid. + metaA := db.meta0 + metaB := db.meta1 + if db.meta1.txid > db.meta0.txid { + metaA = db.meta1 + metaB = db.meta0 } - return db.meta1 + + errA := metaA.validate() + errB := metaB.validate() + + if errA == nil { + return metaA + } + + if errB == nil { + return metaB + } + + // This should never be reached, because both meta1 and meta0 were validated + // on mmap() and we do fsync() on every write. + panic("both meta0 and meta1 could not be validated in DB.meta()!") } // allocate returns a contiguous block of memory starting at a given page. -- cgit v1.2.3 From a5aec31dc3d13cbd7c0e6faca7489835b0b7e27a Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Sun, 24 Apr 2016 14:09:45 -0600 Subject: add additional meta page tests --- db.go | 27 +++++++++---------- db_test.go | 90 ++++++++++++++++++++++++++++++++++++++------------------------ errors.go | 3 ++- 3 files changed, 71 insertions(+), 49 deletions(-) diff --git a/db.go b/db.go index aab9d9a..1223493 100644 --- a/db.go +++ b/db.go @@ -208,8 +208,10 @@ func Open(path string, mode os.FileMode, options *Options) (*DB, error) { // If we can't read the page size, we can assume it's the same // as the OS -- since that's how the page size was chosen in the // first place. - // XXX: Does this cause issues with opening a database on a - // different OS than the one it was created on? + // + // If the first page is invalid and this OS uses a different + // page size than what the database was created with then we + // are out of luck and cannot access the database. db.pageSize = os.Getpagesize() } else { db.pageSize = int(m.pageSize) @@ -286,7 +288,7 @@ func (db *DB) mmap(minsz int) error { err0 := db.meta0.validate() err1 := db.meta1.validate() if err0 != nil && err1 != nil { - return fmt.Errorf("meta0(%v) meta1(%v)", err0, err1) + return err0 } return nil @@ -358,6 +360,7 @@ func (db *DB) init() error { m.root = bucket{root: 3} m.pgid = 4 m.txid = txid(i) + m.checksum = m.sum64() } // Write an empty freelist at page 3. @@ -807,20 +810,16 @@ func (db *DB) meta() *meta { metaB = db.meta0 } - errA := metaA.validate() - errB := metaB.validate() - - if errA == nil { + // Use higher meta page if valid. Otherwise fallback to previous, if valid. + if err := metaA.validate(); err == nil { return metaA - } - - if errB == nil { + } else if err := metaB.validate(); err == nil { return metaB } // This should never be reached, because both meta1 and meta0 were validated // on mmap() and we do fsync() on every write. - panic("both meta0 and meta1 could not be validated in DB.meta()!") + panic("bolt.DB.meta(): invalid meta pages") } // allocate returns a contiguous block of memory starting at a given page. @@ -981,12 +980,12 @@ type meta struct { // validate checks the marker bytes and version of the meta page to ensure it matches this binary. func (m *meta) validate() error { - if m.checksum != 0 && m.checksum != m.sum64() { - return ErrChecksum - } else if m.magic != magic { + if m.magic != magic { return ErrInvalid } else if m.version != version { return ErrVersionMismatch + } else if m.checksum != 0 && m.checksum != m.sum64() { + return ErrChecksum } return nil } diff --git a/db_test.go b/db_test.go index e31b865..74ff93a 100644 --- a/db_test.go +++ b/db_test.go @@ -45,7 +45,7 @@ type meta struct { _ uint32 _ [16]byte _ uint64 - _ uint64 + pgid uint64 _ uint64 checksum uint64 } @@ -85,20 +85,15 @@ func TestOpen_ErrNotExists(t *testing.T) { } } -// Ensure that opening a file with wrong checksum returns ErrChecksum. -func TestOpen_ErrChecksum(t *testing.T) { - buf := make([]byte, pageSize) - meta := (*meta)(unsafe.Pointer(&buf[0])) - meta.magic = magic - meta.version = version - meta.checksum = 123 - +// Ensure that opening a file that is not a Bolt database returns ErrInvalid. +func TestOpen_ErrInvalid(t *testing.T) { path := tempfile() + f, err := os.Create(path) if err != nil { t.Fatal(err) } - if _, err := f.WriteAt(buf, pageHeaderSize); err != nil { + if _, err := fmt.Fprintln(f, "this is not a bolt database"); err != nil { t.Fatal(err) } if err := f.Close(); err != nil { @@ -106,54 +101,81 @@ func TestOpen_ErrChecksum(t *testing.T) { } defer os.Remove(path) - if _, err := bolt.Open(path, 0666, nil); err != bolt.ErrChecksum { + if _, err := bolt.Open(path, 0666, nil); err != bolt.ErrInvalid { t.Fatalf("unexpected error: %s", err) } } -// Ensure that opening a file that is not a Bolt database returns ErrInvalid. -func TestOpen_ErrInvalid(t *testing.T) { - path := tempfile() +// Ensure that opening a file with two invalid versions returns ErrVersionMismatch. +func TestOpen_ErrVersionMismatch(t *testing.T) { + if pageSize != os.Getpagesize() { + t.Skip("page size mismatch") + } - f, err := os.Create(path) - if err != nil { + // Create empty database. + db := MustOpenDB() + path := db.Path() + defer db.MustClose() + + // Close database. + if err := db.DB.Close(); err != nil { t.Fatal(err) } - if _, err := fmt.Fprintln(f, "this is not a bolt database"); err != nil { + + // Read data file. + buf, err := ioutil.ReadFile(path) + if err != nil { t.Fatal(err) } - if err := f.Close(); err != nil { + + // Rewrite meta pages. + meta0 := (*meta)(unsafe.Pointer(&buf[pageHeaderSize])) + meta0.version++ + meta1 := (*meta)(unsafe.Pointer(&buf[pageSize+pageHeaderSize])) + meta1.version++ + if err := ioutil.WriteFile(path, buf, 0666); err != nil { t.Fatal(err) } - defer os.Remove(path) - if _, err := bolt.Open(path, 0666, nil); err != bolt.ErrInvalid { + // Reopen data file. + if _, err := bolt.Open(path, 0666, nil); err != bolt.ErrVersionMismatch { t.Fatalf("unexpected error: %s", err) } } -// Ensure that opening a file created with a different version of Bolt returns -// ErrVersionMismatch. -func TestOpen_ErrVersionMismatch(t *testing.T) { - buf := make([]byte, pageSize) - meta := (*meta)(unsafe.Pointer(&buf[0])) - meta.magic = magic - meta.version = version + 100 +// Ensure that opening a file with two invalid checksums returns ErrChecksum. +func TestOpen_ErrChecksum(t *testing.T) { + if pageSize != os.Getpagesize() { + t.Skip("page size mismatch") + } - path := tempfile() - f, err := os.Create(path) - if err != nil { + // Create empty database. + db := MustOpenDB() + path := db.Path() + defer db.MustClose() + + // Close database. + if err := db.DB.Close(); err != nil { t.Fatal(err) } - if _, err := f.WriteAt(buf, pageHeaderSize); err != nil { + + // Read data file. + buf, err := ioutil.ReadFile(path) + if err != nil { t.Fatal(err) } - if err := f.Close(); err != nil { + + // Rewrite meta pages. + meta0 := (*meta)(unsafe.Pointer(&buf[pageHeaderSize])) + meta0.pgid++ + meta1 := (*meta)(unsafe.Pointer(&buf[pageSize+pageHeaderSize])) + meta1.pgid++ + if err := ioutil.WriteFile(path, buf, 0666); err != nil { t.Fatal(err) } - defer os.Remove(path) - if _, err := bolt.Open(path, 0666, nil); err != bolt.ErrVersionMismatch { + // Reopen data file. + if _, err := bolt.Open(path, 0666, nil); err != bolt.ErrChecksum { t.Fatalf("unexpected error: %s", err) } } diff --git a/errors.go b/errors.go index 6883786..a3620a3 100644 --- a/errors.go +++ b/errors.go @@ -12,7 +12,8 @@ var ( // already open. ErrDatabaseOpen = errors.New("database already open") - // ErrInvalid is returned when a data file is not a Bolt-formatted database. + // ErrInvalid is returned when both meta pages on a database are invalid. + // This typically occurs when a file is not a bolt database. ErrInvalid = errors.New("invalid database") // ErrVersionMismatch is returned when the data file was created with a -- cgit v1.2.3