From 92a9f2e200e83fd0b3ddef177f9a454b0273c17e Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Tue, 27 May 2014 11:31:55 -0600 Subject: [PATCH] Remove DB.Check(). Allow read-only Tx.Check(). This commit removes the DB.Check() function and instead makes the user decide whether a transaction should be writable or read-only. Tx.Check() is not safe to use concurrently on a read-only transaction, however, it significantly improves the performance of it. --- cmd/bolt/check.go | 7 ++++++- db.go | 8 -------- db_test.go | 9 ++++++--- tx.go | 16 ++++++++-------- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/cmd/bolt/check.go b/cmd/bolt/check.go index ec2ea69..1436aba 100644 --- a/cmd/bolt/check.go +++ b/cmd/bolt/check.go @@ -21,7 +21,12 @@ func Check(path string) { defer db.Close() // Perform consistency check. - if err := db.Check(); err != nil { + err = db.View(func(tx *bolt.Tx) error { + return tx.Check() + }) + + // Print out any errors that occur. + if err != nil { if errors, ok := err.(bolt.ErrorList); ok { for _, err := range errors { println(err) diff --git a/db.go b/db.go index 5b6cc3e..7b17086 100644 --- a/db.go +++ b/db.go @@ -501,14 +501,6 @@ func (db *DB) Stats() Stats { return db.stats } -// Check performs several consistency checks on the database. -// An error is returned if any inconsistency is found. -func (db *DB) Check() error { - return db.Update(func(tx *Tx) error { - return tx.Check() - }) -} - // This is for internal access to the raw data bytes from the C cursor, use // carefully, or not at all. func (db *DB) Info() *Info { diff --git a/db_test.go b/db_test.go index b85f8cb..06accd3 100644 --- a/db_test.go +++ b/db_test.go @@ -53,12 +53,12 @@ func TestOpen_Check(t *testing.T) { withTempPath(func(path string) { db, err := Open(path, 0666) assert.NoError(t, err) - assert.NoError(t, db.Check()) + assert.NoError(t, db.View(func(tx *Tx) error { return tx.Check() })) db.Close() db, err = Open(path, 0666) assert.NoError(t, err) - assert.NoError(t, db.Check()) + assert.NoError(t, db.View(func(tx *Tx) error { return tx.Check() })) db.Close() }) } @@ -463,7 +463,10 @@ func withOpenDB(fn func(*DB, string)) { // mustCheck runs a consistency check on the database and panics if any errors are found. func mustCheck(db *DB) { - if err := db.Check(); err != nil { + err := db.Update(func(tx *Tx) error { + return tx.Check() + }) + if err != nil { // Copy db off first. var path = tempfile() db.View(func(tx *Tx) error { return tx.CopyFile(path, 0600) }) diff --git a/tx.go b/tx.go index 8791adf..a0ae5bd 100644 --- a/tx.go +++ b/tx.go @@ -284,12 +284,14 @@ func (tx *Tx) CopyFile(path string, mode os.FileMode) error { } // Check performs several consistency checks on the database for this transaction. -// An error is returned if any inconsistency is found or if executed on a read-only transaction. +// An error is returned if any inconsistency is found. +// +// It can be safely run concurrently on a writable transaction. However, this +// incurs a high cost for large databases and databases with a lot of subbuckets +// because of caching. This overhead can be removed if running on a read-only +// transaction, however, it is not safe to execute other writer transactions at +// the same time. func (tx *Tx) Check() error { - if !tx.writable { - return ErrTxNotWritable - } - var errors ErrorList // Check if any pages are double freed. @@ -466,12 +468,10 @@ func (tx *Tx) forEachPage(pgid pgid, depth int, fn func(*page, int)) { } // Page returns page information for a given page number. -// This is only available from writable transactions. +// This is only safe for concurrent use when used by a writable transaction. func (tx *Tx) Page(id int) (*PageInfo, error) { if tx.db == nil { return nil, ErrTxClosed - } else if !tx.writable { - return nil, ErrTxNotWritable } else if pgid(id) >= tx.meta.pgid { return nil, nil }