From 50a30aafc18bfbd5890e4bab20633e843e173843 Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Tue, 12 Dec 2017 12:44:47 -0800 Subject: [PATCH 01/19] New canonical Iterator --- db/cache_db.go | 2 + db/db.go | 88 --------------------------------------- db/types.go | 111 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 88 deletions(-) create mode 100644 db/types.go diff --git a/db/cache_db.go b/db/cache_db.go index ed85e3057..586f2f679 100644 --- a/db/cache_db.go +++ b/db/cache_db.go @@ -1,3 +1,4 @@ +/* package db import ( @@ -229,3 +230,4 @@ func (cww *cwwMutex) TryWriteLock(version interface{}) bool { cww.written = new(int32) return true } +*/ diff --git a/db/db.go b/db/db.go index 08ebaeaac..ba137743b 100644 --- a/db/db.go +++ b/db/db.go @@ -1,93 +1,5 @@ package db -import . "github.com/tendermint/tmlibs/common" - -type DB interface { - Get([]byte) []byte // NOTE: returns nil iff never set or deleted. - Set([]byte, []byte) - SetSync([]byte, []byte) - Delete([]byte) - DeleteSync([]byte) - Close() - NewBatch() Batch - Iterator() Iterator - - // For debugging - Print() - - // Stats returns a map of property values for all keys and the size of the cache. - Stats() map[string]string - - // CacheDB wraps the DB w/ a cache. - CacheDB() CacheDB -} - -type CacheDB interface { - DB - Write() // Write to the underlying DB -} - -type SetDeleter interface { - Set(key, value []byte) - Delete(key []byte) -} - -type Batch interface { - SetDeleter - Write() -} - -/* - Usage: - - for itr.Seek(mykey); itr.Valid(); itr.Next() { - k, v := itr.Key(); itr.Value() - .... - } -*/ -type Iterator interface { - - // Seek moves the iterator the position of the key given or, if the key - // doesn't exist, the next key that does exist in the database. If the key - // doesn't exist, and there is no next key, the Iterator becomes invalid. - Seek(key []byte) - - // Valid returns false only when an Iterator has iterated past either the - // first or the last key in the database. - Valid() bool - - // Next moves the iterator to the next sequential key in the database, as - // defined by the Comparator in the ReadOptions used to create this Iterator. - // - // If Valid returns false, this method will panic. - Next() - - // Prev moves the iterator to the previous sequential key in the database, as - // defined by the Comparator in the ReadOptions used to create this Iterator. - // - // If Valid returns false, this method will panic. - Prev() - - // Key returns the key of the cursor. - // - // If Valid returns false, this method will panic. - Key() []byte - - // Value returns the key of the cursor. - // - // If Valid returns false, this method will panic. - Value() []byte - - // GetError returns an IteratorError from LevelDB if it had one during - // iteration. - // - // This method is safe to call when Valid returns false. - GetError() error - - // Close deallocates the given Iterator. - Close() -} - //----------------------------------------------------------------------------- // Main entry diff --git a/db/types.go b/db/types.go new file mode 100644 index 000000000..f343e1d72 --- /dev/null +++ b/db/types.go @@ -0,0 +1,111 @@ +package db + +type DB interface { + + // Get returns nil iff key doesn't exist. Panics on nil key. + Get([]byte) []byte + + // Has checks if a key exists. Panics on nil key. + Has(key []byte) bool + + // Set sets the key. Panics on nil key. + Set([]byte, []byte) + SetSync([]byte, []byte) + + // Delete deletes the key. Panics on nil key. + Delete([]byte) + DeleteSync([]byte) + + // Iterator over a domain of keys in ascending order. End is exclusive. + // Start must be less than end, or the Iterator is invalid. + // CONTRACT: No writes may happen within a domain while an iterator exists over it. + Iterator(start, end []byte) Iterator + + // Iterator over a domain of keys in descending order. End is exclusive. + // Start must be greater than end, or the Iterator is invalid. + // CONTRACT: No writes may happen within a domain while an iterator exists over it. + ReverseIterator(start, end []byte) Iterator + + // Releases the connection. + Close() + + // Creates a batch for atomic updates. + NewBatch() Batch + + // For debugging + Print() + + // Stats returns a map of property values for all keys and the size of the cache. + Stats() map[string]string +} + +//---------------------------------------- +// Batch + +type Batch interface { + SetDeleter + Write() +} + +type SetDeleter interface { + Set(key, value []byte) + Delete(key []byte) +} + +//---------------------------------------- + +/* + Usage: + + for itr.Seek(mykey); itr.Valid(); itr.Next() { + k, v := itr.Key(); itr.Value() + .... + } +*/ +type Iterator interface { + + // The start & end (exclusive) limits to iterate over. + // If end < start, then the Iterator goes in reverse order. + // + // A domain of ([]byte{12, 13}, []byte{12, 14}) will iterate + // over anything with the prefix []byte{12, 13}. + // + // The smallest key is the empty byte array []byte{}. + // The largest key is the nil byte array []byte(nil). + Domain() (start []byte, end []byte) + + // Valid returns whether the current position is valid. + // Once invalid, an Iterator is forever invalid. + Valid() bool + + // Next moves the iterator to the next sequential key in the database, as + // defined by the Comparator in the ReadOptions used to create this Iterator. + // + // If Valid returns false, this method will panic. + Next() + + // Prev moves the iterator to the previous sequential key in the database, as + // defined by the Comparator in the ReadOptions used to create this Iterator. + // + // If Valid returns false, this method will panic. + Prev() + + // Key returns the key of the cursor. + // + // If Valid returns false, this method will panic. + Key() []byte + + // Value returns the key of the cursor. + // + // If Valid returns false, this method will panic. + Value() []byte + + // GetError returns an IteratorError from LevelDB if it had one during + // iteration. + // + // This method is safe to call when Valid returns false. + GetError() error + + // Close deallocates the given Iterator. + Close() +} From 0d03cd9e31a1cebca1616a42acb65e307535223a Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Tue, 12 Dec 2017 14:00:03 -0800 Subject: [PATCH 02/19] Make it compile --- db/c_level_db.go | 37 ++++---- db/cache_db.go | 233 ---------------------------------------------- db/db.go | 4 +- db/fsdb.go | 63 ++++++++----- db/go_level_db.go | 42 ++++++--- db/mem_db.go | 53 ++++++----- db/types.go | 19 +++- db/util.go | 107 ++++++--------------- 8 files changed, 167 insertions(+), 391 deletions(-) delete mode 100644 db/cache_db.go diff --git a/db/c_level_db.go b/db/c_level_db.go index e4450aaa6..e867b0004 100644 --- a/db/c_level_db.go +++ b/db/c_level_db.go @@ -22,8 +22,6 @@ type CLevelDB struct { ro *levigo.ReadOptions wo *levigo.WriteOptions woSync *levigo.WriteOptions - - cwwMutex } func NewCLevelDB(name string, dir string) (*CLevelDB, error) { @@ -45,8 +43,6 @@ func NewCLevelDB(name string, dir string) (*CLevelDB, error) { ro: ro, wo: wo, woSync: woSync, - - cwwMutex: NewCWWMutex(), } return database, nil } @@ -59,6 +55,10 @@ func (db *CLevelDB) Get(key []byte) []byte { return res } +func (db *CLevelDB) Has(key []byte) bool { + panic("not implemented yet") +} + func (db *CLevelDB) Set(key []byte, value []byte) { err := db.db.Put(db.wo, key, value) if err != nil { @@ -99,9 +99,9 @@ func (db *CLevelDB) Close() { } func (db *CLevelDB) Print() { - itr := db.Iterator() - defer itr.Close() - for itr.Seek(nil); itr.Valid(); itr.Next() { + itr := db.Iterator(BeginningKey(), EndingKey()) + defer itr.Release() + for ; itr.Valid(); itr.Next() { key := itr.Key() value := itr.Value() fmt.Printf("[%X]:\t[%X]\n", key, value) @@ -120,10 +120,6 @@ func (db *CLevelDB) Stats() map[string]string { return stats } -func (db *CLevelDB) CacheDB() CacheDB { - return NewCacheDB(db, db.GetWriteLockVersion()) -} - //---------------------------------------- // Batch @@ -155,10 +151,19 @@ func (mBatch *cLevelDBBatch) Write() { //---------------------------------------- // Iterator -func (db *CLevelDB) Iterator() Iterator { - itr := db.db.NewIterator(db.ro) - itr.Seek([]byte{0x00}) - return cLevelDBIterator{itr} +func (db *CLevelDB) Iterator(start, end []byte) Iterator { + /* + XXX + itr := db.db.NewIterator(db.ro) + itr.Seek([]byte{0x00}) + return cLevelDBIterator{itr} + */ + return nil +} + +func (db *CLevelDB) ReverseIterator(start, end []byte) Iterator { + // XXX + return nil } type cLevelDBIterator struct { @@ -204,7 +209,7 @@ func (c cLevelDBIterator) Prev() { c.itr.Prev() } -func (c cLevelDBIterator) Close() { +func (c cLevelDBIterator) Release() { c.itr.Close() } diff --git a/db/cache_db.go b/db/cache_db.go deleted file mode 100644 index 586f2f679..000000000 --- a/db/cache_db.go +++ /dev/null @@ -1,233 +0,0 @@ -/* -package db - -import ( - "fmt" - "sort" - "sync" - "sync/atomic" -) - -// If value is nil but deleted is false, -// it means the parent doesn't have the key. -// (No need to delete upon Write()) -type cDBValue struct { - value []byte - deleted bool - dirty bool -} - -// cacheDB wraps an in-memory cache around an underlying DB. -type cacheDB struct { - mtx sync.Mutex - cache map[string]cDBValue - parent DB - lockVersion interface{} - - cwwMutex -} - -// Needed by MultiStore.CacheWrap(). -var _ atomicSetDeleter = (*cacheDB)(nil) -var _ CacheDB = (*cacheDB)(nil) - -// Users should typically not be required to call NewCacheDB directly, as the -// DB implementations here provide a .CacheDB() function already. -// `lockVersion` is typically provided by parent.GetWriteLockVersion(). -func NewCacheDB(parent DB, lockVersion interface{}) CacheDB { - db := &cacheDB{ - cache: make(map[string]cDBValue), - parent: parent, - lockVersion: lockVersion, - cwwMutex: NewCWWMutex(), - } - return db -} - -func (db *cacheDB) Get(key []byte) []byte { - db.mtx.Lock() - defer db.mtx.Unlock() - - dbValue, ok := db.cache[string(key)] - if !ok { - data := db.parent.Get(key) - dbValue = cDBValue{value: data, deleted: false, dirty: false} - db.cache[string(key)] = dbValue - } - return dbValue.value -} - -func (db *cacheDB) Set(key []byte, value []byte) { - db.mtx.Lock() - defer db.mtx.Unlock() - - db.SetNoLock(key, value) -} - -func (db *cacheDB) SetSync(key []byte, value []byte) { - db.mtx.Lock() - defer db.mtx.Unlock() - - db.SetNoLock(key, value) -} - -func (db *cacheDB) SetNoLock(key []byte, value []byte) { - db.cache[string(key)] = cDBValue{value: value, deleted: false, dirty: true} -} - -func (db *cacheDB) Delete(key []byte) { - db.mtx.Lock() - defer db.mtx.Unlock() - - db.DeleteNoLock(key) -} - -func (db *cacheDB) DeleteSync(key []byte) { - db.mtx.Lock() - defer db.mtx.Unlock() - - db.DeleteNoLock(key) -} - -func (db *cacheDB) DeleteNoLock(key []byte) { - db.cache[string(key)] = cDBValue{value: nil, deleted: true, dirty: true} -} - -func (db *cacheDB) Close() { - db.mtx.Lock() - defer db.mtx.Unlock() - - db.parent.Close() -} - -func (db *cacheDB) Print() { - db.mtx.Lock() - defer db.mtx.Unlock() - - fmt.Println("cacheDB\ncache:") - for key, value := range db.cache { - fmt.Printf("[%X]:\t[%v]\n", []byte(key), value) - } - fmt.Println("\nparent:") - db.parent.Print() -} - -func (db *cacheDB) Stats() map[string]string { - db.mtx.Lock() - defer db.mtx.Unlock() - - stats := make(map[string]string) - stats["cache.size"] = fmt.Sprintf("%d", len(db.cache)) - stats["cache.lock_version"] = fmt.Sprintf("%v", db.lockVersion) - mergeStats(db.parent.Stats(), stats, "parent.") - return stats -} - -func (db *cacheDB) Iterator() Iterator { - panic("cacheDB.Iterator() not yet supported") -} - -func (db *cacheDB) NewBatch() Batch { - return &memBatch{db, nil} -} - -// Implements `atomicSetDeleter` for Batch support. -func (db *cacheDB) Mutex() *sync.Mutex { - return &(db.mtx) -} - -// Write writes pending updates to the parent database and clears the cache. -func (db *cacheDB) Write() { - db.mtx.Lock() - defer db.mtx.Unlock() - - // Optional sanity check to ensure that cacheDB is valid - if parent, ok := db.parent.(WriteLocker); ok { - if parent.TryWriteLock(db.lockVersion) { - // All good! - } else { - panic("cacheDB.Write() failed. Did this CacheDB expire?") - } - } - - // We need a copy of all of the keys. - // Not the best, but probably not a bottleneck depending. - keys := make([]string, 0, len(db.cache)) - for key, dbValue := range db.cache { - if dbValue.dirty { - keys = append(keys, key) - } - } - sort.Strings(keys) - - batch := db.parent.NewBatch() - for _, key := range keys { - dbValue := db.cache[key] - if dbValue.deleted { - batch.Delete([]byte(key)) - } else if dbValue.value == nil { - // Skip, it already doesn't exist in parent. - } else { - batch.Set([]byte(key), dbValue.value) - } - } - batch.Write() - - // Clear the cache - db.cache = make(map[string]cDBValue) -} - -//---------------------------------------- -// To cache-wrap this cacheDB further. - -func (db *cacheDB) CacheDB() CacheDB { - return NewCacheDB(db, db.GetWriteLockVersion()) -} - -// If the parent parent DB implements this, (e.g. such as a cacheDB parent to a -// cacheDB child), cacheDB will call `parent.TryWriteLock()` before attempting -// to write. -type WriteLocker interface { - GetWriteLockVersion() (lockVersion interface{}) - TryWriteLock(lockVersion interface{}) bool -} - -// Implements TryWriteLocker. Embed this in DB structs if desired. -type cwwMutex struct { - mtx sync.Mutex - // CONTRACT: reading/writing to `*written` should use `atomic.*`. - // CONTRACT: replacing `written` with another *int32 should use `.mtx`. - written *int32 -} - -func NewCWWMutex() cwwMutex { - return cwwMutex{ - written: new(int32), - } -} - -func (cww *cwwMutex) GetWriteLockVersion() interface{} { - cww.mtx.Lock() - defer cww.mtx.Unlock() - - // `written` works as a "version" object because it gets replaced upon - // successful TryWriteLock. - return cww.written -} - -func (cww *cwwMutex) TryWriteLock(version interface{}) bool { - cww.mtx.Lock() - defer cww.mtx.Unlock() - - if version != cww.written { - return false // wrong "WriteLockVersion" - } - if !atomic.CompareAndSwapInt32(cww.written, 0, 1) { - return false // already written - } - - // New "WriteLockVersion" - cww.written = new(int32) - return true -} -*/ diff --git a/db/db.go b/db/db.go index ba137743b..7eec04d56 100644 --- a/db/db.go +++ b/db/db.go @@ -1,5 +1,7 @@ package db +import "fmt" + //----------------------------------------------------------------------------- // Main entry @@ -26,7 +28,7 @@ func registerDBCreator(backend string, creator dbCreator, force bool) { func NewDB(name string, backend string, dir string) DB { db, err := backends[backend](name, dir) if err != nil { - PanicSanity(Fmt("Error initializing DB: %v", err)) + panic(fmt.Sprintf("Error initializing DB: %v", err)) } return db } diff --git a/db/fsdb.go b/db/fsdb.go index 4b1914453..b6e08daf5 100644 --- a/db/fsdb.go +++ b/db/fsdb.go @@ -7,7 +7,6 @@ import ( "os" "path" "path/filepath" - "sort" "sync" "github.com/pkg/errors" @@ -29,8 +28,6 @@ func init() { type FSDB struct { mtx sync.Mutex dir string - - cwwMutex } func NewFSDB(dir string) *FSDB { @@ -39,8 +36,7 @@ func NewFSDB(dir string) *FSDB { panic(errors.Wrap(err, "Creating FSDB dir "+dir)) } database := &FSDB{ - dir: dir, - cwwMutex: NewCWWMutex(), + dir: dir, } return database } @@ -59,6 +55,20 @@ func (db *FSDB) Get(key []byte) []byte { return value } +func (db *FSDB) Has(key []byte) bool { + db.mtx.Lock() + defer db.mtx.Unlock() + + path := db.nameToPath(key) + _, err := read(path) + if os.IsNotExist(err) { + return false + } else if err != nil { + panic(errors.Wrap(err, fmt.Sprintf("Getting key %s (0x%X)", string(key), key))) + } + return true +} + func (db *FSDB) Set(key []byte, value []byte) { db.mtx.Lock() defer db.mtx.Unlock() @@ -140,27 +150,32 @@ func (db *FSDB) Mutex() *sync.Mutex { return &(db.mtx) } -func (db *FSDB) CacheDB() CacheDB { - return NewCacheDB(db, db.GetWriteLockVersion()) +func (db *FSDB) Iterator(start, end []byte) Iterator { + /* + XXX + it := newMemDBIterator() + it.db = db + it.cur = 0 + + db.mtx.Lock() + defer db.mtx.Unlock() + + // We need a copy of all of the keys. + // Not the best, but probably not a bottleneck depending. + keys, err := list(db.dir) + if err != nil { + panic(errors.Wrap(err, fmt.Sprintf("Listing keys in %s", db.dir))) + } + sort.Strings(keys) + it.keys = keys + return it + */ + return nil } -func (db *FSDB) Iterator() Iterator { - it := newMemDBIterator() - it.db = db - it.cur = 0 - - db.mtx.Lock() - defer db.mtx.Unlock() - - // We need a copy of all of the keys. - // Not the best, but probably not a bottleneck depending. - keys, err := list(db.dir) - if err != nil { - panic(errors.Wrap(err, fmt.Sprintf("Listing keys in %s", db.dir))) - } - sort.Strings(keys) - it.keys = keys - return it +func (db *FSDB) ReverseIterator(start, end []byte) Iterator { + // XXX + return nil } func (db *FSDB) nameToPath(name []byte) string { diff --git a/db/go_level_db.go b/db/go_level_db.go index cffe7329c..e8ed99dee 100644 --- a/db/go_level_db.go +++ b/db/go_level_db.go @@ -22,8 +22,6 @@ func init() { type GoLevelDB struct { db *leveldb.DB - - cwwMutex } func NewGoLevelDB(name string, dir string) (*GoLevelDB, error) { @@ -33,8 +31,7 @@ func NewGoLevelDB(name string, dir string) (*GoLevelDB, error) { return nil, err } database := &GoLevelDB{ - db: db, - cwwMutex: NewCWWMutex(), + db: db, } return database, nil } @@ -51,6 +48,18 @@ func (db *GoLevelDB) Get(key []byte) []byte { return res } +func (db *GoLevelDB) Has(key []byte) bool { + _, err := db.db.Get(key, nil) + if err != nil { + if err == errors.ErrNotFound { + return false + } else { + PanicCrisis(err) + } + } + return true +} + func (db *GoLevelDB) Set(key []byte, value []byte) { err := db.db.Put(key, value, nil) if err != nil { @@ -121,10 +130,6 @@ func (db *GoLevelDB) Stats() map[string]string { return stats } -func (db *GoLevelDB) CacheDB() CacheDB { - return NewCacheDB(db, db.GetWriteLockVersion()) -} - //---------------------------------------- // Batch @@ -156,12 +161,21 @@ func (mBatch *goLevelDBBatch) Write() { //---------------------------------------- // Iterator -func (db *GoLevelDB) Iterator() Iterator { - itr := &goLevelDBIterator{ - source: db.db.NewIterator(nil, nil), - } - itr.Seek(nil) - return itr +func (db *GoLevelDB) Iterator(start, end []byte) Iterator { + /* + XXX + itr := &goLevelDBIterator{ + source: db.db.NewIterator(nil, nil), + } + itr.Seek(nil) + return itr + */ + return nil +} + +func (db *GoLevelDB) ReverseIterator(start, end []byte) Iterator { + // XXX + return nil } type goLevelDBIterator struct { diff --git a/db/mem_db.go b/db/mem_db.go index f5d55f3ae..3127030ae 100644 --- a/db/mem_db.go +++ b/db/mem_db.go @@ -3,7 +3,6 @@ package db import ( "bytes" "fmt" - "sort" "sync" ) @@ -16,14 +15,11 @@ func init() { type MemDB struct { mtx sync.Mutex db map[string][]byte - - cwwMutex } func NewMemDB() *MemDB { database := &MemDB{ - db: make(map[string][]byte), - cwwMutex: NewCWWMutex(), + db: make(map[string][]byte), } return database } @@ -35,6 +31,14 @@ func (db *MemDB) Get(key []byte) []byte { return db.db[string(key)] } +func (db *MemDB) Has(key []byte) bool { + db.mtx.Lock() + defer db.mtx.Unlock() + + _, ok := db.db[string(key)] + return ok +} + func (db *MemDB) Set(key []byte, value []byte) { db.mtx.Lock() defer db.mtx.Unlock() @@ -114,27 +118,32 @@ func (db *MemDB) Mutex() *sync.Mutex { return &(db.mtx) } -func (db *MemDB) CacheDB() CacheDB { - return NewCacheDB(db, db.GetWriteLockVersion()) -} - //---------------------------------------- -func (db *MemDB) Iterator() Iterator { - it := newMemDBIterator() - it.db = db - it.cur = 0 +func (db *MemDB) Iterator(start, end []byte) Iterator { + /* + XXX + it := newMemDBIterator() + it.db = db + it.cur = 0 - db.mtx.Lock() - defer db.mtx.Unlock() + db.mtx.Lock() + defer db.mtx.Unlock() - // We need a copy of all of the keys. - // Not the best, but probably not a bottleneck depending. - for key, _ := range db.db { - it.keys = append(it.keys, key) - } - sort.Strings(it.keys) - return it + // We need a copy of all of the keys. + // Not the best, but probably not a bottleneck depending. + for key, _ := range db.db { + it.keys = append(it.keys, key) + } + sort.Strings(it.keys) + return it + */ + return nil +} + +func (db *MemDB) ReverseIterator(start, end []byte) Iterator { + // XXX + return nil } type memDBIterator struct { diff --git a/db/types.go b/db/types.go index f343e1d72..7422a5155 100644 --- a/db/types.go +++ b/db/types.go @@ -54,12 +54,23 @@ type SetDeleter interface { //---------------------------------------- +func BeginningKey() []byte { + return []byte{} +} + +func EndingKey() []byte { + return nil +} + /* Usage: - for itr.Seek(mykey); itr.Valid(); itr.Next() { + var itr Iterator = ... + defer itr.Release() + + for ; itr.Valid(); itr.Next() { k, v := itr.Key(); itr.Value() - .... + // ... } */ type Iterator interface { @@ -106,6 +117,6 @@ type Iterator interface { // This method is safe to call when Valid returns false. GetError() error - // Close deallocates the given Iterator. - Close() + // Release deallocates the given Iterator. + Release() } diff --git a/db/util.go b/db/util.go index 5f381a5be..89c777622 100644 --- a/db/util.go +++ b/db/util.go @@ -1,82 +1,35 @@ package db -import "bytes" - -// A wrapper around itr that tries to keep the iterator -// within the bounds as defined by `prefix` -type prefixIterator struct { - itr Iterator - prefix []byte - invalid bool -} - -func (pi *prefixIterator) Seek(key []byte) { - if !bytes.HasPrefix(key, pi.prefix) { - pi.invalid = true - return - } - pi.itr.Seek(key) - pi.checkInvalid() -} - -func (pi *prefixIterator) checkInvalid() { - if !pi.itr.Valid() { - pi.invalid = true - } -} - -func (pi *prefixIterator) Valid() bool { - if pi.invalid { - return false - } - key := pi.itr.Key() - ok := bytes.HasPrefix(key, pi.prefix) - if !ok { - pi.invalid = true - return false - } - return true -} - -func (pi *prefixIterator) Next() { - if pi.invalid { - panic("prefixIterator Next() called when invalid") - } - pi.itr.Next() - pi.checkInvalid() -} - -func (pi *prefixIterator) Prev() { - if pi.invalid { - panic("prefixIterator Prev() called when invalid") - } - pi.itr.Prev() - pi.checkInvalid() -} - -func (pi *prefixIterator) Key() []byte { - if pi.invalid { - panic("prefixIterator Key() called when invalid") - } - return pi.itr.Key() -} - -func (pi *prefixIterator) Value() []byte { - if pi.invalid { - panic("prefixIterator Value() called when invalid") - } - return pi.itr.Value() -} - -func (pi *prefixIterator) Close() { pi.itr.Close() } -func (pi *prefixIterator) GetError() error { return pi.itr.GetError() } - func IteratePrefix(db DB, prefix []byte) Iterator { - itr := db.Iterator() - pi := &prefixIterator{ - itr: itr, - prefix: prefix, + var start, end []byte + if len(prefix) == 0 { + start = BeginningKey() + end = EndingKey() + } else { + start = cp(prefix) + end = cpIncr(prefix) } - pi.Seek(prefix) - return pi + return db.Iterator(start, end) +} + +//---------------------------------------- + +func cp(bz []byte) (ret []byte) { + ret = make([]byte, len(bz)) + copy(ret, bz) + return ret +} + +// CONTRACT: len(bz) > 0 +func cpIncr(bz []byte) (ret []byte) { + ret = cp(bz) + for i := len(bz) - 1; i >= 0; i-- { + if ret[i] < byte(0xFF) { + ret[i] += 1 + return + } else { + ret[i] = byte(0x00) + } + } + return EndingKey() } From 7f650cea8673ee3169e51a41c1a8038e38c59ef4 Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Tue, 12 Dec 2017 14:45:31 -0800 Subject: [PATCH 03/19] Remove Prev from Iterator --- db/c_level_db.go | 7 ---- db/cache_db_test.go | 83 --------------------------------------------- db/common_test.go | 51 ++++------------------------ db/mem_db_test.go | 2 +- db/types.go | 13 ++++--- db/util_test.go | 82 +------------------------------------------- 6 files changed, 15 insertions(+), 223 deletions(-) delete mode 100644 db/cache_db_test.go diff --git a/db/c_level_db.go b/db/c_level_db.go index e867b0004..11a6e5ff7 100644 --- a/db/c_level_db.go +++ b/db/c_level_db.go @@ -202,13 +202,6 @@ func (c cLevelDBIterator) Next() { c.itr.Next() } -func (c cLevelDBIterator) Prev() { - if !c.itr.Valid() { - panic("cLevelDBIterator Prev() called when invalid") - } - c.itr.Prev() -} - func (c cLevelDBIterator) Release() { c.itr.Close() } diff --git a/db/cache_db_test.go b/db/cache_db_test.go deleted file mode 100644 index 2a2684fe2..000000000 --- a/db/cache_db_test.go +++ /dev/null @@ -1,83 +0,0 @@ -package db - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func bz(s string) []byte { return []byte(s) } - -func TestCacheDB(t *testing.T) { - mem := NewMemDB() - cdb := mem.CacheDB() - - require.Empty(t, cdb.Get(bz("key1")), "Expected `key1` to be empty") - - mem.Set(bz("key1"), bz("value1")) - cdb.Set(bz("key1"), bz("value1")) - require.Equal(t, bz("value1"), cdb.Get(bz("key1"))) - - cdb.Set(bz("key1"), bz("value2")) - require.Equal(t, bz("value2"), cdb.Get(bz("key1"))) - require.Equal(t, bz("value1"), mem.Get(bz("key1"))) - - cdb.Write() - require.Equal(t, bz("value2"), mem.Get(bz("key1"))) - - require.Panics(t, func() { cdb.Write() }, "Expected second cdb.Write() to fail") - - cdb = mem.CacheDB() - cdb.Delete(bz("key1")) - require.Empty(t, cdb.Get(bz("key1"))) - require.Equal(t, mem.Get(bz("key1")), bz("value2")) - - cdb.Write() - require.Empty(t, cdb.Get(bz("key1")), "Expected `key1` to be empty") - require.Empty(t, mem.Get(bz("key1")), "Expected `key1` to be empty") -} - -func TestCacheDBWriteLock(t *testing.T) { - mem := NewMemDB() - cdb := mem.CacheDB() - require.NotPanics(t, func() { cdb.Write() }) - require.Panics(t, func() { cdb.Write() }) - cdb = mem.CacheDB() - require.NotPanics(t, func() { cdb.Write() }) - require.Panics(t, func() { cdb.Write() }) -} - -func TestCacheDBWriteLockNested(t *testing.T) { - mem := NewMemDB() - cdb := mem.CacheDB() - cdb2 := cdb.CacheDB() - require.NotPanics(t, func() { cdb2.Write() }) - require.Panics(t, func() { cdb2.Write() }) - cdb2 = cdb.CacheDB() - require.NotPanics(t, func() { cdb2.Write() }) - require.Panics(t, func() { cdb2.Write() }) -} - -func TestCacheDBNested(t *testing.T) { - mem := NewMemDB() - cdb := mem.CacheDB() - cdb.Set(bz("key1"), bz("value1")) - - require.Empty(t, mem.Get(bz("key1"))) - require.Equal(t, bz("value1"), cdb.Get(bz("key1"))) - cdb2 := cdb.CacheDB() - require.Equal(t, bz("value1"), cdb2.Get(bz("key1"))) - - cdb2.Set(bz("key1"), bz("VALUE2")) - require.Equal(t, []byte(nil), mem.Get(bz("key1"))) - require.Equal(t, bz("value1"), cdb.Get(bz("key1"))) - require.Equal(t, bz("VALUE2"), cdb2.Get(bz("key1"))) - - cdb2.Write() - require.Equal(t, []byte(nil), mem.Get(bz("key1"))) - require.Equal(t, bz("VALUE2"), cdb.Get(bz("key1"))) - - cdb.Write() - require.Equal(t, bz("VALUE2"), mem.Get(bz("key1"))) - -} diff --git a/db/common_test.go b/db/common_test.go index 505864c20..09fad8424 100644 --- a/db/common_test.go +++ b/db/common_test.go @@ -23,16 +23,6 @@ func checkNextPanics(t *testing.T, itr Iterator) { assert.Panics(t, func() { itr.Next() }, "checkNextPanics expected panic but didn't") } -func checkPrevPanics(t *testing.T, itr Iterator) { - assert.Panics(t, func() { itr.Prev() }, "checkPrevPanics expected panic but didn't") -} - -func checkPrev(t *testing.T, itr Iterator, expected bool) { - itr.Prev() - valid := itr.Valid() - assert.Equal(t, expected, valid) -} - func checkItem(t *testing.T, itr Iterator, key []byte, value []byte) { k, v := itr.Key(), itr.Value() assert.Exactly(t, key, k) @@ -44,7 +34,6 @@ func checkInvalid(t *testing.T, itr Iterator) { checkKeyPanics(t, itr) checkValuePanics(t, itr) checkNextPanics(t, itr) - checkPrevPanics(t, itr) } func checkKeyPanics(t *testing.T, itr Iterator) { @@ -67,7 +56,7 @@ func TestDBIteratorSingleKey(t *testing.T) { t.Run(fmt.Sprintf("Backend %s", backend), func(t *testing.T) { db := newTempDB(t, backend) db.SetSync(bz("1"), bz("value_1")) - itr := db.Iterator() + itr := db.Iterator(BeginningKey(), EndingKey()) checkValid(t, itr, true) checkNext(t, itr, false) @@ -88,15 +77,12 @@ func TestDBIteratorTwoKeys(t *testing.T) { db.SetSync(bz("2"), bz("value_1")) { // Fail by calling Next too much - itr := db.Iterator() + itr := db.Iterator(BeginningKey(), EndingKey()) checkValid(t, itr, true) for i := 0; i < 10; i++ { checkNext(t, itr, true) checkValid(t, itr, true) - - checkPrev(t, itr, true) - checkValid(t, itr, true) } checkNext(t, itr, true) @@ -110,27 +96,6 @@ func TestDBIteratorTwoKeys(t *testing.T) { // Once invalid... checkInvalid(t, itr) } - - { // Fail by calling Prev too much - itr := db.Iterator() - checkValid(t, itr, true) - - for i := 0; i < 10; i++ { - checkNext(t, itr, true) - checkValid(t, itr, true) - - checkPrev(t, itr, true) - checkValid(t, itr, true) - } - - checkPrev(t, itr, false) - checkValid(t, itr, false) - - checkPrevPanics(t, itr) - - // Once invalid... - checkInvalid(t, itr) - } }) } } @@ -139,32 +104,30 @@ func TestDBIteratorEmpty(t *testing.T) { for backend, _ := range backends { t.Run(fmt.Sprintf("Backend %s", backend), func(t *testing.T) { db := newTempDB(t, backend) - itr := db.Iterator() + itr := db.Iterator(BeginningKey(), EndingKey()) checkInvalid(t, itr) }) } } -func TestDBIteratorEmptySeek(t *testing.T) { +func TestDBIteratorEmptyBeginAfter(t *testing.T) { for backend, _ := range backends { t.Run(fmt.Sprintf("Backend %s", backend), func(t *testing.T) { db := newTempDB(t, backend) - itr := db.Iterator() - itr.Seek(bz("1")) + itr := db.Iterator(bz("1"), EndingKey()) checkInvalid(t, itr) }) } } -func TestDBIteratorBadSeek(t *testing.T) { +func TestDBIteratorNonemptyBeginAfter(t *testing.T) { for backend, _ := range backends { t.Run(fmt.Sprintf("Backend %s", backend), func(t *testing.T) { db := newTempDB(t, backend) db.SetSync(bz("1"), bz("value_1")) - itr := db.Iterator() - itr.Seek(bz("2")) + itr := db.Iterator(bz("2"), EndingKey()) checkInvalid(t, itr) }) diff --git a/db/mem_db_test.go b/db/mem_db_test.go index b5c9167c8..42e242857 100644 --- a/db/mem_db_test.go +++ b/db/mem_db_test.go @@ -19,7 +19,7 @@ func TestMemDbIterator(t *testing.T) { db.Set(k, value) } - iter := db.Iterator() + iter := db.Iterator(BeginningKey(), EndingKey()) i := 0 for ; iter.Valid(); iter.Next() { assert.Equal(t, db.Get(iter.Key()), iter.Value(), "values dont match for key") diff --git a/db/types.go b/db/types.go index 7422a5155..8306813c7 100644 --- a/db/types.go +++ b/db/types.go @@ -90,17 +90,11 @@ type Iterator interface { Valid() bool // Next moves the iterator to the next sequential key in the database, as - // defined by the Comparator in the ReadOptions used to create this Iterator. + // defined by order of iteration. // // If Valid returns false, this method will panic. Next() - // Prev moves the iterator to the previous sequential key in the database, as - // defined by the Comparator in the ReadOptions used to create this Iterator. - // - // If Valid returns false, this method will panic. - Prev() - // Key returns the key of the cursor. // // If Valid returns false, this method will panic. @@ -120,3 +114,8 @@ type Iterator interface { // Release deallocates the given Iterator. Release() } + +// For testing convenience. +func bz(s string) []byte { + return []byte(s) +} diff --git a/db/util_test.go b/db/util_test.go index 55a41bf5b..4f8b9c456 100644 --- a/db/util_test.go +++ b/db/util_test.go @@ -66,7 +66,6 @@ func TestPrefixIteratorMatches1N(t *testing.T) { db.SetSync(bz("a/1"), bz("value_1")) db.SetSync(bz("a/3"), bz("value_3")) itr := IteratePrefix(db, []byte("a/")) - itr.Seek(bz("a/1")) checkValid(t, itr, true) checkItem(t, itr, bz("a/1"), bz("value_1")) @@ -82,32 +81,6 @@ func TestPrefixIteratorMatches1N(t *testing.T) { } } -// Search for a/1, fail by too much Prev() -func TestPrefixIteratorMatches1P(t *testing.T) { - for backend, _ := range backends { - t.Run(fmt.Sprintf("Prefix w/ backend %s", backend), func(t *testing.T) { - db := newTempDB(t, backend) - db.SetSync(bz("a/1"), bz("value_1")) - db.SetSync(bz("a/3"), bz("value_3")) - itr := IteratePrefix(db, []byte("a/")) - itr.Seek(bz("a/1")) - - checkValid(t, itr, true) - checkItem(t, itr, bz("a/1"), bz("value_1")) - checkNext(t, itr, true) - checkItem(t, itr, bz("a/3"), bz("value_3")) - checkPrev(t, itr, true) - checkItem(t, itr, bz("a/1"), bz("value_1")) - - // Bad! - checkPrev(t, itr, false) - - // Once invalid... - checkInvalid(t, itr) - }) - } -} - // Search for a/2, fail by too much Next() func TestPrefixIteratorMatches2N(t *testing.T) { for backend, _ := range backends { @@ -116,13 +89,11 @@ func TestPrefixIteratorMatches2N(t *testing.T) { db.SetSync(bz("a/1"), bz("value_1")) db.SetSync(bz("a/3"), bz("value_3")) itr := IteratePrefix(db, []byte("a/")) - itr.Seek(bz("a/2")) checkValid(t, itr, true) - checkItem(t, itr, bz("a/3"), bz("value_3")) - checkPrev(t, itr, true) checkItem(t, itr, bz("a/1"), bz("value_1")) checkNext(t, itr, true) + checkValid(t, itr, true) checkItem(t, itr, bz("a/3"), bz("value_3")) // Bad! @@ -134,30 +105,6 @@ func TestPrefixIteratorMatches2N(t *testing.T) { } } -// Search for a/2, fail by too much Prev() -func TestPrefixIteratorMatches2P(t *testing.T) { - for backend, _ := range backends { - t.Run(fmt.Sprintf("Prefix w/ backend %s", backend), func(t *testing.T) { - db := newTempDB(t, backend) - db.SetSync(bz("a/1"), bz("value_1")) - db.SetSync(bz("a/3"), bz("value_3")) - itr := IteratePrefix(db, []byte("a/")) - itr.Seek(bz("a/2")) - - checkValid(t, itr, true) - checkItem(t, itr, bz("a/3"), bz("value_3")) - checkPrev(t, itr, true) - checkItem(t, itr, bz("a/1"), bz("value_1")) - - // Bad! - checkPrev(t, itr, false) - - // Once invalid... - checkInvalid(t, itr) - }) - } -} - // Search for a/3, fail by too much Next() func TestPrefixIteratorMatches3N(t *testing.T) { for backend, _ := range backends { @@ -166,11 +113,8 @@ func TestPrefixIteratorMatches3N(t *testing.T) { db.SetSync(bz("a/1"), bz("value_1")) db.SetSync(bz("a/3"), bz("value_3")) itr := IteratePrefix(db, []byte("a/")) - itr.Seek(bz("a/3")) checkValid(t, itr, true) - checkItem(t, itr, bz("a/3"), bz("value_3")) - checkPrev(t, itr, true) checkItem(t, itr, bz("a/1"), bz("value_1")) checkNext(t, itr, true) checkItem(t, itr, bz("a/3"), bz("value_3")) @@ -183,27 +127,3 @@ func TestPrefixIteratorMatches3N(t *testing.T) { }) } } - -// Search for a/3, fail by too much Prev() -func TestPrefixIteratorMatches3P(t *testing.T) { - for backend, _ := range backends { - t.Run(fmt.Sprintf("Prefix w/ backend %s", backend), func(t *testing.T) { - db := newTempDB(t, backend) - db.SetSync(bz("a/1"), bz("value_1")) - db.SetSync(bz("a/3"), bz("value_3")) - itr := IteratePrefix(db, []byte("a/")) - itr.Seek(bz("a/3")) - - checkValid(t, itr, true) - checkItem(t, itr, bz("a/3"), bz("value_3")) - checkPrev(t, itr, true) - checkItem(t, itr, bz("a/1"), bz("value_1")) - - // Bad! - checkPrev(t, itr, false) - - // Once invalid... - checkInvalid(t, itr) - }) - } -} From 781f6c5d228c34227dcd059673175ea86398218f Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Tue, 12 Dec 2017 18:04:05 -0500 Subject: [PATCH 04/19] db: some comments in types.go --- db/types.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/db/types.go b/db/types.go index 8306813c7..a6edbdd85 100644 --- a/db/types.go +++ b/db/types.go @@ -54,10 +54,12 @@ type SetDeleter interface { //---------------------------------------- +// BeginningKey is the smallest key. func BeginningKey() []byte { return []byte{} } +// EndingKey is the largest key. func EndingKey() []byte { return nil } @@ -81,8 +83,8 @@ type Iterator interface { // A domain of ([]byte{12, 13}, []byte{12, 14}) will iterate // over anything with the prefix []byte{12, 13}. // - // The smallest key is the empty byte array []byte{}. - // The largest key is the nil byte array []byte(nil). + // The smallest key is the empty byte array []byte{} - see BeginningKey(). + // The largest key is the nil byte array []byte(nil) - see EndingKey(). Domain() (start []byte, end []byte) // Valid returns whether the current position is valid. @@ -100,7 +102,7 @@ type Iterator interface { // If Valid returns false, this method will panic. Key() []byte - // Value returns the key of the cursor. + // Value returns the value of the cursor. // // If Valid returns false, this method will panic. Value() []byte From 5b7f90dfb258c4b26f8209b42181a814b6978eb1 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Tue, 12 Dec 2017 18:42:47 -0500 Subject: [PATCH 05/19] db: test panic on nil key --- db/backend_test.go | 59 +++++++++++++++++++++++++++++++++++++--------- db/fsdb.go | 8 +++++++ db/go_level_db.go | 6 +++++ db/mem_db.go | 14 ++++++----- db/types.go | 7 ++++++ 5 files changed, 77 insertions(+), 17 deletions(-) diff --git a/db/backend_test.go b/db/backend_test.go index b4ffecdc6..b21ce0037 100644 --- a/db/backend_test.go +++ b/db/backend_test.go @@ -2,42 +2,79 @@ package db import ( "fmt" + "os" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" cmn "github.com/tendermint/tmlibs/common" ) -func testBackend(t *testing.T, backend string) { +func testBackendGetSetDelete(t *testing.T, backend string) { // Default dir, dirname := cmn.Tempdir(fmt.Sprintf("test_backend_%s_", backend)) defer dir.Close() db := NewDB("testdb", backend, dirname) require.Nil(t, db.Get([]byte(""))) - require.Nil(t, db.Get(nil)) // Set empty ("") db.Set([]byte(""), []byte("")) require.NotNil(t, db.Get([]byte(""))) - require.NotNil(t, db.Get(nil)) require.Empty(t, db.Get([]byte(""))) - require.Empty(t, db.Get(nil)) // Set empty (nil) db.Set([]byte(""), nil) require.NotNil(t, db.Get([]byte(""))) - require.NotNil(t, db.Get(nil)) require.Empty(t, db.Get([]byte(""))) - require.Empty(t, db.Get(nil)) // Delete db.Delete([]byte("")) require.Nil(t, db.Get([]byte(""))) - require.Nil(t, db.Get(nil)) } -func TestBackends(t *testing.T) { - testBackend(t, CLevelDBBackendStr) - testBackend(t, GoLevelDBBackendStr) - testBackend(t, MemDBBackendStr) +func TestBackendsGetSetDelete(t *testing.T) { + for dbType, _ := range backends { + if dbType == "fsdb" { + // TODO: handle + // fsdb cant deal with length 0 keys + continue + } + testBackendGetSetDelete(t, dbType) + } +} + +func assertPanics(t *testing.T, dbType, name string, fn func()) { + defer func() { + r := recover() + assert.NotNil(t, r, cmn.Fmt("expecting %s.%s to panic", dbType, name)) + }() + + fn() +} + +func TestBackendsNilKeys(t *testing.T) { + // test all backends + for dbType, creator := range backends { + name := cmn.Fmt("test_%x", cmn.RandStr(12)) + db, err := creator(name, "") + assert.Nil(t, err) + defer os.RemoveAll(name) + + assertPanics(t, dbType, "get", func() { db.Get(nil) }) + assertPanics(t, dbType, "has", func() { db.Has(nil) }) + assertPanics(t, dbType, "set", func() { db.Set(nil, []byte("abc")) }) + assertPanics(t, dbType, "setsync", func() { db.SetSync(nil, []byte("abc")) }) + assertPanics(t, dbType, "delete", func() { db.Delete(nil) }) + assertPanics(t, dbType, "deletesync", func() { db.DeleteSync(nil) }) + + db.Close() + } +} + +func TestLevelDBBackendStr(t *testing.T) { + name := cmn.Fmt("test_%x", cmn.RandStr(12)) + db := NewDB(name, LevelDBBackendStr, "") + defer os.RemoveAll(name) + _, ok := db.(*GoLevelDB) + assert.True(t, ok) } diff --git a/db/fsdb.go b/db/fsdb.go index b6e08daf5..19ea9fa3c 100644 --- a/db/fsdb.go +++ b/db/fsdb.go @@ -44,6 +44,7 @@ func NewFSDB(dir string) *FSDB { func (db *FSDB) Get(key []byte) []byte { db.mtx.Lock() defer db.mtx.Unlock() + panicNilKey(key) path := db.nameToPath(key) value, err := read(path) @@ -58,6 +59,7 @@ func (db *FSDB) Get(key []byte) []byte { func (db *FSDB) Has(key []byte) bool { db.mtx.Lock() defer db.mtx.Unlock() + panicNilKey(key) path := db.nameToPath(key) _, err := read(path) @@ -72,6 +74,7 @@ func (db *FSDB) Has(key []byte) bool { func (db *FSDB) Set(key []byte, value []byte) { db.mtx.Lock() defer db.mtx.Unlock() + panicNilKey(key) db.SetNoLock(key, value) } @@ -79,12 +82,14 @@ func (db *FSDB) Set(key []byte, value []byte) { func (db *FSDB) SetSync(key []byte, value []byte) { db.mtx.Lock() defer db.mtx.Unlock() + panicNilKey(key) db.SetNoLock(key, value) } // NOTE: Implements atomicSetDeleter. func (db *FSDB) SetNoLock(key []byte, value []byte) { + panicNilKey(key) if value == nil { value = []byte{} } @@ -98,6 +103,7 @@ func (db *FSDB) SetNoLock(key []byte, value []byte) { func (db *FSDB) Delete(key []byte) { db.mtx.Lock() defer db.mtx.Unlock() + panicNilKey(key) db.DeleteNoLock(key) } @@ -105,12 +111,14 @@ func (db *FSDB) Delete(key []byte) { func (db *FSDB) DeleteSync(key []byte) { db.mtx.Lock() defer db.mtx.Unlock() + panicNilKey(key) db.DeleteNoLock(key) } // NOTE: Implements atomicSetDeleter. func (db *FSDB) DeleteNoLock(key []byte) { + panicNilKey(key) err := remove(string(key)) if os.IsNotExist(err) { return diff --git a/db/go_level_db.go b/db/go_level_db.go index e8ed99dee..201a31949 100644 --- a/db/go_level_db.go +++ b/db/go_level_db.go @@ -37,6 +37,7 @@ func NewGoLevelDB(name string, dir string) (*GoLevelDB, error) { } func (db *GoLevelDB) Get(key []byte) []byte { + panicNilKey(key) res, err := db.db.Get(key, nil) if err != nil { if err == errors.ErrNotFound { @@ -49,6 +50,7 @@ func (db *GoLevelDB) Get(key []byte) []byte { } func (db *GoLevelDB) Has(key []byte) bool { + panicNilKey(key) _, err := db.db.Get(key, nil) if err != nil { if err == errors.ErrNotFound { @@ -61,6 +63,7 @@ func (db *GoLevelDB) Has(key []byte) bool { } func (db *GoLevelDB) Set(key []byte, value []byte) { + panicNilKey(key) err := db.db.Put(key, value, nil) if err != nil { PanicCrisis(err) @@ -68,6 +71,7 @@ func (db *GoLevelDB) Set(key []byte, value []byte) { } func (db *GoLevelDB) SetSync(key []byte, value []byte) { + panicNilKey(key) err := db.db.Put(key, value, &opt.WriteOptions{Sync: true}) if err != nil { PanicCrisis(err) @@ -75,6 +79,7 @@ func (db *GoLevelDB) SetSync(key []byte, value []byte) { } func (db *GoLevelDB) Delete(key []byte) { + panicNilKey(key) err := db.db.Delete(key, nil) if err != nil { PanicCrisis(err) @@ -82,6 +87,7 @@ func (db *GoLevelDB) Delete(key []byte) { } func (db *GoLevelDB) DeleteSync(key []byte) { + panicNilKey(key) err := db.db.Delete(key, &opt.WriteOptions{Sync: true}) if err != nil { PanicCrisis(err) diff --git a/db/mem_db.go b/db/mem_db.go index 3127030ae..ebeb2dded 100644 --- a/db/mem_db.go +++ b/db/mem_db.go @@ -27,14 +27,14 @@ func NewMemDB() *MemDB { func (db *MemDB) Get(key []byte) []byte { db.mtx.Lock() defer db.mtx.Unlock() - + panicNilKey(key) return db.db[string(key)] } func (db *MemDB) Has(key []byte) bool { db.mtx.Lock() defer db.mtx.Unlock() - + panicNilKey(key) _, ok := db.db[string(key)] return ok } @@ -42,14 +42,14 @@ func (db *MemDB) Has(key []byte) bool { func (db *MemDB) Set(key []byte, value []byte) { db.mtx.Lock() defer db.mtx.Unlock() - + panicNilKey(key) db.SetNoLock(key, value) } func (db *MemDB) SetSync(key []byte, value []byte) { db.mtx.Lock() defer db.mtx.Unlock() - + panicNilKey(key) db.SetNoLock(key, value) } @@ -58,25 +58,27 @@ func (db *MemDB) SetNoLock(key []byte, value []byte) { if value == nil { value = []byte{} } + panicNilKey(key) db.db[string(key)] = value } func (db *MemDB) Delete(key []byte) { db.mtx.Lock() defer db.mtx.Unlock() - + panicNilKey(key) delete(db.db, string(key)) } func (db *MemDB) DeleteSync(key []byte) { db.mtx.Lock() defer db.mtx.Unlock() - + panicNilKey(key) delete(db.db, string(key)) } // NOTE: Implements atomicSetDeleter func (db *MemDB) DeleteNoLock(key []byte) { + panicNilKey(key) delete(db.db, string(key)) } diff --git a/db/types.go b/db/types.go index a6edbdd85..54c1025a0 100644 --- a/db/types.go +++ b/db/types.go @@ -121,3 +121,10 @@ type Iterator interface { func bz(s string) []byte { return []byte(s) } + +// All DB funcs should panic on nil key. +func panicNilKey(key []byte) { + if key == nil { + panic("nil key") + } +} From c547caf04f17dee0390733fc9167e68975aecdb9 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Tue, 12 Dec 2017 19:08:22 -0500 Subject: [PATCH 06/19] db: some test cleanup --- db/backend_test.go | 14 +++++++++----- db/c_level_db_test.go | 11 +++++++++++ db/db.go | 2 +- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/db/backend_test.go b/db/backend_test.go index b21ce0037..3d10c66cb 100644 --- a/db/backend_test.go +++ b/db/backend_test.go @@ -58,7 +58,6 @@ func TestBackendsNilKeys(t *testing.T) { name := cmn.Fmt("test_%x", cmn.RandStr(12)) db, err := creator(name, "") assert.Nil(t, err) - defer os.RemoveAll(name) assertPanics(t, dbType, "get", func() { db.Get(nil) }) assertPanics(t, dbType, "has", func() { db.Has(nil) }) @@ -68,13 +67,18 @@ func TestBackendsNilKeys(t *testing.T) { assertPanics(t, dbType, "deletesync", func() { db.DeleteSync(nil) }) db.Close() + err = os.RemoveAll(name + ".db") + assert.Nil(t, err) } } -func TestLevelDBBackendStr(t *testing.T) { +func TestGoLevelDBBackendStr(t *testing.T) { name := cmn.Fmt("test_%x", cmn.RandStr(12)) db := NewDB(name, LevelDBBackendStr, "") - defer os.RemoveAll(name) - _, ok := db.(*GoLevelDB) - assert.True(t, ok) + defer os.RemoveAll(name + ".db") + + if _, ok := backends[CLevelDBBackendStr]; !ok { + _, ok := db.(*GoLevelDB) + assert.True(t, ok) + } } diff --git a/db/c_level_db_test.go b/db/c_level_db_test.go index 864362332..606c34519 100644 --- a/db/c_level_db_test.go +++ b/db/c_level_db_test.go @@ -5,8 +5,10 @@ package db import ( "bytes" "fmt" + "os" "testing" + "github.com/stretchr/testify/assert" cmn "github.com/tendermint/tmlibs/common" ) @@ -84,3 +86,12 @@ func bytes2Int64(buf []byte) int64 { return int64(binary.BigEndian.Uint64(buf)) } */ + +func TestCLevelDBBackendStr(t *testing.T) { + name := cmn.Fmt("test_%x", cmn.RandStr(12)) + db := NewDB(name, LevelDBBackendStr, "") + defer os.RemoveAll(name) + + _, ok := db.(*CLevelDB) + assert.True(t, ok) +} diff --git a/db/db.go b/db/db.go index 7eec04d56..b43b06554 100644 --- a/db/db.go +++ b/db/db.go @@ -6,7 +6,7 @@ import "fmt" // Main entry const ( - LevelDBBackendStr = "leveldb" // legacy, defaults to goleveldb. + LevelDBBackendStr = "leveldb" // legacy, defaults to goleveldb unless +gcc CLevelDBBackendStr = "cleveldb" GoLevelDBBackendStr = "goleveldb" MemDBBackendStr = "memdb" From ba8c5045b5c67df9fbba08974d0194b6e735cbd2 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Tue, 12 Dec 2017 19:22:08 -0500 Subject: [PATCH 07/19] db: fixes to fsdb and clevledb --- db/backend_test.go | 24 ++++++++++-------------- db/c_level_db.go | 6 ++++++ db/fsdb.go | 5 +++-- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/db/backend_test.go b/db/backend_test.go index 3d10c66cb..9dc17201a 100644 --- a/db/backend_test.go +++ b/db/backend_test.go @@ -15,30 +15,26 @@ func testBackendGetSetDelete(t *testing.T, backend string) { dir, dirname := cmn.Tempdir(fmt.Sprintf("test_backend_%s_", backend)) defer dir.Close() db := NewDB("testdb", backend, dirname) - require.Nil(t, db.Get([]byte(""))) + key := []byte("abc") + require.Nil(t, db.Get(key)) // Set empty ("") - db.Set([]byte(""), []byte("")) - require.NotNil(t, db.Get([]byte(""))) - require.Empty(t, db.Get([]byte(""))) + db.Set(key, []byte("")) + require.NotNil(t, db.Get(key)) + require.Empty(t, db.Get(key)) // Set empty (nil) - db.Set([]byte(""), nil) - require.NotNil(t, db.Get([]byte(""))) - require.Empty(t, db.Get([]byte(""))) + db.Set(key, nil) + require.NotNil(t, db.Get(key)) + require.Empty(t, db.Get(key)) // Delete - db.Delete([]byte("")) - require.Nil(t, db.Get([]byte(""))) + db.Delete(key) + require.Nil(t, db.Get(key)) } func TestBackendsGetSetDelete(t *testing.T) { for dbType, _ := range backends { - if dbType == "fsdb" { - // TODO: handle - // fsdb cant deal with length 0 keys - continue - } testBackendGetSetDelete(t, dbType) } } diff --git a/db/c_level_db.go b/db/c_level_db.go index 11a6e5ff7..47e79dfa6 100644 --- a/db/c_level_db.go +++ b/db/c_level_db.go @@ -48,6 +48,7 @@ func NewCLevelDB(name string, dir string) (*CLevelDB, error) { } func (db *CLevelDB) Get(key []byte) []byte { + panicNilKey(key) res, err := db.db.Get(db.ro, key) if err != nil { panic(err) @@ -56,10 +57,12 @@ func (db *CLevelDB) Get(key []byte) []byte { } func (db *CLevelDB) Has(key []byte) bool { + panicNilKey(key) panic("not implemented yet") } func (db *CLevelDB) Set(key []byte, value []byte) { + panicNilKey(key) err := db.db.Put(db.wo, key, value) if err != nil { panic(err) @@ -67,6 +70,7 @@ func (db *CLevelDB) Set(key []byte, value []byte) { } func (db *CLevelDB) SetSync(key []byte, value []byte) { + panicNilKey(key) err := db.db.Put(db.woSync, key, value) if err != nil { panic(err) @@ -74,6 +78,7 @@ func (db *CLevelDB) SetSync(key []byte, value []byte) { } func (db *CLevelDB) Delete(key []byte) { + panicNilKey(key) err := db.db.Delete(db.wo, key) if err != nil { panic(err) @@ -81,6 +86,7 @@ func (db *CLevelDB) Delete(key []byte) { } func (db *CLevelDB) DeleteSync(key []byte) { + panicNilKey(key) err := db.db.Delete(db.woSync, key) if err != nil { panic(err) diff --git a/db/fsdb.go b/db/fsdb.go index 19ea9fa3c..116dc3eef 100644 --- a/db/fsdb.go +++ b/db/fsdb.go @@ -119,7 +119,8 @@ func (db *FSDB) DeleteSync(key []byte) { // NOTE: Implements atomicSetDeleter. func (db *FSDB) DeleteNoLock(key []byte) { panicNilKey(key) - err := remove(string(key)) + path := db.nameToPath(key) + err := remove(path) if os.IsNotExist(err) { return } else if err != nil { @@ -210,7 +211,7 @@ func read(path string) ([]byte, error) { // Write some bytes from a file. // CONTRACT: returns os errors directly without wrapping. func write(path string, d []byte) error { - f, err := os.OpenFile(path, os.O_CREATE|os.O_EXCL|os.O_WRONLY, keyPerm) + f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, keyPerm) if err != nil { return err } From bb115d4d615bdbe04e664b76346900151f83729e Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Tue, 12 Dec 2017 19:28:52 -0500 Subject: [PATCH 08/19] cleanupDBDir --- db/backend_test.go | 11 ++++++++--- db/c_level_db_test.go | 3 +-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/db/backend_test.go b/db/backend_test.go index 9dc17201a..16649cd20 100644 --- a/db/backend_test.go +++ b/db/backend_test.go @@ -3,6 +3,7 @@ package db import ( "fmt" "os" + "path" "testing" "github.com/stretchr/testify/assert" @@ -10,11 +11,16 @@ import ( cmn "github.com/tendermint/tmlibs/common" ) +func cleanupDBDir(dir, name string) { + os.RemoveAll(path.Join(dir, name) + ".db") +} + func testBackendGetSetDelete(t *testing.T, backend string) { // Default dir, dirname := cmn.Tempdir(fmt.Sprintf("test_backend_%s_", backend)) defer dir.Close() db := NewDB("testdb", backend, dirname) + key := []byte("abc") require.Nil(t, db.Get(key)) @@ -53,6 +59,7 @@ func TestBackendsNilKeys(t *testing.T) { for dbType, creator := range backends { name := cmn.Fmt("test_%x", cmn.RandStr(12)) db, err := creator(name, "") + defer cleanupDBDir("", name) assert.Nil(t, err) assertPanics(t, dbType, "get", func() { db.Get(nil) }) @@ -63,15 +70,13 @@ func TestBackendsNilKeys(t *testing.T) { assertPanics(t, dbType, "deletesync", func() { db.DeleteSync(nil) }) db.Close() - err = os.RemoveAll(name + ".db") - assert.Nil(t, err) } } func TestGoLevelDBBackendStr(t *testing.T) { name := cmn.Fmt("test_%x", cmn.RandStr(12)) db := NewDB(name, LevelDBBackendStr, "") - defer os.RemoveAll(name + ".db") + defer cleanupDBDir("", name) if _, ok := backends[CLevelDBBackendStr]; !ok { _, ok := db.(*GoLevelDB) diff --git a/db/c_level_db_test.go b/db/c_level_db_test.go index 606c34519..89993fbac 100644 --- a/db/c_level_db_test.go +++ b/db/c_level_db_test.go @@ -5,7 +5,6 @@ package db import ( "bytes" "fmt" - "os" "testing" "github.com/stretchr/testify/assert" @@ -90,7 +89,7 @@ func bytes2Int64(buf []byte) int64 { func TestCLevelDBBackendStr(t *testing.T) { name := cmn.Fmt("test_%x", cmn.RandStr(12)) db := NewDB(name, LevelDBBackendStr, "") - defer os.RemoveAll(name) + defer cleanupDBDir("", name) _, ok := db.(*CLevelDB) assert.True(t, ok) From 39e40ff5ce8dd496475db872426cd7d5860b2a05 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Tue, 12 Dec 2017 20:06:50 -0500 Subject: [PATCH 09/19] db: memdb iterator --- db/mem_db.go | 90 ++++++++++++++++++++++++++++++----------------- db/mem_db_test.go | 2 +- 2 files changed, 58 insertions(+), 34 deletions(-) diff --git a/db/mem_db.go b/db/mem_db.go index ebeb2dded..84d14de98 100644 --- a/db/mem_db.go +++ b/db/mem_db.go @@ -3,6 +3,8 @@ package db import ( "bytes" "fmt" + "sort" + "strings" "sync" ) @@ -12,6 +14,8 @@ func init() { }, false) } +var _ DB = (*MemDB)(nil) + type MemDB struct { mtx sync.Mutex db map[string][]byte @@ -123,49 +127,67 @@ func (db *MemDB) Mutex() *sync.Mutex { //---------------------------------------- func (db *MemDB) Iterator(start, end []byte) Iterator { - /* - XXX - it := newMemDBIterator() - it.db = db - it.cur = 0 + it := newMemDBIterator(db, start, end) - db.mtx.Lock() - defer db.mtx.Unlock() + db.mtx.Lock() + defer db.mtx.Unlock() - // We need a copy of all of the keys. - // Not the best, but probably not a bottleneck depending. - for key, _ := range db.db { - it.keys = append(it.keys, key) - } - sort.Strings(it.keys) - return it - */ - return nil + // We need a copy of all of the keys. + // Not the best, but probably not a bottleneck depending. + it.keys = db.getSortedKeys(start, end) + return it } func (db *MemDB) ReverseIterator(start, end []byte) Iterator { - // XXX + it := newMemDBIterator(db, start, end) + + db.mtx.Lock() + defer db.mtx.Unlock() + + // We need a copy of all of the keys. + // Not the best, but probably not a bottleneck depending. + it.keys = db.getSortedKeys(end, start) + // reverse the order + l := len(it.keys) - 1 + for i, v := range it.keys { + it.keys[i] = it.keys[l-i] + it.keys[l-i] = v + } return nil } -type memDBIterator struct { - cur int - keys []string - db DB -} - -func newMemDBIterator() *memDBIterator { - return &memDBIterator{} -} - -func (it *memDBIterator) Seek(key []byte) { - for i, ik := range it.keys { - it.cur = i - if bytes.Compare(key, []byte(ik)) <= 0 { - return +func (db *MemDB) getSortedKeys(start, end []byte) []string { + keys := []string{} + for key, _ := range db.db { + leftCondition := bytes.Equal(start, BeginningKey()) || strings.Compare(key, string(start)) >= 0 + rightCondition := bytes.Equal(end, EndingKey()) || strings.Compare(key, string(end)) < 0 + if leftCondition && rightCondition { + keys = append(keys, key) } } - it.cur += 1 // If not found, becomes invalid. + sort.Strings(keys) + return keys +} + +var _ Iterator = (*memDBIterator)(nil) + +type memDBIterator struct { + cur int + keys []string + db DB + start, end []byte +} + +func newMemDBIterator(db DB, start, end []byte) *memDBIterator { + return &memDBIterator{ + db: db, + start: start, + end: end, + } +} + +func (it *memDBIterator) Domain() ([]byte, []byte) { + return it.start, it.end } func (it *memDBIterator) Valid() bool { @@ -208,3 +230,5 @@ func (it *memDBIterator) Close() { func (it *memDBIterator) GetError() error { return nil } + +func (it *memDBIterator) Release() {} diff --git a/db/mem_db_test.go b/db/mem_db_test.go index 42e242857..a08a3679b 100644 --- a/db/mem_db_test.go +++ b/db/mem_db_test.go @@ -7,7 +7,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestMemDbIterator(t *testing.T) { +func TestMemDBIterator(t *testing.T) { db := NewMemDB() keys := make([][]byte, 100) for i := 0; i < 100; i++ { From 3e5dbef6a216a8fcd24ac72c6f65f793753d1a4b Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Tue, 12 Dec 2017 20:38:03 -0500 Subject: [PATCH 10/19] db: goleveldb iterator --- db/common_test.go | 5 ----- db/go_level_db.go | 37 ++++++++++++++++++++++--------------- db/util_test.go | 2 +- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/db/common_test.go b/db/common_test.go index 09fad8424..59c86a2c7 100644 --- a/db/common_test.go +++ b/db/common_test.go @@ -80,11 +80,6 @@ func TestDBIteratorTwoKeys(t *testing.T) { itr := db.Iterator(BeginningKey(), EndingKey()) checkValid(t, itr, true) - for i := 0; i < 10; i++ { - checkNext(t, itr, true) - checkValid(t, itr, true) - } - checkNext(t, itr, true) checkValid(t, itr, true) diff --git a/db/go_level_db.go b/db/go_level_db.go index 201a31949..45b437d2b 100644 --- a/db/go_level_db.go +++ b/db/go_level_db.go @@ -20,6 +20,8 @@ func init() { registerDBCreator(GoLevelDBBackendStr, dbCreator, false) } +var _ DB = (*GoLevelDB)(nil) + type GoLevelDB struct { db *leveldb.DB } @@ -168,15 +170,13 @@ func (mBatch *goLevelDBBatch) Write() { // Iterator func (db *GoLevelDB) Iterator(start, end []byte) Iterator { - /* - XXX - itr := &goLevelDBIterator{ - source: db.db.NewIterator(nil, nil), - } - itr.Seek(nil) - return itr - */ - return nil + itr := &goLevelDBIterator{ + source: db.db.NewIterator(nil, nil), + start: start, + end: end, + } + itr.source.Seek(start) + return itr } func (db *GoLevelDB) ReverseIterator(start, end []byte) Iterator { @@ -184,9 +184,16 @@ func (db *GoLevelDB) ReverseIterator(start, end []byte) Iterator { return nil } +var _ Iterator = (*goLevelDBIterator)(nil) + type goLevelDBIterator struct { - source iterator.Iterator - invalid bool + source iterator.Iterator + invalid bool + start, end []byte +} + +func (it *goLevelDBIterator) Domain() ([]byte, []byte) { + return it.start, it.end } // Key returns a copy of the current key. @@ -217,10 +224,6 @@ func (it *goLevelDBIterator) GetError() error { return it.source.Error() } -func (it *goLevelDBIterator) Seek(key []byte) { - it.source.Seek(key) -} - func (it *goLevelDBIterator) Valid() bool { if it.invalid { return false @@ -246,3 +249,7 @@ func (it *goLevelDBIterator) Prev() { func (it *goLevelDBIterator) Close() { it.source.Release() } + +func (it *goLevelDBIterator) Release() { + it.source.Release() +} diff --git a/db/util_test.go b/db/util_test.go index 4f8b9c456..a0ce9cd51 100644 --- a/db/util_test.go +++ b/db/util_test.go @@ -50,7 +50,7 @@ func TestPrefixIteratorMatch3(t *testing.T) { t.Run(fmt.Sprintf("Prefix w/ backend %s", backend), func(t *testing.T) { db := newTempDB(t, backend) db.SetSync(bz("3"), bz("value_3")) - itr := IteratePrefix(db, []byte("2")) + itr := IteratePrefix(db, []byte("4")) // Once invalid... checkInvalid(t, itr) From bcacaf164b8b79cc09ff2abec2ff4ec212315aba Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Tue, 12 Dec 2017 20:58:35 -0500 Subject: [PATCH 11/19] db: cleveldb iterator --- db/c_level_db.go | 32 +++++++++++++++++++------------- db/common_test.go | 5 +++-- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/db/c_level_db.go b/db/c_level_db.go index 47e79dfa6..527fd7da3 100644 --- a/db/c_level_db.go +++ b/db/c_level_db.go @@ -17,6 +17,8 @@ func init() { registerDBCreator(CLevelDBBackendStr, dbCreator, false) } +var _ DB = (*CLevelDB)(nil) + type CLevelDB struct { db *levigo.DB ro *levigo.ReadOptions @@ -158,13 +160,17 @@ func (mBatch *cLevelDBBatch) Write() { // Iterator func (db *CLevelDB) Iterator(start, end []byte) Iterator { - /* - XXX - itr := db.db.NewIterator(db.ro) - itr.Seek([]byte{0x00}) - return cLevelDBIterator{itr} - */ - return nil + itr := db.db.NewIterator(db.ro) + if len(start) > 0 { + itr.Seek(start) + } else { + itr.SeekToFirst() + } + return cLevelDBIterator{ + itr: itr, + start: start, + end: end, + } } func (db *CLevelDB) ReverseIterator(start, end []byte) Iterator { @@ -172,15 +178,15 @@ func (db *CLevelDB) ReverseIterator(start, end []byte) Iterator { return nil } +var _ Iterator = (*cLevelDBIterator)(nil) + type cLevelDBIterator struct { - itr *levigo.Iterator + itr *levigo.Iterator + start, end []byte } -func (c cLevelDBIterator) Seek(key []byte) { - if key == nil { - key = []byte{0x00} - } - c.itr.Seek(key) +func (c cLevelDBIterator) Domain() ([]byte, []byte) { + return c.start, c.end } func (c cLevelDBIterator) Valid() bool { diff --git a/db/common_test.go b/db/common_test.go index 59c86a2c7..6b3009795 100644 --- a/db/common_test.go +++ b/db/common_test.go @@ -5,18 +5,19 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" cmn "github.com/tendermint/tmlibs/common" ) func checkValid(t *testing.T, itr Iterator, expected bool) { valid := itr.Valid() - assert.Equal(t, expected, valid) + require.Equal(t, expected, valid) } func checkNext(t *testing.T, itr Iterator, expected bool) { itr.Next() valid := itr.Valid() - assert.Equal(t, expected, valid) + require.Equal(t, expected, valid) } func checkNextPanics(t *testing.T, itr Iterator) { From edf07760d6f45663a992cf8a0978521084a6c597 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Tue, 12 Dec 2017 21:08:38 -0500 Subject: [PATCH 12/19] db: fsdb iterator --- db/fsdb.go | 44 ++++++++++++++++++++++---------------------- db/mem_db.go | 6 +----- db/util.go | 11 +++++++++++ 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/db/fsdb.go b/db/fsdb.go index 116dc3eef..ac9cdd741 100644 --- a/db/fsdb.go +++ b/db/fsdb.go @@ -7,6 +7,7 @@ import ( "os" "path" "path/filepath" + "sort" "sync" "github.com/pkg/errors" @@ -24,6 +25,8 @@ func init() { }, false) } +var _ DB = (*FSDB)(nil) + // It's slow. type FSDB struct { mtx sync.Mutex @@ -160,26 +163,20 @@ func (db *FSDB) Mutex() *sync.Mutex { } func (db *FSDB) Iterator(start, end []byte) Iterator { - /* - XXX - it := newMemDBIterator() - it.db = db - it.cur = 0 + it := newMemDBIterator(db, start, end) - db.mtx.Lock() - defer db.mtx.Unlock() + db.mtx.Lock() + defer db.mtx.Unlock() - // We need a copy of all of the keys. - // Not the best, but probably not a bottleneck depending. - keys, err := list(db.dir) - if err != nil { - panic(errors.Wrap(err, fmt.Sprintf("Listing keys in %s", db.dir))) - } - sort.Strings(keys) - it.keys = keys - return it - */ - return nil + // We need a copy of all of the keys. + // Not the best, but probably not a bottleneck depending. + keys, err := list(db.dir, start, end) + if err != nil { + panic(errors.Wrap(err, fmt.Sprintf("Listing keys in %s", db.dir))) + } + sort.Strings(keys) + it.keys = keys + return it } func (db *FSDB) ReverseIterator(start, end []byte) Iterator { @@ -233,7 +230,7 @@ func remove(path string) error { // List files of a path. // Paths will NOT include dir as the prefix. // CONTRACT: returns os errors directly without wrapping. -func list(dirPath string) (paths []string, err error) { +func list(dirPath string, start, end []byte) ([]string, error) { dir, err := os.Open(dirPath) if err != nil { return nil, err @@ -244,12 +241,15 @@ func list(dirPath string) (paths []string, err error) { if err != nil { return nil, err } - for i, name := range names { + var paths []string + for _, name := range names { n, err := url.PathUnescape(name) if err != nil { return nil, fmt.Errorf("Failed to unescape %s while listing", name) } - names[i] = n + if checkKeyCondition(n, start, end) { + paths = append(paths, n) + } } - return names, nil + return paths, nil } diff --git a/db/mem_db.go b/db/mem_db.go index 84d14de98..a9f21d526 100644 --- a/db/mem_db.go +++ b/db/mem_db.go @@ -1,10 +1,8 @@ package db import ( - "bytes" "fmt" "sort" - "strings" "sync" ) @@ -159,9 +157,7 @@ func (db *MemDB) ReverseIterator(start, end []byte) Iterator { func (db *MemDB) getSortedKeys(start, end []byte) []string { keys := []string{} for key, _ := range db.db { - leftCondition := bytes.Equal(start, BeginningKey()) || strings.Compare(key, string(start)) >= 0 - rightCondition := bytes.Equal(end, EndingKey()) || strings.Compare(key, string(end)) < 0 - if leftCondition && rightCondition { + if checkKeyCondition(key, start, end) { keys = append(keys, key) } } diff --git a/db/util.go b/db/util.go index 89c777622..02f4a52f0 100644 --- a/db/util.go +++ b/db/util.go @@ -1,5 +1,10 @@ package db +import ( + "bytes" + "strings" +) + func IteratePrefix(db DB, prefix []byte) Iterator { var start, end []byte if len(prefix) == 0 { @@ -33,3 +38,9 @@ func cpIncr(bz []byte) (ret []byte) { } return EndingKey() } + +func checkKeyCondition(key string, start, end []byte) bool { + leftCondition := bytes.Equal(start, BeginningKey()) || strings.Compare(key, string(start)) >= 0 + rightCondition := bytes.Equal(end, EndingKey()) || strings.Compare(key, string(end)) < 0 + return leftCondition && rightCondition +} From 5636a02d035258701974da39c62d13c1d76f8ae8 Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Tue, 12 Dec 2017 19:19:26 -0800 Subject: [PATCH 13/19] Remove GetError() from Iterator --- db/c_level_db.go | 7 +++++-- db/go_level_db.go | 11 +++++++---- db/mem_db.go | 4 ---- db/types.go | 6 ------ 4 files changed, 12 insertions(+), 16 deletions(-) diff --git a/db/c_level_db.go b/db/c_level_db.go index 527fd7da3..93bc1dfb7 100644 --- a/db/c_level_db.go +++ b/db/c_level_db.go @@ -190,6 +190,7 @@ func (c cLevelDBIterator) Domain() ([]byte, []byte) { } func (c cLevelDBIterator) Valid() bool { + c.assertNoError() return c.itr.Valid() } @@ -218,6 +219,8 @@ func (c cLevelDBIterator) Release() { c.itr.Close() } -func (c cLevelDBIterator) GetError() error { - return c.itr.GetError() +func (c cLevelDBIterator) assertNoError() { + if err := c.itr.GetError(); err != nil { + panic(err) + } } diff --git a/db/go_level_db.go b/db/go_level_db.go index 45b437d2b..89015547a 100644 --- a/db/go_level_db.go +++ b/db/go_level_db.go @@ -220,11 +220,8 @@ func (it *goLevelDBIterator) Value() []byte { return v } -func (it *goLevelDBIterator) GetError() error { - return it.source.Error() -} - func (it *goLevelDBIterator) Valid() bool { + it.assertNoError() if it.invalid { return false } @@ -253,3 +250,9 @@ func (it *goLevelDBIterator) Close() { func (it *goLevelDBIterator) Release() { it.source.Release() } + +func (it *goLevelDBIterator) assertNoError() { + if err := it.source.Error(); err != nil { + panic(err) + } +} diff --git a/db/mem_db.go b/db/mem_db.go index a9f21d526..81e209648 100644 --- a/db/mem_db.go +++ b/db/mem_db.go @@ -223,8 +223,4 @@ func (it *memDBIterator) Close() { it.keys = nil } -func (it *memDBIterator) GetError() error { - return nil -} - func (it *memDBIterator) Release() {} diff --git a/db/types.go b/db/types.go index 54c1025a0..8370ff2da 100644 --- a/db/types.go +++ b/db/types.go @@ -107,12 +107,6 @@ type Iterator interface { // If Valid returns false, this method will panic. Value() []byte - // GetError returns an IteratorError from LevelDB if it had one during - // iteration. - // - // This method is safe to call when Valid returns false. - GetError() error - // Release deallocates the given Iterator. Release() } From 318982c0babe627c7dda57e23a1eae2bf0d2c1bf Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Wed, 13 Dec 2017 01:33:38 -0500 Subject: [PATCH 14/19] checkKeyCondition -> IsKeyInDomain --- db/fsdb.go | 2 +- db/mem_db.go | 2 +- db/util.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/db/fsdb.go b/db/fsdb.go index ac9cdd741..8a40d4f19 100644 --- a/db/fsdb.go +++ b/db/fsdb.go @@ -247,7 +247,7 @@ func list(dirPath string, start, end []byte) ([]string, error) { if err != nil { return nil, fmt.Errorf("Failed to unescape %s while listing", name) } - if checkKeyCondition(n, start, end) { + if IsKeyInDomain(n, start, end) { paths = append(paths, n) } } diff --git a/db/mem_db.go b/db/mem_db.go index 81e209648..d20d0e7ea 100644 --- a/db/mem_db.go +++ b/db/mem_db.go @@ -157,7 +157,7 @@ func (db *MemDB) ReverseIterator(start, end []byte) Iterator { func (db *MemDB) getSortedKeys(start, end []byte) []string { keys := []string{} for key, _ := range db.db { - if checkKeyCondition(key, start, end) { + if IsKeyInDomain(key, start, end) { keys = append(keys, key) } } diff --git a/db/util.go b/db/util.go index 02f4a52f0..203ddcfaf 100644 --- a/db/util.go +++ b/db/util.go @@ -39,7 +39,7 @@ func cpIncr(bz []byte) (ret []byte) { return EndingKey() } -func checkKeyCondition(key string, start, end []byte) bool { +func IsKeyInDomain(key string, start, end []byte) bool { leftCondition := bytes.Equal(start, BeginningKey()) || strings.Compare(key, string(start)) >= 0 rightCondition := bytes.Equal(end, EndingKey()) || strings.Compare(key, string(end)) < 0 return leftCondition && rightCondition From d4aeca8ce30c1cdf84f45e5160240d292fb848bb Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Wed, 13 Dec 2017 20:11:11 -0500 Subject: [PATCH 15/19] fixes from @melekes --- db/backend_test.go | 26 +++++++++----------------- db/c_level_db.go | 4 ++-- db/fsdb.go | 13 ++++++------- db/go_level_db.go | 16 ++++++++++------ 4 files changed, 27 insertions(+), 32 deletions(-) diff --git a/db/backend_test.go b/db/backend_test.go index 16649cd20..7ead549b0 100644 --- a/db/backend_test.go +++ b/db/backend_test.go @@ -3,7 +3,7 @@ package db import ( "fmt" "os" - "path" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -12,7 +12,7 @@ import ( ) func cleanupDBDir(dir, name string) { - os.RemoveAll(path.Join(dir, name) + ".db") + os.RemoveAll(filepath.Join(dir, name) + ".db") } func testBackendGetSetDelete(t *testing.T, backend string) { @@ -45,15 +45,6 @@ func TestBackendsGetSetDelete(t *testing.T) { } } -func assertPanics(t *testing.T, dbType, name string, fn func()) { - defer func() { - r := recover() - assert.NotNil(t, r, cmn.Fmt("expecting %s.%s to panic", dbType, name)) - }() - - fn() -} - func TestBackendsNilKeys(t *testing.T) { // test all backends for dbType, creator := range backends { @@ -62,12 +53,13 @@ func TestBackendsNilKeys(t *testing.T) { defer cleanupDBDir("", name) assert.Nil(t, err) - assertPanics(t, dbType, "get", func() { db.Get(nil) }) - assertPanics(t, dbType, "has", func() { db.Has(nil) }) - assertPanics(t, dbType, "set", func() { db.Set(nil, []byte("abc")) }) - assertPanics(t, dbType, "setsync", func() { db.SetSync(nil, []byte("abc")) }) - assertPanics(t, dbType, "delete", func() { db.Delete(nil) }) - assertPanics(t, dbType, "deletesync", func() { db.DeleteSync(nil) }) + panicMsg := "expecting %s.%s to panic" + assert.Panics(t, func() { db.Get(nil) }, panicMsg, dbType, "get") + assert.Panics(t, func() { db.Has(nil) }, panicMsg, dbType, "has") + assert.Panics(t, func() { db.Set(nil, []byte("abc")) }, panicMsg, dbType, "set") + assert.Panics(t, func() { db.SetSync(nil, []byte("abc")) }, panicMsg, dbType, "setsync") + assert.Panics(t, func() { db.Delete(nil) }, panicMsg, dbType, "delete") + assert.Panics(t, func() { db.DeleteSync(nil) }, panicMsg, dbType, "deletesync") db.Close() } diff --git a/db/c_level_db.go b/db/c_level_db.go index 93bc1dfb7..ff8347cc7 100644 --- a/db/c_level_db.go +++ b/db/c_level_db.go @@ -4,7 +4,7 @@ package db import ( "fmt" - "path" + "path/filepath" "github.com/jmhodges/levigo" ) @@ -27,7 +27,7 @@ type CLevelDB struct { } func NewCLevelDB(name string, dir string) (*CLevelDB, error) { - dbPath := path.Join(dir, name+".db") + dbPath := filepath.Join(dir, name+".db") opts := levigo.NewOptions() opts.SetCache(levigo.NewLRUCache(1 << 30)) diff --git a/db/fsdb.go b/db/fsdb.go index 8a40d4f19..85adae630 100644 --- a/db/fsdb.go +++ b/db/fsdb.go @@ -5,7 +5,6 @@ import ( "io/ioutil" "net/url" "os" - "path" "path/filepath" "sort" "sync" @@ -54,7 +53,7 @@ func (db *FSDB) Get(key []byte) []byte { if os.IsNotExist(err) { return nil } else if err != nil { - panic(errors.Wrap(err, fmt.Sprintf("Getting key %s (0x%X)", string(key), key))) + panic(errors.Wrapf(err, "Getting key %s (0x%X)", string(key), key)) } return value } @@ -69,7 +68,7 @@ func (db *FSDB) Has(key []byte) bool { if os.IsNotExist(err) { return false } else if err != nil { - panic(errors.Wrap(err, fmt.Sprintf("Getting key %s (0x%X)", string(key), key))) + panic(errors.Wrapf(err, "Getting key %s (0x%X)", string(key), key)) } return true } @@ -99,7 +98,7 @@ func (db *FSDB) SetNoLock(key []byte, value []byte) { path := db.nameToPath(key) err := write(path, value) if err != nil { - panic(errors.Wrap(err, fmt.Sprintf("Setting key %s (0x%X)", string(key), key))) + panic(errors.Wrapf(err, "Setting key %s (0x%X)", string(key), key)) } } @@ -127,7 +126,7 @@ func (db *FSDB) DeleteNoLock(key []byte) { if os.IsNotExist(err) { return } else if err != nil { - panic(errors.Wrap(err, fmt.Sprintf("Removing key %s (0x%X)", string(key), key))) + panic(errors.Wrapf(err, "Removing key %s (0x%X)", string(key), key)) } } @@ -172,7 +171,7 @@ func (db *FSDB) Iterator(start, end []byte) Iterator { // Not the best, but probably not a bottleneck depending. keys, err := list(db.dir, start, end) if err != nil { - panic(errors.Wrap(err, fmt.Sprintf("Listing keys in %s", db.dir))) + panic(errors.Wrapf(err, "Listing keys in %s", db.dir)) } sort.Strings(keys) it.keys = keys @@ -186,7 +185,7 @@ func (db *FSDB) ReverseIterator(start, end []byte) Iterator { func (db *FSDB) nameToPath(name []byte) string { n := url.PathEscape(string(name)) - return path.Join(db.dir, n) + return filepath.Join(db.dir, n) } // Read some bytes to a file. diff --git a/db/go_level_db.go b/db/go_level_db.go index 89015547a..d741c6904 100644 --- a/db/go_level_db.go +++ b/db/go_level_db.go @@ -2,7 +2,7 @@ package db import ( "fmt" - "path" + "path/filepath" "github.com/syndtr/goleveldb/leveldb" "github.com/syndtr/goleveldb/leveldb/errors" @@ -27,7 +27,7 @@ type GoLevelDB struct { } func NewGoLevelDB(name string, dir string) (*GoLevelDB, error) { - dbPath := path.Join(dir, name+".db") + dbPath := filepath.Join(dir, name+".db") db, err := leveldb.OpenFile(dbPath, nil) if err != nil { return nil, err @@ -170,13 +170,17 @@ func (mBatch *goLevelDBBatch) Write() { // Iterator func (db *GoLevelDB) Iterator(start, end []byte) Iterator { - itr := &goLevelDBIterator{ - source: db.db.NewIterator(nil, nil), + itr := db.db.NewIterator(nil, nil) + if len(start) > 0 { + itr.Seek(start) + } else { + itr.First() + } + return &goLevelDBIterator{ + source: itr, start: start, end: end, } - itr.source.Seek(start) - return itr } func (db *GoLevelDB) ReverseIterator(start, end []byte) Iterator { From a2f7898b6d89b2d2ef9563f622a7ddee8b193a13 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Wed, 13 Dec 2017 22:28:37 -0500 Subject: [PATCH 16/19] db: fix c and go iterators --- db/c_level_db.go | 40 ++++++++++++------ db/go_level_db.go | 24 ++++++++--- db/util_test.go | 101 +++++++++++++++------------------------------- 3 files changed, 79 insertions(+), 86 deletions(-) diff --git a/db/c_level_db.go b/db/c_level_db.go index ff8347cc7..8e2a9372d 100644 --- a/db/c_level_db.go +++ b/db/c_level_db.go @@ -3,6 +3,7 @@ package db import ( + "bytes" "fmt" "path/filepath" @@ -166,7 +167,7 @@ func (db *CLevelDB) Iterator(start, end []byte) Iterator { } else { itr.SeekToFirst() } - return cLevelDBIterator{ + return &cLevelDBIterator{ itr: itr, start: start, end: end, @@ -183,43 +184,58 @@ var _ Iterator = (*cLevelDBIterator)(nil) type cLevelDBIterator struct { itr *levigo.Iterator start, end []byte + invalid bool } -func (c cLevelDBIterator) Domain() ([]byte, []byte) { +func (c *cLevelDBIterator) Domain() ([]byte, []byte) { return c.start, c.end } -func (c cLevelDBIterator) Valid() bool { +func (c *cLevelDBIterator) Valid() bool { c.assertNoError() - return c.itr.Valid() + if c.invalid { + return false + } + c.invalid = !c.itr.Valid() + return !c.invalid } -func (c cLevelDBIterator) Key() []byte { - if !c.itr.Valid() { +func (c *cLevelDBIterator) Key() []byte { + if !c.Valid() { panic("cLevelDBIterator Key() called when invalid") } return c.itr.Key() } -func (c cLevelDBIterator) Value() []byte { - if !c.itr.Valid() { +func (c *cLevelDBIterator) Value() []byte { + if !c.Valid() { panic("cLevelDBIterator Value() called when invalid") } return c.itr.Value() } -func (c cLevelDBIterator) Next() { - if !c.itr.Valid() { +func (c *cLevelDBIterator) Next() { + if !c.Valid() { panic("cLevelDBIterator Next() called when invalid") } c.itr.Next() + c.checkEndKey() // if we've exceeded the range, we're now invalid } -func (c cLevelDBIterator) Release() { +// levigo has no upper bound when iterating, so need to check ourselves +func (c *cLevelDBIterator) checkEndKey() []byte { + key := c.itr.Key() + if c.end != nil && bytes.Compare(key, c.end) > 0 { + c.invalid = true + } + return key +} + +func (c *cLevelDBIterator) Release() { c.itr.Close() } -func (c cLevelDBIterator) assertNoError() { +func (c *cLevelDBIterator) assertNoError() { if err := c.itr.GetError(); err != nil { panic(err) } diff --git a/db/go_level_db.go b/db/go_level_db.go index d741c6904..0d24020e0 100644 --- a/db/go_level_db.go +++ b/db/go_level_db.go @@ -8,6 +8,7 @@ import ( "github.com/syndtr/goleveldb/leveldb/errors" "github.com/syndtr/goleveldb/leveldb/iterator" "github.com/syndtr/goleveldb/leveldb/opt" + "github.com/syndtr/goleveldb/leveldb/util" . "github.com/tendermint/tmlibs/common" ) @@ -169,13 +170,24 @@ func (mBatch *goLevelDBBatch) Write() { //---------------------------------------- // Iterator -func (db *GoLevelDB) Iterator(start, end []byte) Iterator { - itr := db.db.NewIterator(nil, nil) - if len(start) > 0 { - itr.Seek(start) - } else { - itr.First() +// https://godoc.org/github.com/syndtr/goleveldb/leveldb#DB.NewIterator +// A nil Range.Start is treated as a key before all keys in the DB. +// And a nil Range.Limit is treated as a key after all keys in the DB. +func goLevelDBIterRange(start, end []byte) *util.Range { + // XXX: what if start == nil ? + if len(start) == 0 { + start = nil } + return &util.Range{ + Start: start, + Limit: end, + } +} + +func (db *GoLevelDB) Iterator(start, end []byte) Iterator { + itrRange := goLevelDBIterRange(start, end) + itr := db.db.NewIterator(itrRange, nil) + itr.Seek(start) // if we don't call this the itr is never valid (?!) return &goLevelDBIterator{ source: itr, start: start, diff --git a/db/util_test.go b/db/util_test.go index a0ce9cd51..b273f8d46 100644 --- a/db/util_test.go +++ b/db/util_test.go @@ -5,6 +5,7 @@ import ( "testing" ) +// empty iterator for empty db func TestPrefixIteratorNoMatchNil(t *testing.T) { for backend, _ := range backends { t.Run(fmt.Sprintf("Prefix w/ backend %s", backend), func(t *testing.T) { @@ -16,6 +17,7 @@ func TestPrefixIteratorNoMatchNil(t *testing.T) { } } +// empty iterator for db populated after iterator created func TestPrefixIteratorNoMatch1(t *testing.T) { for backend, _ := range backends { t.Run(fmt.Sprintf("Prefix w/ backend %s", backend), func(t *testing.T) { @@ -28,24 +30,8 @@ func TestPrefixIteratorNoMatch1(t *testing.T) { } } -func TestPrefixIteratorMatch2(t *testing.T) { - for backend, _ := range backends { - t.Run(fmt.Sprintf("Prefix w/ backend %s", backend), func(t *testing.T) { - db := newTempDB(t, backend) - db.SetSync(bz("2"), bz("value_2")) - itr := IteratePrefix(db, []byte("2")) - - checkValid(t, itr, true) - checkItem(t, itr, bz("2"), bz("value_2")) - checkNext(t, itr, false) - - // Once invalid... - checkInvalid(t, itr) - }) - } -} - -func TestPrefixIteratorMatch3(t *testing.T) { +// empty iterator for prefix starting above db entry +func TestPrefixIteratorNoMatch2(t *testing.T) { for backend, _ := range backends { t.Run(fmt.Sprintf("Prefix w/ backend %s", backend), func(t *testing.T) { db := newTempDB(t, backend) @@ -58,14 +44,40 @@ func TestPrefixIteratorMatch3(t *testing.T) { } } -// Search for a/1, fail by too much Next() +// iterator with single val for db with single val, starting from that val +func TestPrefixIteratorMatch1(t *testing.T) { + for backend, _ := range backends { + t.Run(fmt.Sprintf("Prefix w/ backend %s", backend), func(t *testing.T) { + db := newTempDB(t, backend) + db.SetSync(bz("2"), bz("value_2")) + itr := IteratePrefix(db, bz("2")) + + checkValid(t, itr, true) + checkItem(t, itr, bz("2"), bz("value_2")) + checkNext(t, itr, false) + + // Once invalid... + checkInvalid(t, itr) + }) + } +} + +// iterator with prefix iterates over everything with same prefix func TestPrefixIteratorMatches1N(t *testing.T) { for backend, _ := range backends { t.Run(fmt.Sprintf("Prefix w/ backend %s", backend), func(t *testing.T) { db := newTempDB(t, backend) + + // prefixed db.SetSync(bz("a/1"), bz("value_1")) db.SetSync(bz("a/3"), bz("value_3")) - itr := IteratePrefix(db, []byte("a/")) + + // not + db.SetSync(bz("b/3"), bz("value_3")) + db.SetSync(bz("a-3"), bz("value_3")) + db.SetSync(bz("a.3"), bz("value_3")) + db.SetSync(bz("abcdefg"), bz("value_3")) + itr := IteratePrefix(db, bz("a/")) checkValid(t, itr, true) checkItem(t, itr, bz("a/1"), bz("value_1")) @@ -75,54 +87,7 @@ func TestPrefixIteratorMatches1N(t *testing.T) { // Bad! checkNext(t, itr, false) - // Once invalid... - checkInvalid(t, itr) - }) - } -} - -// Search for a/2, fail by too much Next() -func TestPrefixIteratorMatches2N(t *testing.T) { - for backend, _ := range backends { - t.Run(fmt.Sprintf("Prefix w/ backend %s", backend), func(t *testing.T) { - db := newTempDB(t, backend) - db.SetSync(bz("a/1"), bz("value_1")) - db.SetSync(bz("a/3"), bz("value_3")) - itr := IteratePrefix(db, []byte("a/")) - - checkValid(t, itr, true) - checkItem(t, itr, bz("a/1"), bz("value_1")) - checkNext(t, itr, true) - checkValid(t, itr, true) - checkItem(t, itr, bz("a/3"), bz("value_3")) - - // Bad! - checkNext(t, itr, false) - - // Once invalid... - checkInvalid(t, itr) - }) - } -} - -// Search for a/3, fail by too much Next() -func TestPrefixIteratorMatches3N(t *testing.T) { - for backend, _ := range backends { - t.Run(fmt.Sprintf("Prefix w/ backend %s", backend), func(t *testing.T) { - db := newTempDB(t, backend) - db.SetSync(bz("a/1"), bz("value_1")) - db.SetSync(bz("a/3"), bz("value_3")) - itr := IteratePrefix(db, []byte("a/")) - - checkValid(t, itr, true) - checkItem(t, itr, bz("a/1"), bz("value_1")) - checkNext(t, itr, true) - checkItem(t, itr, bz("a/3"), bz("value_3")) - - // Bad! - checkNext(t, itr, false) - - // Once invalid... + //Once invalid... checkInvalid(t, itr) }) } From a7b20d4e46db417d2256dfe81d910834348e2dc5 Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Fri, 15 Dec 2017 02:48:40 -0700 Subject: [PATCH 17/19] db: Simplify exists check, fix IsKeyInDomain signature, Iterator Close + *FSDB.HasKey now uses common.FileExists to test for file existence + IsKeyInDomain takes key as a []byte slice instead of as a string to avoid extraneous []byte<-->string conversions for start and end + Iterator.Close() instead of Iterator.Release() + withDB helper to encapsulate DB creation, deferred cleanups so that for loops can use opened DBs and discard them ASAP Addressing accepted changes from review with @jaekwon --- db/backend_test.go | 48 +++++++++++++++++++++++++--------------------- db/c_level_db.go | 4 ++-- db/fsdb.go | 11 +++-------- db/go_level_db.go | 4 ---- db/mem_db.go | 4 +--- db/types.go | 4 ++-- db/util.go | 7 +++---- 7 files changed, 37 insertions(+), 45 deletions(-) diff --git a/db/backend_test.go b/db/backend_test.go index 7ead549b0..00fece515 100644 --- a/db/backend_test.go +++ b/db/backend_test.go @@ -45,33 +45,37 @@ func TestBackendsGetSetDelete(t *testing.T) { } } +func withDB(t *testing.T, creator dbCreator, fn func(DB)) { + name := cmn.Fmt("test_%x", cmn.RandStr(12)) + db, err := creator(name, "") + defer cleanupDBDir("", name) + assert.Nil(t, err) + fn(db) + db.Close() +} + func TestBackendsNilKeys(t *testing.T) { // test all backends for dbType, creator := range backends { - name := cmn.Fmt("test_%x", cmn.RandStr(12)) - db, err := creator(name, "") - defer cleanupDBDir("", name) - assert.Nil(t, err) - - panicMsg := "expecting %s.%s to panic" - assert.Panics(t, func() { db.Get(nil) }, panicMsg, dbType, "get") - assert.Panics(t, func() { db.Has(nil) }, panicMsg, dbType, "has") - assert.Panics(t, func() { db.Set(nil, []byte("abc")) }, panicMsg, dbType, "set") - assert.Panics(t, func() { db.SetSync(nil, []byte("abc")) }, panicMsg, dbType, "setsync") - assert.Panics(t, func() { db.Delete(nil) }, panicMsg, dbType, "delete") - assert.Panics(t, func() { db.DeleteSync(nil) }, panicMsg, dbType, "deletesync") - - db.Close() + withDB(t, creator, func(db DB) { + panicMsg := "expecting %s.%s to panic" + assert.Panics(t, func() { db.Get(nil) }, panicMsg, dbType, "get") + assert.Panics(t, func() { db.Has(nil) }, panicMsg, dbType, "has") + assert.Panics(t, func() { db.Set(nil, []byte("abc")) }, panicMsg, dbType, "set") + assert.Panics(t, func() { db.SetSync(nil, []byte("abc")) }, panicMsg, dbType, "setsync") + assert.Panics(t, func() { db.Delete(nil) }, panicMsg, dbType, "delete") + assert.Panics(t, func() { db.DeleteSync(nil) }, panicMsg, dbType, "deletesync") + }) } } func TestGoLevelDBBackendStr(t *testing.T) { - name := cmn.Fmt("test_%x", cmn.RandStr(12)) - db := NewDB(name, LevelDBBackendStr, "") - defer cleanupDBDir("", name) - - if _, ok := backends[CLevelDBBackendStr]; !ok { - _, ok := db.(*GoLevelDB) - assert.True(t, ok) - } + name := cmn.Fmt("test_%x", cmn.RandStr(12)) + db := NewDB(name, LevelDBBackendStr, "") + defer cleanupDBDir("", name) + + if _, ok := backends[CLevelDBBackendStr]; !ok { + _, ok := db.(*GoLevelDB) + assert.True(t, ok) + } } diff --git a/db/c_level_db.go b/db/c_level_db.go index 8e2a9372d..961e4d090 100644 --- a/db/c_level_db.go +++ b/db/c_level_db.go @@ -109,7 +109,7 @@ func (db *CLevelDB) Close() { func (db *CLevelDB) Print() { itr := db.Iterator(BeginningKey(), EndingKey()) - defer itr.Release() + defer itr.Close() for ; itr.Valid(); itr.Next() { key := itr.Key() value := itr.Value() @@ -231,7 +231,7 @@ func (c *cLevelDBIterator) checkEndKey() []byte { return key } -func (c *cLevelDBIterator) Release() { +func (c *cLevelDBIterator) Close() { c.itr.Close() } diff --git a/db/fsdb.go b/db/fsdb.go index 85adae630..056cc3982 100644 --- a/db/fsdb.go +++ b/db/fsdb.go @@ -10,6 +10,7 @@ import ( "sync" "github.com/pkg/errors" + cmn "github.com/tendermint/tmlibs/common" ) const ( @@ -64,13 +65,7 @@ func (db *FSDB) Has(key []byte) bool { panicNilKey(key) path := db.nameToPath(key) - _, err := read(path) - if os.IsNotExist(err) { - return false - } else if err != nil { - panic(errors.Wrapf(err, "Getting key %s (0x%X)", string(key), key)) - } - return true + return cmn.FileExists(path) } func (db *FSDB) Set(key []byte, value []byte) { @@ -246,7 +241,7 @@ func list(dirPath string, start, end []byte) ([]string, error) { if err != nil { return nil, fmt.Errorf("Failed to unescape %s while listing", name) } - if IsKeyInDomain(n, start, end) { + if IsKeyInDomain([]byte(n), start, end) { paths = append(paths, n) } } diff --git a/db/go_level_db.go b/db/go_level_db.go index 0d24020e0..45cb04984 100644 --- a/db/go_level_db.go +++ b/db/go_level_db.go @@ -263,10 +263,6 @@ func (it *goLevelDBIterator) Close() { it.source.Release() } -func (it *goLevelDBIterator) Release() { - it.source.Release() -} - func (it *goLevelDBIterator) assertNoError() { if err := it.source.Error(); err != nil { panic(err) diff --git a/db/mem_db.go b/db/mem_db.go index d20d0e7ea..44254870a 100644 --- a/db/mem_db.go +++ b/db/mem_db.go @@ -157,7 +157,7 @@ func (db *MemDB) ReverseIterator(start, end []byte) Iterator { func (db *MemDB) getSortedKeys(start, end []byte) []string { keys := []string{} for key, _ := range db.db { - if IsKeyInDomain(key, start, end) { + if IsKeyInDomain([]byte(key), start, end) { keys = append(keys, key) } } @@ -222,5 +222,3 @@ func (it *memDBIterator) Close() { it.db = nil it.keys = nil } - -func (it *memDBIterator) Release() {} diff --git a/db/types.go b/db/types.go index 8370ff2da..ee8d69cc1 100644 --- a/db/types.go +++ b/db/types.go @@ -68,7 +68,7 @@ func EndingKey() []byte { Usage: var itr Iterator = ... - defer itr.Release() + defer itr.Close() for ; itr.Valid(); itr.Next() { k, v := itr.Key(); itr.Value() @@ -108,7 +108,7 @@ type Iterator interface { Value() []byte // Release deallocates the given Iterator. - Release() + Close() } // For testing convenience. diff --git a/db/util.go b/db/util.go index 203ddcfaf..661d0a16f 100644 --- a/db/util.go +++ b/db/util.go @@ -2,7 +2,6 @@ package db import ( "bytes" - "strings" ) func IteratePrefix(db DB, prefix []byte) Iterator { @@ -39,8 +38,8 @@ func cpIncr(bz []byte) (ret []byte) { return EndingKey() } -func IsKeyInDomain(key string, start, end []byte) bool { - leftCondition := bytes.Equal(start, BeginningKey()) || strings.Compare(key, string(start)) >= 0 - rightCondition := bytes.Equal(end, EndingKey()) || strings.Compare(key, string(end)) < 0 +func IsKeyInDomain(key, start, end []byte) bool { + leftCondition := bytes.Equal(start, BeginningKey()) || bytes.Compare(key, start) >= 0 + rightCondition := bytes.Equal(end, EndingKey()) || bytes.Compare(key, end) < 0 return leftCondition && rightCondition } From 225eace3169c1d1b3c5f0a6786b9f060f2784d38 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Fri, 15 Dec 2017 15:14:48 -0500 Subject: [PATCH 18/19] dont run metalinter on circle ... --- circle.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/circle.yml b/circle.yml index 3dba976be..104cfa6f3 100644 --- a/circle.yml +++ b/circle.yml @@ -15,7 +15,7 @@ dependencies: test: override: - - cd $PROJECT_PATH && make get_vendor_deps && make metalinter_test && bash ./test.sh + - cd $PROJECT_PATH && make get_vendor_deps && bash ./test.sh post: - cd "$PROJECT_PATH" && bash <(curl -s https://codecov.io/bash) -f coverage.txt - cd "$PROJECT_PATH" && mv coverage.txt "${CIRCLE_ARTIFACTS}" From 66b0e8fa2d3b8632bad1eb84076dfe70c84657bf Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Fri, 15 Dec 2017 15:58:23 -0500 Subject: [PATCH 19/19] fix c level db iterator --- db/backend_test.go | 16 ++++++++-------- db/c_level_db.go | 34 ++++++++++++++++++++++------------ 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/db/backend_test.go b/db/backend_test.go index 00fece515..3362fecf6 100644 --- a/db/backend_test.go +++ b/db/backend_test.go @@ -70,12 +70,12 @@ func TestBackendsNilKeys(t *testing.T) { } func TestGoLevelDBBackendStr(t *testing.T) { - name := cmn.Fmt("test_%x", cmn.RandStr(12)) - db := NewDB(name, LevelDBBackendStr, "") - defer cleanupDBDir("", name) - - if _, ok := backends[CLevelDBBackendStr]; !ok { - _, ok := db.(*GoLevelDB) - assert.True(t, ok) - } + name := cmn.Fmt("test_%x", cmn.RandStr(12)) + db := NewDB(name, LevelDBBackendStr, "") + defer cleanupDBDir("", name) + + if _, ok := backends[CLevelDBBackendStr]; !ok { + _, ok := db.(*GoLevelDB) + assert.True(t, ok) + } } diff --git a/db/c_level_db.go b/db/c_level_db.go index 961e4d090..60198d84c 100644 --- a/db/c_level_db.go +++ b/db/c_level_db.go @@ -162,16 +162,7 @@ func (mBatch *cLevelDBBatch) Write() { func (db *CLevelDB) Iterator(start, end []byte) Iterator { itr := db.db.NewIterator(db.ro) - if len(start) > 0 { - itr.Seek(start) - } else { - itr.SeekToFirst() - } - return &cLevelDBIterator{ - itr: itr, - start: start, - end: end, - } + return newCLevelDBIterator(itr, start, end) } func (db *CLevelDB) ReverseIterator(start, end []byte) Iterator { @@ -187,6 +178,21 @@ type cLevelDBIterator struct { invalid bool } +func newCLevelDBIterator(itr *levigo.Iterator, start, end []byte) *cLevelDBIterator { + + if len(start) > 0 { + itr.Seek(start) + } else { + itr.SeekToFirst() + } + + return &cLevelDBIterator{ + itr: itr, + start: start, + end: end, + } +} + func (c *cLevelDBIterator) Domain() ([]byte, []byte) { return c.start, c.end } @@ -223,12 +229,16 @@ func (c *cLevelDBIterator) Next() { } // levigo has no upper bound when iterating, so need to check ourselves -func (c *cLevelDBIterator) checkEndKey() []byte { +func (c *cLevelDBIterator) checkEndKey() { + if !c.itr.Valid() { + c.invalid = true + return + } + key := c.itr.Key() if c.end != nil && bytes.Compare(key, c.end) > 0 { c.invalid = true } - return key } func (c *cLevelDBIterator) Close() {