From cb08d28d401f04093a0ac6cda8cb0a67a7258d36 Mon Sep 17 00:00:00 2001 From: Jeremiah Andrews Date: Sun, 24 Jun 2018 19:37:32 -0700 Subject: [PATCH] Reverse iterators (#224) with passing tests --- Makefile | 2 +- db/backend_test.go | 64 ++++++++++++++++++++++++++++++++++++ db/c_level_db.go | 45 +++++++++++++++++++------ db/fsdb.go | 18 +++++++--- db/go_level_db.go | 44 +++++++++++++++++++++---- db/remotedb/remotedb_test.go | 2 +- 6 files changed, 151 insertions(+), 24 deletions(-) diff --git a/Makefile b/Makefile index 3c79e680..a55bc139 100644 --- a/Makefile +++ b/Makefile @@ -79,7 +79,7 @@ clean_certs: rm -f db/remotedb/::.crt db/remotedb/::.key test: gen_certs - go test -tags gcc $(shell go list ./... | grep -v vendor) + GOCACHE=off go test -tags gcc $(shell go list ./... | grep -v vendor) make clean_certs test100: diff --git a/db/backend_test.go b/db/backend_test.go index c407b214..d451b7c5 100644 --- a/db/backend_test.go +++ b/db/backend_test.go @@ -149,3 +149,67 @@ func TestGoLevelDBBackend(t *testing.T) { _, ok := db.(*GoLevelDB) assert.True(t, ok) } + +func TestDBIterator(t *testing.T) { + for dbType := range backends { + t.Run(fmt.Sprintf("%v", dbType), func(t *testing.T) { + testDBIterator(t, dbType) + }) + } +} + +func testDBIterator(t *testing.T, backend DBBackendType) { + name := cmn.Fmt("test_%x", cmn.RandStr(12)) + db := NewDB(name, backend, "") + defer cleanupDBDir("", name) + + for i := 0; i < 10; i++ { + if i != 6 { // but skip 6. + db.Set(int642Bytes(int64(i)), nil) + } + } + + verifyIterator(t, db.Iterator(nil, nil), []int64{0, 1, 2, 3, 4, 5, 7, 8, 9}, "forward iterator") + verifyIterator(t, db.ReverseIterator(nil, nil), []int64{9, 8, 7, 5, 4, 3, 2, 1, 0}, "reverse iterator") + + verifyIterator(t, db.Iterator(nil, int642Bytes(0)), []int64(nil), "forward iterator to 0") + verifyIterator(t, db.ReverseIterator(nil, int642Bytes(10)), []int64(nil), "reverse iterator 10") + + verifyIterator(t, db.Iterator(int642Bytes(0), nil), []int64{0, 1, 2, 3, 4, 5, 7, 8, 9}, "forward iterator from 0") + verifyIterator(t, db.Iterator(int642Bytes(1), nil), []int64{1, 2, 3, 4, 5, 7, 8, 9}, "forward iterator from 1") + verifyIterator(t, db.ReverseIterator(int642Bytes(10), nil), []int64{9, 8, 7, 5, 4, 3, 2, 1, 0}, "reverse iterator from 10") + verifyIterator(t, db.ReverseIterator(int642Bytes(9), nil), []int64{9, 8, 7, 5, 4, 3, 2, 1, 0}, "reverse iterator from 9") + verifyIterator(t, db.ReverseIterator(int642Bytes(8), nil), []int64{8, 7, 5, 4, 3, 2, 1, 0}, "reverse iterator from 8") + + verifyIterator(t, db.Iterator(int642Bytes(5), int642Bytes(6)), []int64{5}, "forward iterator from 5 to 6") + verifyIterator(t, db.Iterator(int642Bytes(5), int642Bytes(7)), []int64{5}, "forward iterator from 5 to 7") + verifyIterator(t, db.Iterator(int642Bytes(5), int642Bytes(8)), []int64{5, 7}, "forward iterator from 5 to 8") + verifyIterator(t, db.Iterator(int642Bytes(6), int642Bytes(7)), []int64(nil), "forward iterator from 6 to 7") + verifyIterator(t, db.Iterator(int642Bytes(6), int642Bytes(8)), []int64{7}, "forward iterator from 6 to 8") + verifyIterator(t, db.Iterator(int642Bytes(7), int642Bytes(8)), []int64{7}, "forward iterator from 7 to 8") + + verifyIterator(t, db.ReverseIterator(int642Bytes(5), int642Bytes(4)), []int64{5}, "reverse iterator from 5 to 4") + verifyIterator(t, db.ReverseIterator(int642Bytes(6), int642Bytes(4)), []int64{5}, "reverse iterator from 6 to 4") + verifyIterator(t, db.ReverseIterator(int642Bytes(7), int642Bytes(4)), []int64{7, 5}, "reverse iterator from 7 to 4") + verifyIterator(t, db.ReverseIterator(int642Bytes(6), int642Bytes(5)), []int64(nil), "reverse iterator from 6 to 5") + verifyIterator(t, db.ReverseIterator(int642Bytes(7), int642Bytes(5)), []int64{7}, "reverse iterator from 7 to 5") + verifyIterator(t, db.ReverseIterator(int642Bytes(7), int642Bytes(6)), []int64{7}, "reverse iterator from 7 to 6") + + verifyIterator(t, db.Iterator(int642Bytes(0), int642Bytes(1)), []int64{0}, "forward iterator from 0 to 1") + verifyIterator(t, db.ReverseIterator(int642Bytes(9), int642Bytes(8)), []int64{9}, "reverse iterator from 9 to 8") + + verifyIterator(t, db.Iterator(int642Bytes(2), int642Bytes(4)), []int64{2, 3}, "forward iterator from 2 to 4") + verifyIterator(t, db.Iterator(int642Bytes(4), int642Bytes(2)), []int64(nil), "forward iterator from 4 to 2") + verifyIterator(t, db.ReverseIterator(int642Bytes(4), int642Bytes(2)), []int64{4, 3}, "reverse iterator from 4 to 2") + verifyIterator(t, db.ReverseIterator(int642Bytes(2), int642Bytes(4)), []int64(nil), "reverse iterator from 2 to 4") + +} + +func verifyIterator(t *testing.T, itr Iterator, expected []int64, msg string) { + var list []int64 + for itr.Valid() { + list = append(list, bytes2Int64(itr.Key())) + itr.Next() + } + assert.Equal(t, expected, list, msg) +} diff --git a/db/c_level_db.go b/db/c_level_db.go index e3e6c1d5..30746126 100644 --- a/db/c_level_db.go +++ b/db/c_level_db.go @@ -190,7 +190,8 @@ func (db *CLevelDB) Iterator(start, end []byte) Iterator { } func (db *CLevelDB) ReverseIterator(start, end []byte) Iterator { - panic("not implemented yet") // XXX + itr := db.db.NewIterator(db.ro) + return newCLevelDBIterator(itr, start, end, true) } var _ Iterator = (*cLevelDBIterator)(nil) @@ -204,12 +205,25 @@ type cLevelDBIterator struct { func newCLevelDBIterator(source *levigo.Iterator, start, end []byte, isReverse bool) *cLevelDBIterator { if isReverse { - panic("not implemented yet") // XXX - } - if start != nil { - source.Seek(start) + if start == nil { + source.SeekToLast() + } else { + source.Seek(start) + if source.Valid() { + soakey := source.Key() // start or after key + if bytes.Compare(start, soakey) < 0 { + source.Prev() + } + } else { + source.SeekToLast() + } + } } else { - source.SeekToFirst() + if start == nil { + source.SeekToFirst() + } else { + source.Seek(start) + } } return &cLevelDBIterator{ source: source, @@ -243,9 +257,16 @@ func (itr cLevelDBIterator) Valid() bool { // If key is end or past it, invalid. var end = itr.end var key = itr.source.Key() - if end != nil && bytes.Compare(end, key) <= 0 { - itr.isInvalid = true - return false + if itr.isReverse { + if end != nil && bytes.Compare(key, end) <= 0 { + itr.isInvalid = true + return false + } + } else { + if end != nil && bytes.Compare(end, key) <= 0 { + itr.isInvalid = true + return false + } } // It's valid. @@ -267,7 +288,11 @@ func (itr cLevelDBIterator) Value() []byte { func (itr cLevelDBIterator) Next() { itr.assertNoError() itr.assertIsValid() - itr.source.Next() + if itr.isReverse { + itr.source.Prev() + } else { + itr.source.Next() + } } func (itr cLevelDBIterator) Close() { diff --git a/db/fsdb.go b/db/fsdb.go index 578c1785..b5711ba3 100644 --- a/db/fsdb.go +++ b/db/fsdb.go @@ -151,21 +151,29 @@ func (db *FSDB) Mutex() *sync.Mutex { } func (db *FSDB) Iterator(start, end []byte) Iterator { + return db.MakeIterator(start, end, false) +} + +func (db *FSDB) MakeIterator(start, end []byte, isReversed bool) Iterator { 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, start, end) + keys, err := list(db.dir, start, end, isReversed) if err != nil { panic(errors.Wrapf(err, "Listing keys in %s", db.dir)) } - sort.Strings(keys) + if isReversed { + sort.Sort(sort.Reverse(sort.StringSlice(keys))) + } else { + sort.Strings(keys) + } return newMemDBIterator(db, keys, start, end) } func (db *FSDB) ReverseIterator(start, end []byte) Iterator { - panic("not implemented yet") // XXX + return db.MakeIterator(start, end, true) } func (db *FSDB) nameToPath(name []byte) string { @@ -213,7 +221,7 @@ func remove(path string) error { // List keys in a directory, stripping of escape sequences and dir portions. // CONTRACT: returns os errors directly without wrapping. -func list(dirPath string, start, end []byte) ([]string, error) { +func list(dirPath string, start, end []byte, isReversed bool) ([]string, error) { dir, err := os.Open(dirPath) if err != nil { return nil, err @@ -231,7 +239,7 @@ func list(dirPath string, start, end []byte) ([]string, error) { return nil, fmt.Errorf("Failed to unescape %s while listing", name) } key := unescapeKey([]byte(n)) - if IsKeyInDomain(key, start, end, false) { + if IsKeyInDomain(key, start, end, isReversed) { keys = append(keys, string(key)) } } diff --git a/db/go_level_db.go b/db/go_level_db.go index 9ff162e3..eca8a07f 100644 --- a/db/go_level_db.go +++ b/db/go_level_db.go @@ -193,7 +193,8 @@ func (db *GoLevelDB) Iterator(start, end []byte) Iterator { // Implements DB. func (db *GoLevelDB) ReverseIterator(start, end []byte) Iterator { - panic("not implemented yet") // XXX + itr := db.db.NewIterator(nil, nil) + return newGoLevelDBIterator(itr, start, end, true) } type goLevelDBIterator struct { @@ -208,9 +209,26 @@ var _ Iterator = (*goLevelDBIterator)(nil) func newGoLevelDBIterator(source iterator.Iterator, start, end []byte, isReverse bool) *goLevelDBIterator { if isReverse { - panic("not implemented yet") // XXX + if start == nil { + source.Last() + } else { + valid := source.Seek(start) + if valid { + soakey := source.Key() // start or after key + if bytes.Compare(start, soakey) < 0 { + source.Prev() + } + } else { + source.Last() + } + } + } else { + if start == nil { + source.First() + } else { + source.Seek(start) + } } - source.Seek(start) return &goLevelDBIterator{ source: source, start: start, @@ -245,9 +263,17 @@ func (itr *goLevelDBIterator) Valid() bool { // If key is end or past it, invalid. var end = itr.end var key = itr.source.Key() - if end != nil && bytes.Compare(end, key) <= 0 { - itr.isInvalid = true - return false + + if itr.isReverse { + if end != nil && bytes.Compare(key, end) <= 0 { + itr.isInvalid = true + return false + } + } else { + if end != nil && bytes.Compare(end, key) <= 0 { + itr.isInvalid = true + return false + } } // Valid @@ -276,7 +302,11 @@ func (itr *goLevelDBIterator) Value() []byte { func (itr *goLevelDBIterator) Next() { itr.assertNoError() itr.assertIsValid() - itr.source.Next() + if itr.isReverse { + itr.source.Prev() + } else { + itr.source.Next() + } } // Implements Iterator. diff --git a/db/remotedb/remotedb_test.go b/db/remotedb/remotedb_test.go index b126a901..3cf698a6 100644 --- a/db/remotedb/remotedb_test.go +++ b/db/remotedb/remotedb_test.go @@ -38,7 +38,7 @@ func TestRemoteDB(t *testing.T) { k1 := []byte("key-1") v1 := client.Get(k1) - require.Equal(t, 0, len(v1), "expecting no key1 to have been stored") + require.Equal(t, 0, len(v1), "expecting no key1 to have been stored, got %X (%s)", v1, v1) vv1 := []byte("value-1") client.Set(k1, vv1) gv1 := client.Get(k1)