From 6402c42b6748c0e8494e6432f40fe4527650076b Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Wed, 15 Apr 2020 13:08:53 +0200 Subject: [PATCH] all: simplify and fix database iteration with prefix/start (#20808) * core/state/snapshot: start fixing disk iterator seek * ethdb, rawdb, leveldb, memorydb: implement iterators with prefix and start * les, core/state/snapshot: iterator fixes * all: remove two iterator methods * all: rename Iteratee.NewIteratorWith -> NewIterator * ethdb: fix review concerns --- cmd/utils/cmd.go | 2 +- common/bytes.go | 11 ++++ common/bytes_test.go | 19 +++++++ core/rawdb/accessors_chain.go | 2 +- core/rawdb/accessors_snapshot.go | 2 +- core/rawdb/database.go | 2 +- core/rawdb/table.go | 27 +++------- core/rawdb/table_test.go | 67 +++++++++-------------- core/state/iterator_test.go | 2 +- core/state/snapshot/disklayer_test.go | 76 +++++++++++++++++++++++++++ core/state/snapshot/iterator.go | 4 +- core/state/snapshot/wipe.go | 5 +- core/state/snapshot/wipe_test.go | 10 ++-- core/state/statedb_test.go | 6 +-- eth/filters/bench_test.go | 2 +- eth/handler_test.go | 2 +- ethdb/dbtest/testsuite.go | 73 ++++++++++++++----------- ethdb/iterator.go | 19 +++---- ethdb/leveldb/leveldb.go | 31 +++++------ ethdb/memorydb/memorydb.go | 49 ++++------------- les/clientpool.go | 14 ++++- trie/iterator_test.go | 4 +- trie/proof_test.go | 2 +- trie/sync_bloom.go | 4 +- 24 files changed, 248 insertions(+), 187 deletions(-) diff --git a/cmd/utils/cmd.go b/cmd/utils/cmd.go index a3ee45ba7..a40978bcb 100644 --- a/cmd/utils/cmd.go +++ b/cmd/utils/cmd.go @@ -301,7 +301,7 @@ func ExportPreimages(db ethdb.Database, fn string) error { defer writer.(*gzip.Writer).Close() } // Iterate over the preimages and export them - it := db.NewIteratorWithPrefix([]byte("secure-key-")) + it := db.NewIterator([]byte("secure-key-"), nil) defer it.Release() for it.Next() { diff --git a/common/bytes.go b/common/bytes.go index fa457b92c..634041804 100644 --- a/common/bytes.go +++ b/common/bytes.go @@ -145,3 +145,14 @@ func TrimLeftZeroes(s []byte) []byte { } return s[idx:] } + +// TrimRightZeroes returns a subslice of s without trailing zeroes +func TrimRightZeroes(s []byte) []byte { + idx := len(s) + for ; idx > 0; idx-- { + if s[idx-1] != 0 { + break + } + } + return s[:idx] +} diff --git a/common/bytes_test.go b/common/bytes_test.go index 7cf655337..0e3ec974e 100644 --- a/common/bytes_test.go +++ b/common/bytes_test.go @@ -105,3 +105,22 @@ func TestNoPrefixShortHexOddLength(t *testing.T) { t.Errorf("Expected %x got %x", expected, result) } } + +func TestTrimRightZeroes(t *testing.T) { + tests := []struct { + arr []byte + exp []byte + }{ + {FromHex("0x00ffff00ff0000"), FromHex("0x00ffff00ff")}, + {FromHex("0x00000000000000"), []byte{}}, + {FromHex("0xff"), FromHex("0xff")}, + {[]byte{}, []byte{}}, + {FromHex("0x00ffffffffffff"), FromHex("0x00ffffffffffff")}, + } + for i, test := range tests { + got := TrimRightZeroes(test.arr) + if !bytes.Equal(got, test.exp) { + t.Errorf("test %d, got %x exp %x", i, got, test.exp) + } + } +} diff --git a/core/rawdb/accessors_chain.go b/core/rawdb/accessors_chain.go index 4d064a339..9fa327062 100644 --- a/core/rawdb/accessors_chain.go +++ b/core/rawdb/accessors_chain.go @@ -69,7 +69,7 @@ func ReadAllHashes(db ethdb.Iteratee, number uint64) []common.Hash { prefix := headerKeyPrefix(number) hashes := make([]common.Hash, 0, 1) - it := db.NewIteratorWithPrefix(prefix) + it := db.NewIterator(prefix, nil) defer it.Release() for it.Next() { diff --git a/core/rawdb/accessors_snapshot.go b/core/rawdb/accessors_snapshot.go index 3a8d6c779..ecd4e6597 100644 --- a/core/rawdb/accessors_snapshot.go +++ b/core/rawdb/accessors_snapshot.go @@ -93,7 +93,7 @@ func DeleteStorageSnapshot(db ethdb.KeyValueWriter, accountHash, storageHash com // IterateStorageSnapshots returns an iterator for walking the entire storage // space of a specific account. func IterateStorageSnapshots(db ethdb.Iteratee, accountHash common.Hash) ethdb.Iterator { - return db.NewIteratorWithPrefix(storageSnapshotsKey(accountHash)) + return db.NewIterator(storageSnapshotsKey(accountHash), nil) } // ReadSnapshotJournal retrieves the serialized in-memory diff layers saved at diff --git a/core/rawdb/database.go b/core/rawdb/database.go index b74d8e2e3..cc05491b8 100644 --- a/core/rawdb/database.go +++ b/core/rawdb/database.go @@ -221,7 +221,7 @@ func NewLevelDBDatabaseWithFreezer(file string, cache int, handles int, freezer // InspectDatabase traverses the entire database and checks the size // of all different categories of data. func InspectDatabase(db ethdb.Database) error { - it := db.NewIterator() + it := db.NewIterator(nil, nil) defer it.Release() var ( diff --git a/core/rawdb/table.go b/core/rawdb/table.go index 092341fbf..323ef6293 100644 --- a/core/rawdb/table.go +++ b/core/rawdb/table.go @@ -103,27 +103,12 @@ func (t *table) Delete(key []byte) error { return t.db.Delete(append([]byte(t.prefix), key...)) } -// NewIterator creates a binary-alphabetical iterator over the entire keyspace -// contained within the database. -func (t *table) NewIterator() ethdb.Iterator { - return t.NewIteratorWithPrefix(nil) -} - -// NewIteratorWithStart creates a binary-alphabetical iterator over a subset of -// database content starting at a particular initial key (or after, if it does -// not exist). -func (t *table) NewIteratorWithStart(start []byte) ethdb.Iterator { - iter := t.db.NewIteratorWithStart(append([]byte(t.prefix), start...)) - return &tableIterator{ - iter: iter, - prefix: t.prefix, - } -} - -// NewIteratorWithPrefix creates a binary-alphabetical iterator over a subset -// of database content with a particular key prefix. -func (t *table) NewIteratorWithPrefix(prefix []byte) ethdb.Iterator { - iter := t.db.NewIteratorWithPrefix(append([]byte(t.prefix), prefix...)) +// NewIterator creates a binary-alphabetical iterator over a subset +// of database content with a particular key prefix, starting at a particular +// initial key (or after, if it does not exist). +func (t *table) NewIterator(prefix []byte, start []byte) ethdb.Iterator { + innerPrefix := append([]byte(t.prefix), prefix...) + iter := t.db.NewIterator(innerPrefix, start) return &tableIterator{ iter: iter, prefix: t.prefix, diff --git a/core/rawdb/table_test.go b/core/rawdb/table_test.go index 977eb4bf0..aa6adf3e7 100644 --- a/core/rawdb/table_test.go +++ b/core/rawdb/table_test.go @@ -19,6 +19,8 @@ package rawdb import ( "bytes" "testing" + + "github.com/ethereum/go-ethereum/ethdb" ) func TestTableDatabase(t *testing.T) { testTableDatabase(t, "prefix") } @@ -96,48 +98,31 @@ func testTableDatabase(t *testing.T, prefix string) { } } + check := func(iter ethdb.Iterator, expCount, index int) { + count := 0 + for iter.Next() { + key, value := iter.Key(), iter.Value() + if !bytes.Equal(key, entries[index].key) { + t.Fatalf("Key mismatch: want=%v, got=%v", entries[index].key, key) + } + if !bytes.Equal(value, entries[index].value) { + t.Fatalf("Value mismatch: want=%v, got=%v", entries[index].value, value) + } + index += 1 + count++ + } + if count != expCount { + t.Fatalf("Wrong number of elems, exp %d got %d", expCount, count) + } + iter.Release() + } // Test iterators - iter := db.NewIterator() - var index int - for iter.Next() { - key, value := iter.Key(), iter.Value() - if !bytes.Equal(key, entries[index].key) { - t.Fatalf("Key mismatch: want=%v, got=%v", entries[index].key, key) - } - if !bytes.Equal(value, entries[index].value) { - t.Fatalf("Value mismatch: want=%v, got=%v", entries[index].value, value) - } - index += 1 - } - iter.Release() - + check(db.NewIterator(nil, nil), 6, 0) // Test iterators with prefix - iter = db.NewIteratorWithPrefix([]byte{0xff, 0xff}) - index = 3 - for iter.Next() { - key, value := iter.Key(), iter.Value() - if !bytes.Equal(key, entries[index].key) { - t.Fatalf("Key mismatch: want=%v, got=%v", entries[index].key, key) - } - if !bytes.Equal(value, entries[index].value) { - t.Fatalf("Value mismatch: want=%v, got=%v", entries[index].value, value) - } - index += 1 - } - iter.Release() - + check(db.NewIterator([]byte{0xff, 0xff}, nil), 3, 3) // Test iterators with start point - iter = db.NewIteratorWithStart([]byte{0xff, 0xff, 0x02}) - index = 4 - for iter.Next() { - key, value := iter.Key(), iter.Value() - if !bytes.Equal(key, entries[index].key) { - t.Fatalf("Key mismatch: want=%v, got=%v", entries[index].key, key) - } - if !bytes.Equal(value, entries[index].value) { - t.Fatalf("Value mismatch: want=%v, got=%v", entries[index].value, value) - } - index += 1 - } - iter.Release() + check(db.NewIterator(nil, []byte{0xff, 0xff, 0x02}), 2, 4) + // Test iterators with prefix and start point + check(db.NewIterator([]byte{0xee}, nil), 0, 0) + check(db.NewIterator(nil, []byte{0x00}), 6, 0) } diff --git a/core/state/iterator_test.go b/core/state/iterator_test.go index e9946e9b3..5060f7a65 100644 --- a/core/state/iterator_test.go +++ b/core/state/iterator_test.go @@ -51,7 +51,7 @@ func TestNodeIteratorCoverage(t *testing.T) { t.Errorf("state entry not reported %x", hash) } } - it := db.TrieDB().DiskDB().(ethdb.Database).NewIterator() + it := db.TrieDB().DiskDB().(ethdb.Database).NewIterator(nil, nil) for it.Next() { key := it.Key() if bytes.HasPrefix(key, []byte("secure-key-")) { diff --git a/core/state/snapshot/disklayer_test.go b/core/state/snapshot/disklayer_test.go index aae2aa6b5..15f3a6b1f 100644 --- a/core/state/snapshot/disklayer_test.go +++ b/core/state/snapshot/disklayer_test.go @@ -18,11 +18,15 @@ package snapshot import ( "bytes" + "io/ioutil" + "os" "testing" "github.com/VictoriaMetrics/fastcache" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/rawdb" + "github.com/ethereum/go-ethereum/ethdb" + "github.com/ethereum/go-ethereum/ethdb/leveldb" "github.com/ethereum/go-ethereum/ethdb/memorydb" ) @@ -432,4 +436,76 @@ func TestDiskPartialMerge(t *testing.T) { // This test case is a tiny specialized case of TestDiskPartialMerge, which tests // some very specific cornercases that random tests won't ever trigger. func TestDiskMidAccountPartialMerge(t *testing.T) { + // TODO(@karalabe) ? +} + +// TestDiskSeek tests that seek-operations work on the disk layer +func TestDiskSeek(t *testing.T) { + // Create some accounts in the disk layer + var db ethdb.Database + + if dir, err := ioutil.TempDir("", "disklayer-test"); err != nil { + t.Fatal(err) + } else { + defer os.RemoveAll(dir) + diskdb, err := leveldb.New(dir, 256, 0, "") + if err != nil { + t.Fatal(err) + } + db = rawdb.NewDatabase(diskdb) + } + // Fill even keys [0,2,4...] + for i := 0; i < 0xff; i += 2 { + acc := common.Hash{byte(i)} + rawdb.WriteAccountSnapshot(db, acc, acc[:]) + } + // Add an 'higher' key, with incorrect (higher) prefix + highKey := []byte{rawdb.SnapshotAccountPrefix[0] + 1} + db.Put(highKey, []byte{0xff, 0xff}) + + baseRoot := randomHash() + rawdb.WriteSnapshotRoot(db, baseRoot) + + snaps := &Tree{ + layers: map[common.Hash]snapshot{ + baseRoot: &diskLayer{ + diskdb: db, + cache: fastcache.New(500 * 1024), + root: baseRoot, + }, + }, + } + // Test some different seek positions + type testcase struct { + pos byte + expkey byte + } + var cases = []testcase{ + {0xff, 0x55}, // this should exit immediately without checking key + {0x01, 0x02}, + {0xfe, 0xfe}, + {0xfd, 0xfe}, + {0x00, 0x00}, + } + for i, tc := range cases { + it, err := snaps.AccountIterator(baseRoot, common.Hash{tc.pos}) + if err != nil { + t.Fatalf("case %d, error: %v", i, err) + } + count := 0 + for it.Next() { + k, v, err := it.Hash()[0], it.Account()[0], it.Error() + if err != nil { + t.Fatalf("test %d, item %d, error: %v", i, count, err) + } + // First item in iterator should have the expected key + if count == 0 && k != tc.expkey { + t.Fatalf("test %d, item %d, got %v exp %v", i, count, k, tc.expkey) + } + count++ + if v != k { + t.Fatalf("test %d, item %d, value wrong, got %v exp %v", i, count, v, k) + } + } + } } diff --git a/core/state/snapshot/iterator.go b/core/state/snapshot/iterator.go index e062298fa..84cc5c3bc 100644 --- a/core/state/snapshot/iterator.go +++ b/core/state/snapshot/iterator.go @@ -148,10 +148,10 @@ type diskAccountIterator struct { // AccountIterator creates an account iterator over a disk layer. func (dl *diskLayer) AccountIterator(seek common.Hash) AccountIterator { - // TODO: Fix seek position, or remove seek parameter + pos := common.TrimRightZeroes(seek[:]) return &diskAccountIterator{ layer: dl, - it: dl.diskdb.NewIteratorWithPrefix(rawdb.SnapshotAccountPrefix), + it: dl.diskdb.NewIterator(rawdb.SnapshotAccountPrefix, pos), } } diff --git a/core/state/snapshot/wipe.go b/core/state/snapshot/wipe.go index 052af6f1f..53eb18a2d 100644 --- a/core/state/snapshot/wipe.go +++ b/core/state/snapshot/wipe.go @@ -92,7 +92,7 @@ func wipeKeyRange(db ethdb.KeyValueStore, kind string, prefix []byte, keylen int // Iterate over the key-range and delete all of them start, logged := time.Now(), time.Now() - it := db.NewIteratorWithStart(prefix) + it := db.NewIterator(prefix, nil) for it.Next() { // Skip any keys with the correct prefix but wrong lenth (trie nodes) key := it.Key() @@ -113,7 +113,8 @@ func wipeKeyRange(db ethdb.KeyValueStore, kind string, prefix []byte, keylen int return err } batch.Reset() - it = db.NewIteratorWithStart(key) + seekPos := key[len(prefix):] + it = db.NewIterator(prefix, seekPos) if time.Since(logged) > 8*time.Second { log.Info("Deleting state snapshot leftovers", "kind", kind, "wiped", items, "elapsed", common.PrettyDuration(time.Since(start))) diff --git a/core/state/snapshot/wipe_test.go b/core/state/snapshot/wipe_test.go index cb6e174b3..2c45652a9 100644 --- a/core/state/snapshot/wipe_test.go +++ b/core/state/snapshot/wipe_test.go @@ -60,7 +60,7 @@ func TestWipe(t *testing.T) { // Sanity check that all the keys are present var items int - it := db.NewIteratorWithPrefix(rawdb.SnapshotAccountPrefix) + it := db.NewIterator(rawdb.SnapshotAccountPrefix, nil) defer it.Release() for it.Next() { @@ -69,7 +69,7 @@ func TestWipe(t *testing.T) { items++ } } - it = db.NewIteratorWithPrefix(rawdb.SnapshotStoragePrefix) + it = db.NewIterator(rawdb.SnapshotStoragePrefix, nil) defer it.Release() for it.Next() { @@ -88,7 +88,7 @@ func TestWipe(t *testing.T) { <-wipeSnapshot(db, true) // Iterate over the database end ensure no snapshot information remains - it = db.NewIteratorWithPrefix(rawdb.SnapshotAccountPrefix) + it = db.NewIterator(rawdb.SnapshotAccountPrefix, nil) defer it.Release() for it.Next() { @@ -97,7 +97,7 @@ func TestWipe(t *testing.T) { t.Errorf("snapshot entry remained after wipe: %x", key) } } - it = db.NewIteratorWithPrefix(rawdb.SnapshotStoragePrefix) + it = db.NewIterator(rawdb.SnapshotStoragePrefix, nil) defer it.Release() for it.Next() { @@ -112,7 +112,7 @@ func TestWipe(t *testing.T) { // Iterate over the database and ensure miscellaneous items are present items = 0 - it = db.NewIterator() + it = db.NewIterator(nil, nil) defer it.Release() for it.Next() { diff --git a/core/state/statedb_test.go b/core/state/statedb_test.go index ad6aeb22e..1cb73f015 100644 --- a/core/state/statedb_test.go +++ b/core/state/statedb_test.go @@ -60,7 +60,7 @@ func TestUpdateLeaks(t *testing.T) { } // Ensure that no data was leaked into the database - it := db.NewIterator() + it := db.NewIterator(nil, nil) for it.Next() { t.Errorf("State leaked into database: %x -> %x", it.Key(), it.Value()) } @@ -118,7 +118,7 @@ func TestIntermediateLeaks(t *testing.T) { t.Errorf("can not commit trie %v to persistent database", finalRoot.Hex()) } - it := finalDb.NewIterator() + it := finalDb.NewIterator(nil, nil) for it.Next() { key, fvalue := it.Key(), it.Value() tvalue, err := transDb.Get(key) @@ -131,7 +131,7 @@ func TestIntermediateLeaks(t *testing.T) { } it.Release() - it = transDb.NewIterator() + it = transDb.NewIterator(nil, nil) for it.Next() { key, tvalue := it.Key(), it.Value() fvalue, err := finalDb.Get(key) diff --git a/eth/filters/bench_test.go b/eth/filters/bench_test.go index 584ea23d0..4f70d6d04 100644 --- a/eth/filters/bench_test.go +++ b/eth/filters/bench_test.go @@ -147,7 +147,7 @@ var bloomBitsPrefix = []byte("bloomBits-") func clearBloomBits(db ethdb.Database) { fmt.Println("Clearing bloombits data...") - it := db.NewIteratorWithPrefix(bloomBitsPrefix) + it := db.NewIterator(bloomBitsPrefix, nil) for it.Next() { db.Delete(it.Key()) } diff --git a/eth/handler_test.go b/eth/handler_test.go index e5449c342..1b398a8c0 100644 --- a/eth/handler_test.go +++ b/eth/handler_test.go @@ -317,7 +317,7 @@ func testGetNodeData(t *testing.T, protocol int) { // Fetch for now the entire chain db hashes := []common.Hash{} - it := db.NewIterator() + it := db.NewIterator(nil, nil) for it.Next() { if key := it.Key(); len(key) == common.HashLength { hashes = append(hashes, common.BytesToHash(key)) diff --git a/ethdb/dbtest/testsuite.go b/ethdb/dbtest/testsuite.go index dce2ba2a1..06ee2211e 100644 --- a/ethdb/dbtest/testsuite.go +++ b/ethdb/dbtest/testsuite.go @@ -32,31 +32,32 @@ func TestDatabaseSuite(t *testing.T, New func() ethdb.KeyValueStore) { tests := []struct { content map[string]string prefix string + start string order []string }{ // Empty databases should be iterable - {map[string]string{}, "", nil}, - {map[string]string{}, "non-existent-prefix", nil}, + {map[string]string{}, "", "", nil}, + {map[string]string{}, "non-existent-prefix", "", nil}, // Single-item databases should be iterable - {map[string]string{"key": "val"}, "", []string{"key"}}, - {map[string]string{"key": "val"}, "k", []string{"key"}}, - {map[string]string{"key": "val"}, "l", nil}, + {map[string]string{"key": "val"}, "", "", []string{"key"}}, + {map[string]string{"key": "val"}, "k", "", []string{"key"}}, + {map[string]string{"key": "val"}, "l", "", nil}, // Multi-item databases should be fully iterable { map[string]string{"k1": "v1", "k5": "v5", "k2": "v2", "k4": "v4", "k3": "v3"}, - "", + "", "", []string{"k1", "k2", "k3", "k4", "k5"}, }, { map[string]string{"k1": "v1", "k5": "v5", "k2": "v2", "k4": "v4", "k3": "v3"}, - "k", + "k", "", []string{"k1", "k2", "k3", "k4", "k5"}, }, { map[string]string{"k1": "v1", "k5": "v5", "k2": "v2", "k4": "v4", "k3": "v3"}, - "l", + "l", "", nil, }, // Multi-item databases should be prefix-iterable @@ -65,7 +66,7 @@ func TestDatabaseSuite(t *testing.T, New func() ethdb.KeyValueStore) { "ka1": "va1", "ka5": "va5", "ka2": "va2", "ka4": "va4", "ka3": "va3", "kb1": "vb1", "kb5": "vb5", "kb2": "vb2", "kb4": "vb4", "kb3": "vb3", }, - "ka", + "ka", "", []string{"ka1", "ka2", "ka3", "ka4", "ka5"}, }, { @@ -73,7 +74,24 @@ func TestDatabaseSuite(t *testing.T, New func() ethdb.KeyValueStore) { "ka1": "va1", "ka5": "va5", "ka2": "va2", "ka4": "va4", "ka3": "va3", "kb1": "vb1", "kb5": "vb5", "kb2": "vb2", "kb4": "vb4", "kb3": "vb3", }, - "kc", + "kc", "", + nil, + }, + // Multi-item databases should be prefix-iterable with start position + { + map[string]string{ + "ka1": "va1", "ka5": "va5", "ka2": "va2", "ka4": "va4", "ka3": "va3", + "kb1": "vb1", "kb5": "vb5", "kb2": "vb2", "kb4": "vb4", "kb3": "vb3", + }, + "ka", "3", + []string{"ka3", "ka4", "ka5"}, + }, + { + map[string]string{ + "ka1": "va1", "ka5": "va5", "ka2": "va2", "ka4": "va4", "ka3": "va3", + "kb1": "vb1", "kb5": "vb5", "kb2": "vb2", "kb4": "vb4", "kb3": "vb3", + }, + "ka", "8", nil, }, } @@ -86,7 +104,7 @@ func TestDatabaseSuite(t *testing.T, New func() ethdb.KeyValueStore) { } } // Iterate over the database with the given configs and verify the results - it, idx := db.NewIteratorWithPrefix([]byte(tt.prefix)), 0 + it, idx := db.NewIterator([]byte(tt.prefix), []byte(tt.start)), 0 for it.Next() { if len(tt.order) <= idx { t.Errorf("test %d: prefix=%q more items than expected: checking idx=%d (key %q), expecting len=%d", i, tt.prefix, idx, it.Key(), len(tt.order)) @@ -124,62 +142,57 @@ func TestDatabaseSuite(t *testing.T, New func() ethdb.KeyValueStore) { } { - it := db.NewIterator() + it := db.NewIterator(nil, nil) got, want := iterateKeys(it), keys if err := it.Error(); err != nil { t.Fatal(err) } - it.Release() if !reflect.DeepEqual(got, want) { t.Errorf("Iterator: got: %s; want: %s", got, want) } } { - it := db.NewIteratorWithPrefix([]byte("1")) + it := db.NewIterator([]byte("1"), nil) got, want := iterateKeys(it), []string{"1", "10", "11", "12"} if err := it.Error(); err != nil { t.Fatal(err) } - it.Release() if !reflect.DeepEqual(got, want) { - t.Errorf("IteratorWithPrefix(1): got: %s; want: %s", got, want) + t.Errorf("IteratorWith(1,nil): got: %s; want: %s", got, want) } } { - it := db.NewIteratorWithPrefix([]byte("5")) + it := db.NewIterator([]byte("5"), nil) got, want := iterateKeys(it), []string{} if err := it.Error(); err != nil { t.Fatal(err) } - it.Release() if !reflect.DeepEqual(got, want) { - t.Errorf("IteratorWithPrefix(1): got: %s; want: %s", got, want) + t.Errorf("IteratorWith(5,nil): got: %s; want: %s", got, want) } } { - it := db.NewIteratorWithStart([]byte("2")) + it := db.NewIterator(nil, []byte("2")) got, want := iterateKeys(it), []string{"2", "20", "21", "22", "3", "4", "6"} if err := it.Error(); err != nil { t.Fatal(err) } - it.Release() if !reflect.DeepEqual(got, want) { - t.Errorf("IteratorWithStart(2): got: %s; want: %s", got, want) + t.Errorf("IteratorWith(nil,2): got: %s; want: %s", got, want) } } { - it := db.NewIteratorWithStart([]byte("5")) + it := db.NewIterator(nil, []byte("5")) got, want := iterateKeys(it), []string{"6"} if err := it.Error(); err != nil { t.Fatal(err) } - it.Release() if !reflect.DeepEqual(got, want) { - t.Errorf("IteratorWithStart(2): got: %s; want: %s", got, want) + t.Errorf("IteratorWith(nil,5): got: %s; want: %s", got, want) } } }) @@ -246,11 +259,10 @@ func TestDatabaseSuite(t *testing.T, New func() ethdb.KeyValueStore) { } { - it := db.NewIterator() + it := db.NewIterator(nil, nil) if got, want := iterateKeys(it), []string{"1", "2", "3", "4"}; !reflect.DeepEqual(got, want) { t.Errorf("got: %s; want: %s", got, want) } - it.Release() } b.Reset() @@ -267,11 +279,10 @@ func TestDatabaseSuite(t *testing.T, New func() ethdb.KeyValueStore) { } { - it := db.NewIterator() + it := db.NewIterator(nil, nil) if got, want := iterateKeys(it), []string{"2", "3", "4", "5", "6"}; !reflect.DeepEqual(got, want) { t.Errorf("got: %s; want: %s", got, want) } - it.Release() } }) @@ -296,11 +307,10 @@ func TestDatabaseSuite(t *testing.T, New func() ethdb.KeyValueStore) { t.Fatal(err) } - it := db.NewIterator() + it := db.NewIterator(nil, nil) if got := iterateKeys(it); !reflect.DeepEqual(got, want) { t.Errorf("got: %s; want: %s", got, want) } - it.Release() }) } @@ -311,5 +321,6 @@ func iterateKeys(it ethdb.Iterator) []string { keys = append(keys, string(it.Key())) } sort.Strings(keys) + it.Release() return keys } diff --git a/ethdb/iterator.go b/ethdb/iterator.go index 419e9bdfc..2b49c93a9 100644 --- a/ethdb/iterator.go +++ b/ethdb/iterator.go @@ -51,16 +51,11 @@ type Iterator interface { // Iteratee wraps the NewIterator methods of a backing data store. type Iteratee interface { - // NewIterator creates a binary-alphabetical iterator over the entire keyspace - // contained within the key-value database. - NewIterator() Iterator - - // NewIteratorWithStart creates a binary-alphabetical iterator over a subset of - // database content starting at a particular initial key (or after, if it does - // not exist). - NewIteratorWithStart(start []byte) Iterator - - // NewIteratorWithPrefix creates a binary-alphabetical iterator over a subset - // of database content with a particular key prefix. - NewIteratorWithPrefix(prefix []byte) Iterator + // NewIterator creates a binary-alphabetical iterator over a subset + // of database content with a particular key prefix, starting at a particular + // initial key (or after, if it does not exist). + // + // Note: This method assumes that the prefix is NOT part of the start, so there's + // no need for the caller to prepend the prefix to the start + NewIterator(prefix []byte, start []byte) Iterator } diff --git a/ethdb/leveldb/leveldb.go b/ethdb/leveldb/leveldb.go index 378d4c3cd..7ff52e59f 100644 --- a/ethdb/leveldb/leveldb.go +++ b/ethdb/leveldb/leveldb.go @@ -183,23 +183,11 @@ func (db *Database) NewBatch() ethdb.Batch { } } -// NewIterator creates a binary-alphabetical iterator over the entire keyspace -// contained within the leveldb database. -func (db *Database) NewIterator() ethdb.Iterator { - return db.db.NewIterator(new(util.Range), nil) -} - -// NewIteratorWithStart creates a binary-alphabetical iterator over a subset of -// database content starting at a particular initial key (or after, if it does -// not exist). -func (db *Database) NewIteratorWithStart(start []byte) ethdb.Iterator { - return db.db.NewIterator(&util.Range{Start: start}, nil) -} - -// NewIteratorWithPrefix creates a binary-alphabetical iterator over a subset -// of database content with a particular key prefix. -func (db *Database) NewIteratorWithPrefix(prefix []byte) ethdb.Iterator { - return db.db.NewIterator(util.BytesPrefix(prefix), nil) +// NewIterator creates a binary-alphabetical iterator over a subset +// of database content with a particular key prefix, starting at a particular +// initial key (or after, if it does not exist). +func (db *Database) NewIterator(prefix []byte, start []byte) ethdb.Iterator { + return db.db.NewIterator(bytesPrefixRange(prefix, start), nil) } // Stat returns a particular internal stat of the database. @@ -488,3 +476,12 @@ func (r *replayer) Delete(key []byte) { } r.failure = r.writer.Delete(key) } + +// bytesPrefixRange returns key range that satisfy +// - the given prefix, and +// - the given seek position +func bytesPrefixRange(prefix, start []byte) *util.Range { + r := util.BytesPrefix(prefix) + r.Start = append(r.Start, start...) + return r +} diff --git a/ethdb/memorydb/memorydb.go b/ethdb/memorydb/memorydb.go index 346edc438..4c5e1a84d 100644 --- a/ethdb/memorydb/memorydb.go +++ b/ethdb/memorydb/memorydb.go @@ -129,55 +129,26 @@ func (db *Database) NewBatch() ethdb.Batch { } } -// NewIterator creates a binary-alphabetical iterator over the entire keyspace -// contained within the memory database. -func (db *Database) NewIterator() ethdb.Iterator { - return db.NewIteratorWithStart(nil) -} - -// NewIteratorWithStart creates a binary-alphabetical iterator over a subset of -// database content starting at a particular initial key (or after, if it does -// not exist). -func (db *Database) NewIteratorWithStart(start []byte) ethdb.Iterator { - db.lock.RLock() - defer db.lock.RUnlock() - - var ( - st = string(start) - keys = make([]string, 0, len(db.db)) - values = make([][]byte, 0, len(db.db)) - ) - // Collect the keys from the memory database corresponding to the given start - for key := range db.db { - if key >= st { - keys = append(keys, key) - } - } - // Sort the items and retrieve the associated values - sort.Strings(keys) - for _, key := range keys { - values = append(values, db.db[key]) - } - return &iterator{ - keys: keys, - values: values, - } -} - -// NewIteratorWithPrefix creates a binary-alphabetical iterator over a subset -// of database content with a particular key prefix. -func (db *Database) NewIteratorWithPrefix(prefix []byte) ethdb.Iterator { +// NewIterator creates a binary-alphabetical iterator over a subset +// of database content with a particular key prefix, starting at a particular +// initial key (or after, if it does not exist). +func (db *Database) NewIterator(prefix []byte, start []byte) ethdb.Iterator { db.lock.RLock() defer db.lock.RUnlock() var ( pr = string(prefix) + st = string(append(prefix, start...)) keys = make([]string, 0, len(db.db)) values = make([][]byte, 0, len(db.db)) ) // Collect the keys from the memory database corresponding to the given prefix + // and start for key := range db.db { - if strings.HasPrefix(key, pr) { + if !strings.HasPrefix(key, pr) { + continue + } + if key >= st { keys = append(keys, key) } } diff --git a/les/clientpool.go b/les/clientpool.go index b01c825a7..c86264950 100644 --- a/les/clientpool.go +++ b/les/clientpool.go @@ -691,6 +691,14 @@ func (db *nodeDB) close() { close(db.closeCh) } +func (db *nodeDB) getPrefix(neg bool) []byte { + prefix := positiveBalancePrefix + if neg { + prefix = negativeBalancePrefix + } + return append(db.verbuf[:], prefix...) +} + func (db *nodeDB) key(id []byte, neg bool) []byte { prefix := positiveBalancePrefix if neg { @@ -761,7 +769,8 @@ func (db *nodeDB) getPosBalanceIDs(start, stop enode.ID, maxCount int) (result [ if maxCount <= 0 { return } - it := db.db.NewIteratorWithStart(db.key(start.Bytes(), false)) + prefix := db.getPrefix(false) + it := db.db.NewIterator(prefix, start.Bytes()) defer it.Release() for i := len(stop[:]) - 1; i >= 0; i-- { stop[i]-- @@ -840,8 +849,9 @@ func (db *nodeDB) expireNodes() { visited int deleted int start = time.Now() + prefix = db.getPrefix(true) ) - iter := db.db.NewIteratorWithPrefix(append(db.verbuf[:], negativeBalancePrefix...)) + iter := db.db.NewIterator(prefix, nil) for iter.Next() { visited += 1 var balance negBalance diff --git a/trie/iterator_test.go b/trie/iterator_test.go index 88b8103fb..54dd3069f 100644 --- a/trie/iterator_test.go +++ b/trie/iterator_test.go @@ -120,7 +120,7 @@ func TestNodeIteratorCoverage(t *testing.T) { } } } - it := db.diskdb.NewIterator() + it := db.diskdb.NewIterator(nil, nil) for it.Next() { key := it.Key() if _, ok := hashes[common.BytesToHash(key)]; !ok { @@ -312,7 +312,7 @@ func testIteratorContinueAfterError(t *testing.T, memonly bool) { if memonly { memKeys = triedb.Nodes() } else { - it := diskdb.NewIterator() + it := diskdb.NewIterator(nil, nil) for it.Next() { diskKeys = append(diskKeys, it.Key()) } diff --git a/trie/proof_test.go b/trie/proof_test.go index c488f342c..4caae7338 100644 --- a/trie/proof_test.go +++ b/trie/proof_test.go @@ -106,7 +106,7 @@ func TestBadProof(t *testing.T) { if proof == nil { t.Fatalf("prover %d: nil proof", i) } - it := proof.NewIterator() + it := proof.NewIterator(nil, nil) for i, d := 0, mrand.Intn(proof.Len()); i <= d; i++ { it.Next() } diff --git a/trie/sync_bloom.go b/trie/sync_bloom.go index 2182d1c43..3108b0593 100644 --- a/trie/sync_bloom.go +++ b/trie/sync_bloom.go @@ -99,7 +99,7 @@ func (b *SyncBloom) init(database ethdb.Iteratee) { // Note, this is fine, because everything inserted into leveldb by fast sync is // also pushed into the bloom directly, so we're not missing anything when the // iterator is swapped out for a new one. - it := database.NewIterator() + it := database.NewIterator(nil, nil) var ( start = time.Now() @@ -116,7 +116,7 @@ func (b *SyncBloom) init(database ethdb.Iteratee) { key := common.CopyBytes(it.Key()) it.Release() - it = database.NewIteratorWithStart(key) + it = database.NewIterator(nil, key) log.Info("Initializing fast sync bloom", "items", b.bloom.N(), "errorrate", b.errorRate(), "elapsed", common.PrettyDuration(time.Since(start))) swap = time.Now()