From a7b20d4e46db417d2256dfe81d910834348e2dc5 Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Fri, 15 Dec 2017 02:48:40 -0700 Subject: [PATCH] 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 }