From 8604adb0c11c24c3e7797cc355ed6a9b46c6bf20 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Fri, 8 Apr 2022 11:44:11 +0800 Subject: [PATCH 01/30] core/state/snapshot: check dangling storages when generating snapshot --- core/state/snapshot/dangling.go | 105 ++++++++++++++++++++++++++++++++ core/state/snapshot/generate.go | 41 +++++++++---- 2 files changed, 135 insertions(+), 11 deletions(-) create mode 100644 core/state/snapshot/dangling.go diff --git a/core/state/snapshot/dangling.go b/core/state/snapshot/dangling.go new file mode 100644 index 000000000000..dc628237bc35 --- /dev/null +++ b/core/state/snapshot/dangling.go @@ -0,0 +1,105 @@ +// Copyright 2022 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package snapshot + +import ( + "bytes" + "time" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/rawdb" + "github.com/ethereum/go-ethereum/ethdb" +) + +// danglingRange describes the range for detecting dangling storages. +type danglingRange struct { + db ethdb.KeyValueStore // The database stores the snapshot data + start []byte // The start of the key range + limit []byte // The last of the key range + + result []common.Hash // The list of account hashes which have the dangling storages + duration time.Duration // Total time spent on the iteration +} + +// newDanglingRange initializes a dangling storage scanner and detects all the +// dangling accounts out. +func newDanglingRange(db ethdb.KeyValueStore, start, limit []byte) *danglingRange { + r := &danglingRange{ + db: db, + start: start, + limit: limit, + } + r.result, r.duration = r.detect() + snapDanglingStoragesCounter.Inc(int64(len(r.result))) + snapDanglingStoragesTimer.Update(r.duration) + return r +} + +// detect iterates the storage snapshot in the specified key range and +// returns a list of account hash of the dangling storages. Note both +// start and limit are included for iteration. +func (r *danglingRange) detect() ([]common.Hash, time.Duration) { + var ( + checked []byte + result []common.Hash + start = time.Now() + ) + iter := rawdb.NewKeyLengthIterator(r.db.NewIterator(rawdb.SnapshotStoragePrefix, r.start), len(rawdb.SnapshotStoragePrefix)+2*common.HashLength) + defer iter.Release() + + for iter.Next() { + account := iter.Key()[len(rawdb.SnapshotStoragePrefix) : len(rawdb.SnapshotStoragePrefix)+common.HashLength] + if r.limit != nil && bytes.Compare(account, r.limit) > 0 { + break + } + // Skip unnecessary checks for checked storage. + if bytes.Equal(account, checked) { + continue + } + checked = common.CopyBytes(account) + + // Check the presence of the corresponding account. + accountHash := common.BytesToHash(account) + data := rawdb.ReadAccountSnapshot(r.db, accountHash) + if len(data) != 0 { + continue + } + result = append(result, accountHash) + } + return result, time.Since(start) +} + +// cleanup wipes the dangling storages which fall within the range before the given key. +func (r *danglingRange) cleanup(limit []byte) error { + var ( + err error + wiped int + ) + for _, accountHash := range r.result { + if bytes.Compare(accountHash.Bytes(), limit) >= 0 { + break + } + prefix := append(rawdb.SnapshotStoragePrefix, accountHash.Bytes()...) + keyLen := len(rawdb.SnapshotStoragePrefix) + 2*common.HashLength + if err = wipeKeyRange(r.db, "storage", prefix, nil, nil, keyLen, snapWipedStorageMeter, false); err != nil { + break + } + wiped += 1 + } + r.result = r.result[wiped:] + return err +} diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 39d30a20c008..4c211e5e040f 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -74,6 +74,8 @@ var ( snapMissallStorageMeter = metrics.NewRegisteredMeter("state/snapshot/generation/storage/missall", nil) snapSuccessfulRangeProofMeter = metrics.NewRegisteredMeter("state/snapshot/generation/proof/success", nil) snapFailedRangeProofMeter = metrics.NewRegisteredMeter("state/snapshot/generation/proof/failure", nil) + snapDanglingStoragesCounter = metrics.NewRegisteredCounter("state/snapshot/generation/storage/dangling/counter", nil) + snapDanglingStoragesTimer = metrics.NewRegisteredTimer("state/snapshot/generation/storage/dangling/timer", nil) // snapAccountProveCounter measures time spent on the account proving snapAccountProveCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/account/prove", nil) @@ -371,12 +373,16 @@ func (dl *diskLayer) proveRange(stats *generatorStats, root common.Hash, prefix // onStateCallback is a function that is called by generateRange, when processing a range of // accounts or storage slots. For each element, the callback is invoked. -// If 'delete' is true, then this element (and potential slots) needs to be deleted from the snapshot. -// If 'write' is true, then this element needs to be updated with the 'val'. -// If 'write' is false, then this element is already correct, and needs no update. However, -// for accounts, the storage trie of the account needs to be checked. +// +// - If 'delete' is true, then this element (and potential slots) needs to be deleted from the snapshot. +// - If 'write' is true, then this element needs to be updated with the 'val'. +// - If 'write' is false, then this element is already correct, and needs no update. // The 'val' is the canonical encoding of the value (not the slim format for accounts) -type onStateCallback func(key []byte, val []byte, write bool, delete bool) error +// +// However, for accounts, the storage trie of the account needs to be checked. Also, +// dangling storages(storage exists but the corresponding account is missing) need to +// be cleaned up. The range between the prevKey +type onStateCallback func(key []byte, val []byte, r *danglingRange, write bool, delete bool) error // generateRange generates the state segment with particular prefix. Generation can // either verify the correctness of existing state through range-proof and skip @@ -404,7 +410,11 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, // The verification is passed, process each state with the given // callback function. If this state represents a contract, the // corresponding storage check will be performed in the callback - if err := result.forEach(func(key []byte, val []byte) error { return onState(key, val, false, false) }); err != nil { + var r *danglingRange + if kind != "storage" { + r = newDanglingRange(dl.diskdb, origin, result.last()) + } + if err := result.forEach(func(key []byte, val []byte) error { return onState(key, val, r, false, false) }); err != nil { return false, nil, err } // Only abort the iteration when both database and trie are exhausted @@ -466,6 +476,11 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, internal time.Duration ) nodeIt.AddResolver(snapNodeCache) + + var r *danglingRange + if kind != "storage" { + r = newDanglingRange(dl.diskdb, origin, result.last()) + } for iter.Next() { if last != nil && bytes.Compare(iter.Key, last) > 0 { trieMore = true @@ -478,7 +493,7 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, if cmp := bytes.Compare(kvkeys[0], iter.Key); cmp < 0 { // delete the key istart := time.Now() - if err := onState(kvkeys[0], nil, false, true); err != nil { + if err := onState(kvkeys[0], nil, r, false, true); err != nil { return false, nil, err } kvkeys = kvkeys[1:] @@ -500,7 +515,7 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, break } istart := time.Now() - if err := onState(iter.Key, iter.Value, write, false); err != nil { + if err := onState(iter.Key, iter.Value, r, write, false); err != nil { return false, nil, err } internal += time.Since(istart) @@ -511,7 +526,7 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, // Delete all stale snapshot states remaining istart := time.Now() for _, key := range kvkeys { - if err := onState(key, nil, false, true); err != nil { + if err := onState(key, nil, r, false, true); err != nil { return false, nil, err } deleted += 1 @@ -573,7 +588,7 @@ func (dl *diskLayer) checkAndFlush(current []byte, batch ethdb.Batch, stats *gen // generateStorages generates the missing storage slots of the specific contract. // It's supposed to restart the generation from the given origin position. func generateStorages(dl *diskLayer, account common.Hash, storageRoot common.Hash, storeMarker []byte, batch ethdb.Batch, stats *generatorStats, logged *time.Time) error { - onStorage := func(key []byte, val []byte, write bool, delete bool) error { + onStorage := func(key []byte, val []byte, r *danglingRange, write bool, delete bool) error { defer func(start time.Time) { snapStorageWriteCounter.Inc(time.Since(start).Nanoseconds()) }(time.Now()) @@ -620,11 +635,15 @@ func generateStorages(dl *diskLayer, account common.Hash, storageRoot common.Has // storage slots in the main trie. It's supposed to restart the generation // from the given origin position. func generateAccounts(dl *diskLayer, accMarker []byte, batch ethdb.Batch, stats *generatorStats, logged *time.Time) error { - onAccount := func(key []byte, val []byte, write bool, delete bool) error { + onAccount := func(key []byte, val []byte, r *danglingRange, write bool, delete bool) error { var ( start = time.Now() accountHash = common.BytesToHash(key) ) + // Clean up the dangling storages which have no corresponding accounts present. + if err := r.cleanup(key); err != nil { + return err + } if delete { rawdb.DeleteAccountSnapshot(batch, accountHash) snapWipedAccountMeter.Mark(1) From 51f8a0e0b953d18cc61357099ff9026a6e1376f5 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Fri, 8 Apr 2022 11:52:42 +0800 Subject: [PATCH 02/30] core/state/snapshot: polish --- core/state/snapshot/dangling.go | 35 +++++++++++++++++++++++++-------- core/state/snapshot/generate.go | 4 ++-- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/core/state/snapshot/dangling.go b/core/state/snapshot/dangling.go index dc628237bc35..d05557e1a68e 100644 --- a/core/state/snapshot/dangling.go +++ b/core/state/snapshot/dangling.go @@ -18,11 +18,14 @@ package snapshot import ( "bytes" + "fmt" "time" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/ethdb" + "github.com/ethereum/go-ethereum/log" ) // danglingRange describes the range for detecting dangling storages. @@ -37,26 +40,37 @@ type danglingRange struct { // newDanglingRange initializes a dangling storage scanner and detects all the // dangling accounts out. -func newDanglingRange(db ethdb.KeyValueStore, start, limit []byte) *danglingRange { +func newDanglingRange(db ethdb.KeyValueStore, start, limit []byte, report bool) *danglingRange { r := &danglingRange{ db: db, start: start, limit: limit, } - r.result, r.duration = r.detect() + r.result, r.duration = r.detect(report) snapDanglingStoragesCounter.Inc(int64(len(r.result))) snapDanglingStoragesTimer.Update(r.duration) + + if len(r.result) > 0 { + log.Warn("Detected dangling storages", "number", len(r.result), "start", hexutil.Encode(start), "limit", hexutil.Encode(limit), "elapsed", common.PrettyDuration(r.duration)) + } else { + logger := log.Debug + if report { + logger = log.Info + } + logger("Verified snapshot storages", "start", hexutil.Encode(start), "limit", hexutil.Encode(limit), "elapsed", common.PrettyDuration(r.duration)) + } return r } // detect iterates the storage snapshot in the specified key range and // returns a list of account hash of the dangling storages. Note both // start and limit are included for iteration. -func (r *danglingRange) detect() ([]common.Hash, time.Duration) { +func (r *danglingRange) detect(report bool) ([]common.Hash, time.Duration) { var ( - checked []byte - result []common.Hash - start = time.Now() + checked []byte + result []common.Hash + start = time.Now() + lastReport = time.Now() ) iter := rawdb.NewKeyLengthIterator(r.db.NewIterator(rawdb.SnapshotStoragePrefix, r.start), len(rawdb.SnapshotStoragePrefix)+2*common.HashLength) defer iter.Release() @@ -79,6 +93,11 @@ func (r *danglingRange) detect() ([]common.Hash, time.Duration) { continue } result = append(result, accountHash) + + if time.Since(lastReport) > time.Second*8 && report { + log.Info("Detecting dangling storage", "at", fmt.Sprintf("%#x", accountHash), "elapsed", common.PrettyDuration(time.Since(start))) + lastReport = time.Now() + } } return result, time.Since(start) } @@ -94,8 +113,8 @@ func (r *danglingRange) cleanup(limit []byte) error { break } prefix := append(rawdb.SnapshotStoragePrefix, accountHash.Bytes()...) - keyLen := len(rawdb.SnapshotStoragePrefix) + 2*common.HashLength - if err = wipeKeyRange(r.db, "storage", prefix, nil, nil, keyLen, snapWipedStorageMeter, false); err != nil { + keylen := len(rawdb.SnapshotStoragePrefix) + 2*common.HashLength + if err = wipeKeyRange(r.db, "storage", prefix, nil, nil, keylen, snapWipedStorageMeter, false); err != nil { break } wiped += 1 diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 4c211e5e040f..80b1eb364931 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -412,7 +412,7 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, // corresponding storage check will be performed in the callback var r *danglingRange if kind != "storage" { - r = newDanglingRange(dl.diskdb, origin, result.last()) + r = newDanglingRange(dl.diskdb, origin, result.last(), false) } if err := result.forEach(func(key []byte, val []byte) error { return onState(key, val, r, false, false) }); err != nil { return false, nil, err @@ -479,7 +479,7 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, var r *danglingRange if kind != "storage" { - r = newDanglingRange(dl.diskdb, origin, result.last()) + r = newDanglingRange(dl.diskdb, origin, result.last(), false) } for iter.Next() { if last != nil && bytes.Compare(iter.Key, last) > 0 { From 0ee1ebecec9de70af190c0400f2638eb925a5760 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Fri, 8 Apr 2022 14:16:14 +0800 Subject: [PATCH 03/30] core/state/snapshot: wipe the last part of the dangling storages --- core/state/snapshot/dangling.go | 4 +++- core/state/snapshot/generate.go | 11 ++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/core/state/snapshot/dangling.go b/core/state/snapshot/dangling.go index d05557e1a68e..ec13686dc207 100644 --- a/core/state/snapshot/dangling.go +++ b/core/state/snapshot/dangling.go @@ -47,6 +47,8 @@ func newDanglingRange(db ethdb.KeyValueStore, start, limit []byte, report bool) limit: limit, } r.result, r.duration = r.detect(report) + + // Update metrics snapDanglingStoragesCounter.Inc(int64(len(r.result))) snapDanglingStoragesTimer.Update(r.duration) @@ -109,7 +111,7 @@ func (r *danglingRange) cleanup(limit []byte) error { wiped int ) for _, accountHash := range r.result { - if bytes.Compare(accountHash.Bytes(), limit) >= 0 { + if limit != nil && bytes.Compare(accountHash.Bytes(), limit) >= 0 { break } prefix := append(rawdb.SnapshotStoragePrefix, accountHash.Bytes()...) diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 80b1eb364931..c07344b98004 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -620,13 +620,18 @@ func generateStorages(dl *diskLayer, account common.Hash, storageRoot common.Has if err != nil { return err // The procedure it aborted, either by external signal or internal error. } + if origin = increaseKey(last); origin == nil { + break // special case, the last is 0xffffffff...fff + } // Abort the procedure if the entire contract storage is generated if exhausted { + // Last step, cleanup the storages after the last generated + // account. All the left storages should be treated as dangling. + if err := newDanglingRange(dl.diskdb, origin, nil, false).cleanup(nil); err != nil { + return err + } break } - if origin = increaseKey(last); origin == nil { - break // special case, the last is 0xffffffff...fff - } } return nil } From 9b9dba18eb3daf34cce11e294c1328993b97cca0 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Fri, 8 Apr 2022 15:30:50 +0800 Subject: [PATCH 04/30] core/state/snapshot: fix and add tests --- core/state/snapshot/generate.go | 31 ++++--- core/state/snapshot/generate_test.go | 125 +++++++++++++++++++++++++++ 2 files changed, 145 insertions(+), 11 deletions(-) diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index c07344b98004..79935647969d 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -542,6 +542,15 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, logger.Debug("Regenerated state range", "root", root, "last", hexutil.Encode(last), "count", count, "created", created, "updated", updated, "untouched", untouched, "deleted", deleted) + // Clean up the left dangling storages at the end in case the entire + // snapshot is regenerated. Note if the last is nil, it means there + // is no snapshot data at disk at all, the entire snapshot is expected + // to be re-generated already. + if last == nil && r != nil { + if err := r.cleanup(nil); err != nil { + return false, nil, err + } + } // If there are either more trie items, or there are more snap items // (in the next segment), then we need to keep working return !trieMore && !result.diskMore, last, nil @@ -620,18 +629,13 @@ func generateStorages(dl *diskLayer, account common.Hash, storageRoot common.Has if err != nil { return err // The procedure it aborted, either by external signal or internal error. } - if origin = increaseKey(last); origin == nil { - break // special case, the last is 0xffffffff...fff - } // Abort the procedure if the entire contract storage is generated if exhausted { - // Last step, cleanup the storages after the last generated - // account. All the left storages should be treated as dangling. - if err := newDanglingRange(dl.diskdb, origin, nil, false).cleanup(nil); err != nil { - return err - } break } + if origin = increaseKey(last); origin == nil { + break // special case, the last is 0xffffffff...fff + } } return nil } @@ -743,13 +747,18 @@ func generateAccounts(dl *diskLayer, accMarker []byte, batch ethdb.Batch, stats if err != nil { return err // The procedure it aborted, either by external signal or internal error. } + if origin = increaseKey(last); origin == nil { + break // special case, the last is 0xffffffff...fff + } // Abort the procedure if the entire snapshot is generated if exhausted { + // Last step, cleanup the storages after the last account. + // All the left storages should be treated as dangling. + if err := newDanglingRange(dl.diskdb, origin, nil, false).cleanup(nil); err != nil { + return err + } break } - if origin = increaseKey(last); origin == nil { - break // special case, the last is 0xffffffff...fff - } accountRange = accountCheckRange } return nil diff --git a/core/state/snapshot/generate_test.go b/core/state/snapshot/generate_test.go index 582da6a2e7e2..9fe613dd275f 100644 --- a/core/state/snapshot/generate_test.go +++ b/core/state/snapshot/generate_test.go @@ -148,8 +148,10 @@ func TestGenerateExistentState(t *testing.T) { func checkSnapRoot(t *testing.T, snap *diskLayer, trieRoot common.Hash) { t.Helper() + accIt := snap.AccountIterator(common.Hash{}) defer accIt.Release() + snapRoot, err := generateTrieRoot(nil, accIt, common.Hash{}, stackTrieGenerate, func(db ethdb.KeyValueWriter, accountHash, codeHash common.Hash, stat *generateStats) (common.Hash, error) { storageIt, _ := snap.StorageIterator(accountHash, common.Hash{}) @@ -168,6 +170,10 @@ func checkSnapRoot(t *testing.T, snap *diskLayer, trieRoot common.Hash) { if snapRoot != trieRoot { t.Fatalf("snaproot: %#x != trieroot #%x", snapRoot, trieRoot) } + scanner := newDanglingRange(snap.diskdb, nil, nil, false) + if len(scanner.result) != 0 { + t.Fatalf("Detected dangling storages %d", len(scanner.result)) + } } type testHelper struct { @@ -831,3 +837,122 @@ func TestGenerateWithIncompleteStorage(t *testing.T) { snap.genAbort <- stop <-stop } + +func incKey(key []byte) []byte { + for i := len(key) - 1; i >= 0; i-- { + key[i]++ + if key[i] != 0x0 { + break + } + } + return key +} + +func decKey(key []byte) []byte { + for i := len(key) - 1; i >= 0; i-- { + key[i]-- + if key[i] != 0xff { + break + } + } + return key +} + +func populateDangling(disk ethdb.KeyValueStore) { + populate := func(accountHash common.Hash, keys []string, vals []string) { + for i, key := range keys { + rawdb.WriteStorageSnapshot(disk, accountHash, hashData([]byte(key)), []byte(vals[i])) + } + } + // Dangling storages of the "first" account + populate(common.Hash{}, []string{"key-1", "key-2", "key-3"}, []string{"val-1", "val-2", "val-3"}) + + // Dangling storages of the "last" account + populate(common.HexToHash("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"), []string{"key-1", "key-2", "key-3"}, []string{"val-1", "val-2", "val-3"}) + + // Dangling storages around the account 1 + hash := decKey(hashData([]byte("acc-1")).Bytes()) + populate(common.BytesToHash(hash), []string{"key-1", "key-2", "key-3"}, []string{"val-1", "val-2", "val-3"}) + hash = incKey(hashData([]byte("acc-1")).Bytes()) + populate(common.BytesToHash(hash), []string{"key-1", "key-2", "key-3"}, []string{"val-1", "val-2", "val-3"}) + + // Dangling storages around the account 2 + hash = decKey(hashData([]byte("acc-2")).Bytes()) + populate(common.BytesToHash(hash), []string{"key-1", "key-2", "key-3"}, []string{"val-1", "val-2", "val-3"}) + hash = incKey(hashData([]byte("acc-2")).Bytes()) + populate(common.BytesToHash(hash), []string{"key-1", "key-2", "key-3"}, []string{"val-1", "val-2", "val-3"}) + + // Dangling storages around the account 3 + hash = decKey(hashData([]byte("acc-3")).Bytes()) + populate(common.BytesToHash(hash), []string{"key-1", "key-2", "key-3"}, []string{"val-1", "val-2", "val-3"}) + hash = incKey(hashData([]byte("acc-3")).Bytes()) + populate(common.BytesToHash(hash), []string{"key-1", "key-2", "key-3"}, []string{"val-1", "val-2", "val-3"}) + + // Dangling storages of the random account + populate(randomHash(), []string{"key-1", "key-2", "key-3"}, []string{"val-1", "val-2", "val-3"}) + populate(randomHash(), []string{"key-1", "key-2", "key-3"}, []string{"val-1", "val-2", "val-3"}) + populate(randomHash(), []string{"key-1", "key-2", "key-3"}, []string{"val-1", "val-2", "val-3"}) +} + +// Tests that snapshot generation with dangling storages. Dangling storage means +// the storage data is existent while the corresponding account data is missing. +// +// This test will populate some dangling storages to see if they can be cleaned up. +func TestGenerateCompleteSnapshotWithDanglingStorage(t *testing.T) { + var helper = newHelper() + stRoot := helper.makeStorageTrie([]string{"key-1", "key-2", "key-3"}, []string{"val-1", "val-2", "val-3"}) + + helper.addAccount("acc-1", &Account{Balance: big.NewInt(1), Root: stRoot, CodeHash: emptyCode.Bytes()}) + helper.addAccount("acc-2", &Account{Balance: big.NewInt(1), Root: emptyRoot.Bytes(), CodeHash: emptyCode.Bytes()}) + helper.addAccount("acc-3", &Account{Balance: big.NewInt(1), Root: stRoot, CodeHash: emptyCode.Bytes()}) + + helper.addSnapStorage("acc-1", []string{"key-1", "key-2", "key-3"}, []string{"val-1", "val-2", "val-3"}) + helper.addSnapStorage("acc-3", []string{"key-1", "key-2", "key-3"}, []string{"val-1", "val-2", "val-3"}) + + populateDangling(helper.diskdb) + + root, snap := helper.Generate() + select { + case <-snap.genPending: + // Snapshot generation succeeded + + case <-time.After(3 * time.Second): + t.Errorf("Snapshot generation failed") + } + checkSnapRoot(t, snap, root) + + // Signal abortion to the generator and wait for it to tear down + stop := make(chan *generatorStats) + snap.genAbort <- stop + <-stop +} + +// Tests that snapshot generation with dangling storages. Dangling storage means +// the storage data is existent while the corresponding account data is missing. +// +// This test will populate some dangling storages to see if they can be cleaned up. +func TestGenerateBrokenSnapshotWithDanglingStorage(t *testing.T) { + var helper = newHelper() + stRoot := helper.makeStorageTrie([]string{"key-1", "key-2", "key-3"}, []string{"val-1", "val-2", "val-3"}) + + helper.addTrieAccount("acc-1", &Account{Balance: big.NewInt(1), Root: stRoot, CodeHash: emptyCode.Bytes()}) + helper.addTrieAccount("acc-2", &Account{Balance: big.NewInt(2), Root: emptyRoot.Bytes(), CodeHash: emptyCode.Bytes()}) + helper.addTrieAccount("acc-3", &Account{Balance: big.NewInt(3), Root: stRoot, CodeHash: emptyCode.Bytes()}) + + populateDangling(helper.diskdb) + + root, snap := helper.Generate() + select { + case <-snap.genPending: + // Snapshot generation succeeded + + case <-time.After(3 * time.Second): + t.Errorf("Snapshot generation failed") + } + checkSnapRoot(t, snap, root) + + // Signal abortion to the generator and wait for it to tear down + stop := make(chan *generatorStats) + snap.genAbort <- stop + <-stop +} From 617eda9bef41e67e76498d06ba9525c4e391d9af Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Fri, 8 Apr 2022 15:33:23 +0800 Subject: [PATCH 05/30] core/state/snapshot: fix comment --- core/state/snapshot/generate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 79935647969d..8cbf680a9af1 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -381,7 +381,7 @@ func (dl *diskLayer) proveRange(stats *generatorStats, root common.Hash, prefix // // However, for accounts, the storage trie of the account needs to be checked. Also, // dangling storages(storage exists but the corresponding account is missing) need to -// be cleaned up. The range between the prevKey +// be cleaned up. type onStateCallback func(key []byte, val []byte, r *danglingRange, write bool, delete bool) error // generateRange generates the state segment with particular prefix. Generation can From bbb6b94cfa0a1b0820fe0e8e0d79c9ff8df0d571 Mon Sep 17 00:00:00 2001 From: Tbnoapi <63448616+nuoomnoy02@users.noreply.github.com> Date: Thu, 7 Apr 2022 13:23:55 +0700 Subject: [PATCH 06/30] README: remove mentions of fast sync (#24656) Co-authored-by: Marius van der Wijden From 816f6cb6a37c4afb713fcd78c7355cb6d9e422ae Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Tue, 26 Apr 2022 10:25:05 +0800 Subject: [PATCH 07/30] core, cmd: expose dangling storage detector for wider usage --- core/state/snapshot/dangling.go | 17 ++++++++++------- core/state/snapshot/generate.go | 16 ++++++++-------- core/state/snapshot/generate_test.go | 2 +- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/core/state/snapshot/dangling.go b/core/state/snapshot/dangling.go index ec13686dc207..b896c2903ce8 100644 --- a/core/state/snapshot/dangling.go +++ b/core/state/snapshot/dangling.go @@ -28,8 +28,8 @@ import ( "github.com/ethereum/go-ethereum/log" ) -// danglingRange describes the range for detecting dangling storages. -type danglingRange struct { +// DanglingRange describes the range for detecting dangling storages. +type DanglingRange struct { db ethdb.KeyValueStore // The database stores the snapshot data start []byte // The start of the key range limit []byte // The last of the key range @@ -38,10 +38,10 @@ type danglingRange struct { duration time.Duration // Total time spent on the iteration } -// newDanglingRange initializes a dangling storage scanner and detects all the +// NewDanglingRange initializes a dangling storage scanner and detects all the // dangling accounts out. -func newDanglingRange(db ethdb.KeyValueStore, start, limit []byte, report bool) *danglingRange { - r := &danglingRange{ +func NewDanglingRange(db ethdb.KeyValueStore, start, limit []byte, report bool) *DanglingRange { + r := &DanglingRange{ db: db, start: start, limit: limit, @@ -67,7 +67,7 @@ func newDanglingRange(db ethdb.KeyValueStore, start, limit []byte, report bool) // detect iterates the storage snapshot in the specified key range and // returns a list of account hash of the dangling storages. Note both // start and limit are included for iteration. -func (r *danglingRange) detect(report bool) ([]common.Hash, time.Duration) { +func (r *DanglingRange) detect(report bool) ([]common.Hash, time.Duration) { var ( checked []byte result []common.Hash @@ -96,6 +96,9 @@ func (r *danglingRange) detect(report bool) ([]common.Hash, time.Duration) { } result = append(result, accountHash) + if report { + log.Warn("Dangling storage - missing account", "account", fmt.Sprintf("%#x", accountHash)) + } if time.Since(lastReport) > time.Second*8 && report { log.Info("Detecting dangling storage", "at", fmt.Sprintf("%#x", accountHash), "elapsed", common.PrettyDuration(time.Since(start))) lastReport = time.Now() @@ -105,7 +108,7 @@ func (r *danglingRange) detect(report bool) ([]common.Hash, time.Duration) { } // cleanup wipes the dangling storages which fall within the range before the given key. -func (r *danglingRange) cleanup(limit []byte) error { +func (r *DanglingRange) cleanup(limit []byte) error { var ( err error wiped int diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 8cbf680a9af1..3871d22fdfb9 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -382,7 +382,7 @@ func (dl *diskLayer) proveRange(stats *generatorStats, root common.Hash, prefix // However, for accounts, the storage trie of the account needs to be checked. Also, // dangling storages(storage exists but the corresponding account is missing) need to // be cleaned up. -type onStateCallback func(key []byte, val []byte, r *danglingRange, write bool, delete bool) error +type onStateCallback func(key []byte, val []byte, r *DanglingRange, write bool, delete bool) error // generateRange generates the state segment with particular prefix. Generation can // either verify the correctness of existing state through range-proof and skip @@ -410,9 +410,9 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, // The verification is passed, process each state with the given // callback function. If this state represents a contract, the // corresponding storage check will be performed in the callback - var r *danglingRange + var r *DanglingRange if kind != "storage" { - r = newDanglingRange(dl.diskdb, origin, result.last(), false) + r = NewDanglingRange(dl.diskdb, origin, result.last(), false) } if err := result.forEach(func(key []byte, val []byte) error { return onState(key, val, r, false, false) }); err != nil { return false, nil, err @@ -477,9 +477,9 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, ) nodeIt.AddResolver(snapNodeCache) - var r *danglingRange + var r *DanglingRange if kind != "storage" { - r = newDanglingRange(dl.diskdb, origin, result.last(), false) + r = NewDanglingRange(dl.diskdb, origin, result.last(), false) } for iter.Next() { if last != nil && bytes.Compare(iter.Key, last) > 0 { @@ -597,7 +597,7 @@ func (dl *diskLayer) checkAndFlush(current []byte, batch ethdb.Batch, stats *gen // generateStorages generates the missing storage slots of the specific contract. // It's supposed to restart the generation from the given origin position. func generateStorages(dl *diskLayer, account common.Hash, storageRoot common.Hash, storeMarker []byte, batch ethdb.Batch, stats *generatorStats, logged *time.Time) error { - onStorage := func(key []byte, val []byte, r *danglingRange, write bool, delete bool) error { + onStorage := func(key []byte, val []byte, r *DanglingRange, write bool, delete bool) error { defer func(start time.Time) { snapStorageWriteCounter.Inc(time.Since(start).Nanoseconds()) }(time.Now()) @@ -644,7 +644,7 @@ func generateStorages(dl *diskLayer, account common.Hash, storageRoot common.Has // storage slots in the main trie. It's supposed to restart the generation // from the given origin position. func generateAccounts(dl *diskLayer, accMarker []byte, batch ethdb.Batch, stats *generatorStats, logged *time.Time) error { - onAccount := func(key []byte, val []byte, r *danglingRange, write bool, delete bool) error { + onAccount := func(key []byte, val []byte, r *DanglingRange, write bool, delete bool) error { var ( start = time.Now() accountHash = common.BytesToHash(key) @@ -754,7 +754,7 @@ func generateAccounts(dl *diskLayer, accMarker []byte, batch ethdb.Batch, stats if exhausted { // Last step, cleanup the storages after the last account. // All the left storages should be treated as dangling. - if err := newDanglingRange(dl.diskdb, origin, nil, false).cleanup(nil); err != nil { + if err := NewDanglingRange(dl.diskdb, origin, nil, false).cleanup(nil); err != nil { return err } break diff --git a/core/state/snapshot/generate_test.go b/core/state/snapshot/generate_test.go index 9fe613dd275f..5196f267844d 100644 --- a/core/state/snapshot/generate_test.go +++ b/core/state/snapshot/generate_test.go @@ -170,7 +170,7 @@ func checkSnapRoot(t *testing.T, snap *diskLayer, trieRoot common.Hash) { if snapRoot != trieRoot { t.Fatalf("snaproot: %#x != trieroot #%x", snapRoot, trieRoot) } - scanner := newDanglingRange(snap.diskdb, nil, nil, false) + scanner := NewDanglingRange(snap.diskdb, nil, nil, false) if len(scanner.result) != 0 { t.Fatalf("Detected dangling storages %d", len(scanner.result)) } From af23c13c82d3608a5e501082e9ea54ceaf9bea00 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Tue, 26 Apr 2022 10:48:27 +0800 Subject: [PATCH 08/30] core/state/snapshot: rename variable --- core/state/snapshot/generate.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 3871d22fdfb9..42562936fdd4 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -410,11 +410,11 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, // The verification is passed, process each state with the given // callback function. If this state represents a contract, the // corresponding storage check will be performed in the callback - var r *DanglingRange + var drange *DanglingRange if kind != "storage" { - r = NewDanglingRange(dl.diskdb, origin, result.last(), false) + drange = NewDanglingRange(dl.diskdb, origin, result.last(), false) } - if err := result.forEach(func(key []byte, val []byte) error { return onState(key, val, r, false, false) }); err != nil { + if err := result.forEach(func(key []byte, val []byte) error { return onState(key, val, drange, false, false) }); err != nil { return false, nil, err } // Only abort the iteration when both database and trie are exhausted @@ -477,9 +477,9 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, ) nodeIt.AddResolver(snapNodeCache) - var r *DanglingRange + var drange *DanglingRange if kind != "storage" { - r = NewDanglingRange(dl.diskdb, origin, result.last(), false) + drange = NewDanglingRange(dl.diskdb, origin, result.last(), false) } for iter.Next() { if last != nil && bytes.Compare(iter.Key, last) > 0 { @@ -493,7 +493,7 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, if cmp := bytes.Compare(kvkeys[0], iter.Key); cmp < 0 { // delete the key istart := time.Now() - if err := onState(kvkeys[0], nil, r, false, true); err != nil { + if err := onState(kvkeys[0], nil, drange, false, true); err != nil { return false, nil, err } kvkeys = kvkeys[1:] @@ -515,7 +515,7 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, break } istart := time.Now() - if err := onState(iter.Key, iter.Value, r, write, false); err != nil { + if err := onState(iter.Key, iter.Value, drange, write, false); err != nil { return false, nil, err } internal += time.Since(istart) @@ -526,7 +526,7 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, // Delete all stale snapshot states remaining istart := time.Now() for _, key := range kvkeys { - if err := onState(key, nil, r, false, true); err != nil { + if err := onState(key, nil, drange, false, true); err != nil { return false, nil, err } deleted += 1 @@ -546,8 +546,8 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, // snapshot is regenerated. Note if the last is nil, it means there // is no snapshot data at disk at all, the entire snapshot is expected // to be re-generated already. - if last == nil && r != nil { - if err := r.cleanup(nil); err != nil { + if last == nil && drange != nil { + if err := drange.cleanup(nil); err != nil { return false, nil, err } } From 87d8bc38a84db9ad62bc650585249169a1c1f746 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Thu, 28 Apr 2022 14:46:55 +0800 Subject: [PATCH 09/30] core, ethdb: use global iterators for snapshot generation --- core/rawdb/table.go | 6 + core/state/snapshot/context.go | 193 +++++++++++++++++++++ core/state/snapshot/dangling.go | 4 - core/state/snapshot/generate.go | 294 ++++++++++---------------------- core/state/snapshot/metrics.go | 50 ++++++ ethdb/iterator.go | 4 + ethdb/memorydb/memorydb.go | 55 ++++-- 7 files changed, 382 insertions(+), 224 deletions(-) create mode 100644 core/state/snapshot/context.go create mode 100644 core/state/snapshot/metrics.go diff --git a/core/rawdb/table.go b/core/rawdb/table.go index 6d6fa0555da9..9fb32cd182b4 100644 --- a/core/rawdb/table.go +++ b/core/rawdb/table.go @@ -270,6 +270,12 @@ type tableIterator struct { prefix string } +// Prev moves the iterator to the previous key/value pair. It returns false +// if the iterator is exhausted. +func (iter *tableIterator) Prev() bool { + return iter.iter.Prev() +} + // Next moves the iterator to the next key/value pair. It returns whether the // iterator is exhausted. func (iter *tableIterator) Next() bool { diff --git a/core/state/snapshot/context.go b/core/state/snapshot/context.go new file mode 100644 index 000000000000..bca280509d69 --- /dev/null +++ b/core/state/snapshot/context.go @@ -0,0 +1,193 @@ +// Copyright 2022 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package snapshot + +import ( + "bytes" + "encoding/binary" + "errors" + "time" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/math" + "github.com/ethereum/go-ethereum/core/rawdb" + "github.com/ethereum/go-ethereum/ethdb" + "github.com/ethereum/go-ethereum/log" +) + +const ( + snapAccount = "account" // Identifier of account snapshot generation + snapStorage = "storage" // Identifier of storage snapshot generation +) + +// generatorStats is a collection of statistics gathered by the snapshot generator +// for logging purposes. +type generatorStats struct { + origin uint64 // Origin prefix where generation started + start time.Time // Timestamp when generation started + accounts uint64 // Number of accounts indexed(generated or recovered) + slots uint64 // Number of storage slots indexed(generated or recovered) + storage common.StorageSize // Total account and storage slot size(generation or recovery) +} + +// Log creates an contextual log with the given message and the context pulled +// from the internally maintained statistics. +func (gs *generatorStats) Log(msg string, root common.Hash, marker []byte) { + var ctx []interface{} + if root != (common.Hash{}) { + ctx = append(ctx, []interface{}{"root", root}...) + } + // Figure out whether we're after or within an account + switch len(marker) { + case common.HashLength: + ctx = append(ctx, []interface{}{"at", common.BytesToHash(marker)}...) + case 2 * common.HashLength: + ctx = append(ctx, []interface{}{ + "in", common.BytesToHash(marker[:common.HashLength]), + "at", common.BytesToHash(marker[common.HashLength:]), + }...) + } + // Add the usual measurements + ctx = append(ctx, []interface{}{ + "accounts", gs.accounts, + "slots", gs.slots, + "storage", gs.storage, + "elapsed", common.PrettyDuration(time.Since(gs.start)), + }...) + // Calculate the estimated indexing time based on current stats + if len(marker) > 0 { + if done := binary.BigEndian.Uint64(marker[:8]) - gs.origin; done > 0 { + left := math.MaxUint64 - binary.BigEndian.Uint64(marker[:8]) + + speed := done/uint64(time.Since(gs.start)/time.Millisecond+1) + 1 // +1s to avoid division by zero + ctx = append(ctx, []interface{}{ + "eta", common.PrettyDuration(time.Duration(left/speed) * time.Millisecond), + }...) + } + } + log.Info(msg, ctx...) +} + +// generatorContext carries a few global values to be shared by all generation functions. +type generatorContext struct { + stats *generatorStats // Generation statistic collection + db ethdb.KeyValueStore // Key-value store containing the snapshot data + account ethdb.Iterator // Iterator of account snapshot data + storage ethdb.Iterator // Iterator of storage snapshot data + batch ethdb.Batch // Database batch for writing batch data atomically + logged time.Time // The timestamp when last generation progress was displayed +} + +// newGeneratorContext initializes the context for generation. +func newGeneratorContext(stats *generatorStats, db ethdb.KeyValueStore, accMarker []byte) *generatorContext { + // Construct global account and storage snapshot iterators at the + // interrupted position. These iterators will be reopened from time + // to time to avoid blocking leveldb compaction for a long time. + // + // Note for the storage iterator, we use the interrupted account hash as + // the iteration start point instead of the storage interruption position, + // because this account can be deleted since last generation and storages + // need to wiped in this case. + var ( + acctIter = db.NewIterator(rawdb.SnapshotAccountPrefix, accMarker) + storageIter = db.NewIterator(rawdb.SnapshotStoragePrefix, accMarker) + acctIt = rawdb.NewKeyLengthIterator(acctIter, 1+common.HashLength) + storageIt = rawdb.NewKeyLengthIterator(storageIter, 1+2*common.HashLength) + ) + return &generatorContext{ + stats: stats, + db: db, + account: acctIt, + storage: storageIt, + batch: db.NewBatch(), + logged: time.Now(), + } +} + +// close releases all the held resources. +func (ctx *generatorContext) close() { + ctx.account.Release() + ctx.storage.Release() +} + +// iterator returns the corresponding iterator specified by the kind. +func (ctx *generatorContext) iterator(kind string) ethdb.Iterator { + if kind == snapAccount { + return ctx.account + } + return ctx.storage +} + +// reopen re-constructs the database iterator with given start position. +func (ctx *generatorContext) reopen(kind string, prefix []byte, start []byte) { + iter := ctx.iterator(kind) + iter.Release() + + iter = ctx.db.NewIterator(prefix, start) + if kind == snapStorage { + ctx.storage = rawdb.NewKeyLengthIterator(iter, 1+2*common.HashLength) + } else { + ctx.account = rawdb.NewKeyLengthIterator(iter, 1+common.HashLength) + } +} + +// removeStorageBefore starting from the current iterator position, iterate and +// delete all storage snapshots before the specified account. When the iterator +// touches the storage located in the account range, or the storage is larger +// than the account range, it stops and moves back the iterator a step. +func (ctx *generatorContext) removeStorageBefore(account common.Hash) { + iter := ctx.storage + for iter.Next() { + key := iter.Key() + if bytes.Compare(key[1:1+common.HashLength], account.Bytes()) >= 0 { + iter.Prev() + return + } + ctx.batch.Delete(key) + } +} + +// removeStorageAt starting from the current iterator position, iterate and +// delete all storage snapshots located in the specified account range. When +// the iterator touches the storage is larger than the account range, it stops +// and moves back the iterator a step. An error will be returned if the initial +// position of iterator is not in the specified account range. +func (ctx *generatorContext) removeStorageAt(account common.Hash) error { + iter := ctx.iterator(snapStorage) + for iter.Next() { + key := iter.Key() + cmp := bytes.Compare(key[1:1+common.HashLength], account.Bytes()) + if cmp < 0 { + return errors.New("invalid iterator position") + } + if cmp > 0 { + iter.Prev() + break + } + ctx.batch.Delete(key) + } + return nil +} + +// removeAllStorage starting from the current iterator position, iterate and +// delete all storage snapshots left. +func (ctx *generatorContext) removeAllStorage() { + iter := ctx.iterator(snapStorage) + for iter.Next() { + ctx.batch.Delete(iter.Key()) + } +} diff --git a/core/state/snapshot/dangling.go b/core/state/snapshot/dangling.go index b896c2903ce8..b5ae59f9b858 100644 --- a/core/state/snapshot/dangling.go +++ b/core/state/snapshot/dangling.go @@ -48,10 +48,6 @@ func NewDanglingRange(db ethdb.KeyValueStore, start, limit []byte, report bool) } r.result, r.duration = r.detect(report) - // Update metrics - snapDanglingStoragesCounter.Inc(int64(len(r.result))) - snapDanglingStoragesTimer.Update(r.duration) - if len(r.result) > 0 { log.Warn("Detected dangling storages", "number", len(r.result), "start", hexutil.Encode(start), "limit", hexutil.Encode(limit), "elapsed", common.PrettyDuration(r.duration)) } else { diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 42562936fdd4..1f0b3cf82430 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -18,7 +18,6 @@ package snapshot import ( "bytes" - "encoding/binary" "errors" "fmt" "math/big" @@ -27,13 +26,11 @@ import ( "github.com/VictoriaMetrics/fastcache" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" - "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/ethdb/memorydb" "github.com/ethereum/go-ethereum/log" - "github.com/ethereum/go-ethereum/metrics" "github.com/ethereum/go-ethereum/rlp" "github.com/ethereum/go-ethereum/trie" ) @@ -47,14 +44,14 @@ var ( // accountCheckRange is the upper limit of the number of accounts involved in // each range check. This is a value estimated based on experience. If this - // value is too large, the failure rate of range prove will increase. Otherwise - // the value is too small, the efficiency of the state recovery will decrease. + // range is too large, the failure rate of range proof will increase. Otherwise, + // if the range is too small, the efficiency of the state recovery will decrease. accountCheckRange = 128 // storageCheckRange is the upper limit of the number of storage slots involved // in each range check. This is a value estimated based on experience. If this - // value is too large, the failure rate of range prove will increase. Otherwise - // the value is too small, the efficiency of the state recovery will decrease. + // range is too large, the failure rate of range proof will increase. Otherwise, + // if the range is too small, the efficiency of the state recovery will decrease. storageCheckRange = 1024 // errMissingTrie is returned if the target trie is missing while the generation @@ -62,87 +59,6 @@ var ( errMissingTrie = errors.New("missing trie") ) -// Metrics in generation -var ( - snapGeneratedAccountMeter = metrics.NewRegisteredMeter("state/snapshot/generation/account/generated", nil) - snapRecoveredAccountMeter = metrics.NewRegisteredMeter("state/snapshot/generation/account/recovered", nil) - snapWipedAccountMeter = metrics.NewRegisteredMeter("state/snapshot/generation/account/wiped", nil) - snapMissallAccountMeter = metrics.NewRegisteredMeter("state/snapshot/generation/account/missall", nil) - snapGeneratedStorageMeter = metrics.NewRegisteredMeter("state/snapshot/generation/storage/generated", nil) - snapRecoveredStorageMeter = metrics.NewRegisteredMeter("state/snapshot/generation/storage/recovered", nil) - snapWipedStorageMeter = metrics.NewRegisteredMeter("state/snapshot/generation/storage/wiped", nil) - snapMissallStorageMeter = metrics.NewRegisteredMeter("state/snapshot/generation/storage/missall", nil) - snapSuccessfulRangeProofMeter = metrics.NewRegisteredMeter("state/snapshot/generation/proof/success", nil) - snapFailedRangeProofMeter = metrics.NewRegisteredMeter("state/snapshot/generation/proof/failure", nil) - snapDanglingStoragesCounter = metrics.NewRegisteredCounter("state/snapshot/generation/storage/dangling/counter", nil) - snapDanglingStoragesTimer = metrics.NewRegisteredTimer("state/snapshot/generation/storage/dangling/timer", nil) - - // snapAccountProveCounter measures time spent on the account proving - snapAccountProveCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/account/prove", nil) - // snapAccountTrieReadCounter measures time spent on the account trie iteration - snapAccountTrieReadCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/account/trieread", nil) - // snapAccountSnapReadCounter measues time spent on the snapshot account iteration - snapAccountSnapReadCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/account/snapread", nil) - // snapAccountWriteCounter measures time spent on writing/updating/deleting accounts - snapAccountWriteCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/account/write", nil) - // snapStorageProveCounter measures time spent on storage proving - snapStorageProveCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/storage/prove", nil) - // snapStorageTrieReadCounter measures time spent on the storage trie iteration - snapStorageTrieReadCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/storage/trieread", nil) - // snapStorageSnapReadCounter measures time spent on the snapshot storage iteration - snapStorageSnapReadCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/storage/snapread", nil) - // snapStorageWriteCounter measures time spent on writing/updating/deleting storages - snapStorageWriteCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/storage/write", nil) -) - -// generatorStats is a collection of statistics gathered by the snapshot generator -// for logging purposes. -type generatorStats struct { - origin uint64 // Origin prefix where generation started - start time.Time // Timestamp when generation started - accounts uint64 // Number of accounts indexed(generated or recovered) - slots uint64 // Number of storage slots indexed(generated or recovered) - storage common.StorageSize // Total account and storage slot size(generation or recovery) -} - -// Log creates an contextual log with the given message and the context pulled -// from the internally maintained statistics. -func (gs *generatorStats) Log(msg string, root common.Hash, marker []byte) { - var ctx []interface{} - if root != (common.Hash{}) { - ctx = append(ctx, []interface{}{"root", root}...) - } - // Figure out whether we're after or within an account - switch len(marker) { - case common.HashLength: - ctx = append(ctx, []interface{}{"at", common.BytesToHash(marker)}...) - case 2 * common.HashLength: - ctx = append(ctx, []interface{}{ - "in", common.BytesToHash(marker[:common.HashLength]), - "at", common.BytesToHash(marker[common.HashLength:]), - }...) - } - // Add the usual measurements - ctx = append(ctx, []interface{}{ - "accounts", gs.accounts, - "slots", gs.slots, - "storage", gs.storage, - "elapsed", common.PrettyDuration(time.Since(gs.start)), - }...) - // Calculate the estimated indexing time based on current stats - if len(marker) > 0 { - if done := binary.BigEndian.Uint64(marker[:8]) - gs.origin; done > 0 { - left := math.MaxUint64 - binary.BigEndian.Uint64(marker[:8]) - - speed := done/uint64(time.Since(gs.start)/time.Millisecond+1) + 1 // +1s to avoid division by zero - ctx = append(ctx, []interface{}{ - "eta", common.PrettyDuration(time.Duration(left/speed) * time.Millisecond), - }...) - } - } - log.Info(msg, ctx...) -} - // generateSnapshot regenerates a brand new snapshot based on an existing state // database and head block asynchronously. The snapshot is returned immediately // and generation is continued in the background until done. @@ -250,25 +166,35 @@ func (result *proofResult) forEach(callback func(key []byte, val []byte) error) // // The proof result will be returned if the range proving is finished, otherwise // the error will be returned to abort the entire procedure. -func (dl *diskLayer) proveRange(stats *generatorStats, root common.Hash, prefix []byte, kind string, origin []byte, max int, valueConvertFn func([]byte) ([]byte, error)) (*proofResult, error) { +func (dl *diskLayer) proveRange(ctx *generatorContext, root common.Hash, prefix []byte, kind string, origin []byte, max int, valueConvertFn func([]byte) ([]byte, error)) (*proofResult, error) { var ( keys [][]byte vals [][]byte proof = rawdb.NewMemoryDatabase() diskMore = false + iter = ctx.iterator(kind) + start = time.Now() + min = append(prefix, origin...) ) - iter := dl.diskdb.NewIterator(prefix, origin) - defer iter.Release() - - var start = time.Now() for iter.Next() { + // Ensure the iterated item always larger than the given origin. key := iter.Key() - if len(key) != len(prefix)+common.HashLength { - continue + if bytes.Compare(key, min) < 0 { + return nil, errors.New("invalid iteration position") + } + // Ensure the iterated item still fall in the specified prefix. If + // not which means the items in the specified area are all visited. + // Move the iterator a step back since we iterate one extra element + // out. + if !bytes.Equal(key[:len(prefix)], prefix) { + iter.Prev() + break } + // Break if we've reached the max size, and signal that we're not + // done yet. Move the iterator a step back since we iterate one + // extra element out. if len(keys) == max { - // Break if we've reached the max size, and signal that we're not - // done yet. + iter.Prev() diskMore = true break } @@ -284,7 +210,7 @@ func (dl *diskLayer) proveRange(stats *generatorStats, root common.Hash, prefix // generation to heal the invalid data. // // Here append the original value to ensure that the number of key and - // value are the same. + // value are aligned. vals = append(vals, common.CopyBytes(iter.Value())) log.Error("Failed to convert account state data", "err", err) } else { @@ -324,7 +250,7 @@ func (dl *diskLayer) proveRange(stats *generatorStats, root common.Hash, prefix // Snap state is chunked, generate edge proofs for verification. tr, err := trie.New(root, dl.triedb) if err != nil { - stats.Log("Trie missing, state snapshotting paused", dl.root, dl.genMarker) + ctx.stats.Log("Trie missing, state snapshotting paused", dl.root, dl.genMarker) return nil, errMissingTrie } // Firstly find out the key of last iterated element. @@ -382,14 +308,14 @@ func (dl *diskLayer) proveRange(stats *generatorStats, root common.Hash, prefix // However, for accounts, the storage trie of the account needs to be checked. Also, // dangling storages(storage exists but the corresponding account is missing) need to // be cleaned up. -type onStateCallback func(key []byte, val []byte, r *DanglingRange, write bool, delete bool) error +type onStateCallback func(key []byte, val []byte, write bool, delete bool) error // generateRange generates the state segment with particular prefix. Generation can // either verify the correctness of existing state through range-proof and skip // generation, or iterate trie to regenerate state on demand. -func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, origin []byte, max int, stats *generatorStats, onState onStateCallback, valueConvertFn func([]byte) ([]byte, error)) (bool, []byte, error) { +func (dl *diskLayer) generateRange(ctx *generatorContext, root common.Hash, prefix []byte, kind string, origin []byte, max int, onState onStateCallback, valueConvertFn func([]byte) ([]byte, error)) (bool, []byte, error) { // Use range prover to check the validity of the flat state in the range - result, err := dl.proveRange(stats, root, prefix, kind, origin, max, valueConvertFn) + result, err := dl.proveRange(ctx, root, prefix, kind, origin, max, valueConvertFn) if err != nil { return false, nil, err } @@ -410,11 +336,7 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, // The verification is passed, process each state with the given // callback function. If this state represents a contract, the // corresponding storage check will be performed in the callback - var drange *DanglingRange - if kind != "storage" { - drange = NewDanglingRange(dl.diskdb, origin, result.last(), false) - } - if err := result.forEach(func(key []byte, val []byte) error { return onState(key, val, drange, false, false) }); err != nil { + if err := result.forEach(func(key []byte, val []byte) error { return onState(key, val, false, false) }); err != nil { return false, nil, err } // Only abort the iteration when both database and trie are exhausted @@ -424,18 +346,17 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, snapFailedRangeProofMeter.Mark(1) // Special case, the entire trie is missing. In the original trie scheme, - // all the duplicated subtries will be filter out(only one copy of data + // all the duplicated subtries will be filtered ot(only one copy of data // will be stored). While in the snapshot model, all the storage tries // belong to different contracts will be kept even they are duplicated. // Track it to a certain extent remove the noise data used for statistics. if origin == nil && last == nil { meter := snapMissallAccountMeter - if kind == "storage" { + if kind == snapStorage { meter = snapMissallStorageMeter } meter.Mark(1) } - // We use the snap data to build up a cache which can be used by the // main account trie as a primary lookup when resolving hashes var snapNodeCache ethdb.KeyValueStore @@ -449,15 +370,16 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, root, _, _ := snapTrie.Commit(nil) snapTrieDb.Commit(root, false, nil) } + // Construct the trie for state iteration, reuse the trie + // if it's already opened with some nodes resolved. tr := result.tr if tr == nil { tr, err = trie.New(root, dl.triedb) if err != nil { - stats.Log("Trie missing, state snapshotting paused", dl.root, dl.genMarker) + ctx.stats.Log("Trie missing, state snapshotting paused", dl.root, dl.genMarker) return false, nil, errMissingTrie } } - var ( trieMore bool nodeIt = tr.NodeIterator(origin) @@ -477,10 +399,6 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, ) nodeIt.AddResolver(snapNodeCache) - var drange *DanglingRange - if kind != "storage" { - drange = NewDanglingRange(dl.diskdb, origin, result.last(), false) - } for iter.Next() { if last != nil && bytes.Compare(iter.Key, last) > 0 { trieMore = true @@ -493,7 +411,7 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, if cmp := bytes.Compare(kvkeys[0], iter.Key); cmp < 0 { // delete the key istart := time.Now() - if err := onState(kvkeys[0], nil, drange, false, true); err != nil { + if err := onState(kvkeys[0], nil, false, true); err != nil { return false, nil, err } kvkeys = kvkeys[1:] @@ -515,7 +433,7 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, break } istart := time.Now() - if err := onState(iter.Key, iter.Value, drange, write, false); err != nil { + if err := onState(iter.Key, iter.Value, write, false); err != nil { return false, nil, err } internal += time.Since(istart) @@ -526,7 +444,7 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, // Delete all stale snapshot states remaining istart := time.Now() for _, key := range kvkeys { - if err := onState(key, nil, drange, false, true); err != nil { + if err := onState(key, nil, false, true); err != nil { return false, nil, err } deleted += 1 @@ -534,7 +452,7 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, internal += time.Since(istart) // Update metrics for counting trie iteration - if kind == "storage" { + if kind == snapStorage { snapStorageTrieReadCounter.Inc((time.Since(start) - internal).Nanoseconds()) } else { snapAccountTrieReadCounter.Inc((time.Since(start) - internal).Nanoseconds()) @@ -542,15 +460,6 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, logger.Debug("Regenerated state range", "root", root, "last", hexutil.Encode(last), "count", count, "created", created, "updated", updated, "untouched", untouched, "deleted", deleted) - // Clean up the left dangling storages at the end in case the entire - // snapshot is regenerated. Note if the last is nil, it means there - // is no snapshot data at disk at all, the entire snapshot is expected - // to be re-generated already. - if last == nil && drange != nil { - if err := drange.cleanup(nil); err != nil { - return false, nil, err - } - } // If there are either more trie items, or there are more snap items // (in the next segment), then we need to keep working return !trieMore && !result.diskMore, last, nil @@ -558,66 +467,66 @@ func (dl *diskLayer) generateRange(root common.Hash, prefix []byte, kind string, // checkAndFlush checks if an interruption signal is received or the // batch size has exceeded the allowance. -func (dl *diskLayer) checkAndFlush(current []byte, batch ethdb.Batch, stats *generatorStats, logged *time.Time) error { +func (dl *diskLayer) checkAndFlush(ctx *generatorContext, current []byte) error { var abort chan *generatorStats select { case abort = <-dl.genAbort: default: } - if batch.ValueSize() > ethdb.IdealBatchSize || abort != nil { + if ctx.batch.ValueSize() > ethdb.IdealBatchSize || abort != nil { if bytes.Compare(current, dl.genMarker) < 0 { log.Error("Snapshot generator went backwards", "current", fmt.Sprintf("%x", current), "genMarker", fmt.Sprintf("%x", dl.genMarker)) } // Flush out the batch anyway no matter it's empty or not. // It's possible that all the states are recovered and the // generation indeed makes progress. - journalProgress(batch, current, stats) + journalProgress(ctx.batch, current, ctx.stats) - if err := batch.Write(); err != nil { + if err := ctx.batch.Write(); err != nil { return err } - batch.Reset() + ctx.batch.Reset() dl.lock.Lock() dl.genMarker = current dl.lock.Unlock() if abort != nil { - stats.Log("Aborting state snapshot generation", dl.root, current) + ctx.stats.Log("Aborting state snapshot generation", dl.root, current) return newAbortErr(abort) // bubble up an error for interruption } } - if time.Since(*logged) > 8*time.Second { - stats.Log("Generating state snapshot", dl.root, current) - *logged = time.Now() + if time.Since(ctx.logged) > 8*time.Second { + ctx.stats.Log("Generating state snapshot", dl.root, current) + ctx.logged = time.Now() } return nil } // generateStorages generates the missing storage slots of the specific contract. // It's supposed to restart the generation from the given origin position. -func generateStorages(dl *diskLayer, account common.Hash, storageRoot common.Hash, storeMarker []byte, batch ethdb.Batch, stats *generatorStats, logged *time.Time) error { - onStorage := func(key []byte, val []byte, r *DanglingRange, write bool, delete bool) error { +func generateStorages(ctx *generatorContext, dl *diskLayer, account common.Hash, storageRoot common.Hash, storeMarker []byte) error { + onStorage := func(key []byte, val []byte, write bool, delete bool) error { defer func(start time.Time) { snapStorageWriteCounter.Inc(time.Since(start).Nanoseconds()) }(time.Now()) if delete { - rawdb.DeleteStorageSnapshot(batch, account, common.BytesToHash(key)) + rawdb.DeleteStorageSnapshot(ctx.batch, account, common.BytesToHash(key)) snapWipedStorageMeter.Mark(1) return nil } if write { - rawdb.WriteStorageSnapshot(batch, account, common.BytesToHash(key), val) + rawdb.WriteStorageSnapshot(ctx.batch, account, common.BytesToHash(key), val) snapGeneratedStorageMeter.Mark(1) } else { snapRecoveredStorageMeter.Mark(1) } - stats.storage += common.StorageSize(1 + 2*common.HashLength + len(val)) - stats.slots++ + ctx.stats.storage += common.StorageSize(1 + 2*common.HashLength + len(val)) + ctx.stats.slots++ // If we've exceeded our batch allowance or termination was requested, flush to disk - if err := dl.checkAndFlush(append(account[:], key...), batch, stats, logged); err != nil { + if err := dl.checkAndFlush(ctx, append(account[:], key...)); err != nil { return err } return nil @@ -625,7 +534,7 @@ func generateStorages(dl *diskLayer, account common.Hash, storageRoot common.Has // Loop for re-generating the missing storage slots. var origin = common.CopyBytes(storeMarker) for { - exhausted, last, err := dl.generateRange(storageRoot, append(rawdb.SnapshotStoragePrefix, account.Bytes()...), "storage", origin, storageCheckRange, stats, onStorage, nil) + exhausted, last, err := dl.generateRange(ctx, storageRoot, append(rawdb.SnapshotStoragePrefix, account.Bytes()...), snapStorage, origin, storageCheckRange, onStorage, nil) if err != nil { return err // The procedure it aborted, either by external signal or internal error. } @@ -643,26 +552,20 @@ func generateStorages(dl *diskLayer, account common.Hash, storageRoot common.Has // generateAccounts generates the missing snapshot accounts as well as their // storage slots in the main trie. It's supposed to restart the generation // from the given origin position. -func generateAccounts(dl *diskLayer, accMarker []byte, batch ethdb.Batch, stats *generatorStats, logged *time.Time) error { - onAccount := func(key []byte, val []byte, r *DanglingRange, write bool, delete bool) error { +func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) error { + onAccount := func(key []byte, val []byte, write bool, delete bool) error { var ( start = time.Now() accountHash = common.BytesToHash(key) ) - // Clean up the dangling storages which have no corresponding accounts present. - if err := r.cleanup(key); err != nil { - return err - } + // Make sure to clear all dangling storages before processing this account + ctx.removeStorageBefore(accountHash) + if delete { - rawdb.DeleteAccountSnapshot(batch, accountHash) + rawdb.DeleteAccountSnapshot(ctx.batch, accountHash) snapWipedAccountMeter.Mark(1) - // Ensure that any previous snapshot storage values are cleared - prefix := append(rawdb.SnapshotStoragePrefix, accountHash.Bytes()...) - keyLen := len(rawdb.SnapshotStoragePrefix) + 2*common.HashLength - if err := wipeKeyRange(dl.diskdb, "storage", prefix, nil, nil, keyLen, snapWipedStorageMeter, false); err != nil { - return err - } + ctx.removeStorageAt(accountHash) snapAccountWriteCounter.Inc(time.Since(start).Nanoseconds()) return nil } @@ -690,11 +593,11 @@ func generateAccounts(dl *diskLayer, accMarker []byte, batch ethdb.Batch, stats } else { data := SlimAccountRLP(acc.Nonce, acc.Balance, acc.Root, acc.CodeHash) dataLen = len(data) - rawdb.WriteAccountSnapshot(batch, accountHash, data) + rawdb.WriteAccountSnapshot(ctx.batch, accountHash, data) snapGeneratedAccountMeter.Mark(1) } - stats.storage += common.StorageSize(1 + common.HashLength + dataLen) - stats.accounts++ + ctx.stats.storage += common.StorageSize(1 + common.HashLength + dataLen) + ctx.stats.accounts++ } marker := accountHash[:] // If the snap generation goes here after interrupted, genMarker may go backward @@ -703,22 +606,13 @@ func generateAccounts(dl *diskLayer, accMarker []byte, batch ethdb.Batch, stats marker = dl.genMarker[:] } // If we've exceeded our batch allowance or termination was requested, flush to disk - if err := dl.checkAndFlush(marker, batch, stats, logged); err != nil { + if err := dl.checkAndFlush(ctx, marker); err != nil { return err } // If the iterated account is the contract, create a further loop to // verify or regenerate the contract storage. if acc.Root == emptyRoot { - // If the root is empty, we still need to ensure that any previous snapshot - // storage values are cleared - // TODO: investigate if this can be avoided, this will be very costly since it - // affects every single EOA account - // - Perhaps we can avoid if where codeHash is emptyCode - prefix := append(rawdb.SnapshotStoragePrefix, accountHash.Bytes()...) - keyLen := len(rawdb.SnapshotStoragePrefix) + 2*common.HashLength - if err := wipeKeyRange(dl.diskdb, "storage", prefix, nil, nil, keyLen, snapWipedStorageMeter, false); err != nil { - return err - } + ctx.removeStorageAt(accountHash) snapAccountWriteCounter.Inc(time.Since(start).Nanoseconds()) } else { snapAccountWriteCounter.Inc(time.Since(start).Nanoseconds()) @@ -727,7 +621,11 @@ func generateAccounts(dl *diskLayer, accMarker []byte, batch ethdb.Batch, stats if accMarker != nil && bytes.Equal(accountHash[:], accMarker) && len(dl.genMarker) > common.HashLength { storeMarker = dl.genMarker[common.HashLength:] } - if err := generateStorages(dl, accountHash, acc.Root, storeMarker, batch, stats, logged); err != nil { + // Shift iterator to the interruption position + if storeMarker != nil { + ctx.reopen(snapStorage, accMarker, storeMarker) + } + if err := generateStorages(ctx, dl, accountHash, acc.Root, storeMarker); err != nil { return err } } @@ -735,31 +633,20 @@ func generateAccounts(dl *diskLayer, accMarker []byte, batch ethdb.Batch, stats accMarker = nil return nil } - // Always reset the initial account range as 1 whenever recover from the interruption. - var accountRange = accountCheckRange - if len(accMarker) > 0 { - accountRange = 1 - } - // Global loop for re-generating the account snapshots + all layered storage snapshots. origin := common.CopyBytes(accMarker) for { - exhausted, last, err := dl.generateRange(dl.root, rawdb.SnapshotAccountPrefix, "account", origin, accountRange, stats, onAccount, FullAccountRLP) + exhausted, last, err := dl.generateRange(ctx, dl.root, rawdb.SnapshotAccountPrefix, snapAccount, origin, accountCheckRange, onAccount, FullAccountRLP) if err != nil { return err // The procedure it aborted, either by external signal or internal error. } - if origin = increaseKey(last); origin == nil { - break // special case, the last is 0xffffffff...fff - } - // Abort the procedure if the entire snapshot is generated - if exhausted { - // Last step, cleanup the storages after the last account. - // All the left storages should be treated as dangling. - if err := NewDanglingRange(dl.diskdb, origin, nil, false).cleanup(nil); err != nil { - return err - } + origin = increaseKey(last) + + // Last step, cleanup the storages after the last account. + // All the left storages should be treated as dangling. + if origin == nil || exhausted { + ctx.removeAllStorage() break } - accountRange = accountCheckRange } return nil } @@ -769,19 +656,20 @@ func generateAccounts(dl *diskLayer, accMarker []byte, batch ethdb.Batch, stats // gathering and logging, since the method surfs the blocks as they arrive, often // being restarted. func (dl *diskLayer) generate(stats *generatorStats) { - var accMarker []byte + var ( + accMarker []byte + abort chan *generatorStats + ) if len(dl.genMarker) > 0 { // []byte{} is the start, use nil for that accMarker = dl.genMarker[:common.HashLength] } - var ( - batch = dl.diskdb.NewBatch() - logged = time.Now() - abort chan *generatorStats - ) stats.Log("Resuming state snapshot generation", dl.root, dl.genMarker) - // Generate the snapshot accounts from the point where they left off. - if err := generateAccounts(dl, accMarker, batch, stats, &logged); err != nil { + // Initialize the global generator context + ctx := newGeneratorContext(stats, dl.diskdb, accMarker) + defer ctx.close() + + if err := generateAccounts(ctx, dl, accMarker); err != nil { // Extract the received interruption signal if exists if aerr, ok := err.(*abortErr); ok { abort = aerr.abort @@ -796,15 +684,15 @@ func (dl *diskLayer) generate(stats *generatorStats) { // Snapshot fully generated, set the marker to nil. // Note even there is nothing to commit, persist the // generator anyway to mark the snapshot is complete. - journalProgress(batch, nil, stats) - if err := batch.Write(); err != nil { + journalProgress(ctx.batch, nil, stats) + if err := ctx.batch.Write(); err != nil { log.Error("Failed to flush batch", "err", err) abort = <-dl.genAbort abort <- stats return } - batch.Reset() + ctx.batch.Reset() log.Info("Generated state snapshot", "accounts", stats.accounts, "slots", stats.slots, "storage", stats.storage, "elapsed", common.PrettyDuration(time.Since(stats.start))) diff --git a/core/state/snapshot/metrics.go b/core/state/snapshot/metrics.go new file mode 100644 index 000000000000..23b240c3e74f --- /dev/null +++ b/core/state/snapshot/metrics.go @@ -0,0 +1,50 @@ +// Copyright 2022 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see + +package snapshot + +import "github.com/ethereum/go-ethereum/metrics" + +// Metrics in generation +var ( + snapGeneratedAccountMeter = metrics.NewRegisteredMeter("state/snapshot/generation/account/generated", nil) + snapRecoveredAccountMeter = metrics.NewRegisteredMeter("state/snapshot/generation/account/recovered", nil) + snapWipedAccountMeter = metrics.NewRegisteredMeter("state/snapshot/generation/account/wiped", nil) + snapMissallAccountMeter = metrics.NewRegisteredMeter("state/snapshot/generation/account/missall", nil) + snapGeneratedStorageMeter = metrics.NewRegisteredMeter("state/snapshot/generation/storage/generated", nil) + snapRecoveredStorageMeter = metrics.NewRegisteredMeter("state/snapshot/generation/storage/recovered", nil) + snapWipedStorageMeter = metrics.NewRegisteredMeter("state/snapshot/generation/storage/wiped", nil) + snapMissallStorageMeter = metrics.NewRegisteredMeter("state/snapshot/generation/storage/missall", nil) + snapSuccessfulRangeProofMeter = metrics.NewRegisteredMeter("state/snapshot/generation/proof/success", nil) + snapFailedRangeProofMeter = metrics.NewRegisteredMeter("state/snapshot/generation/proof/failure", nil) + + // snapAccountProveCounter measures time spent on the account proving + snapAccountProveCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/account/prove", nil) + // snapAccountTrieReadCounter measures time spent on the account trie iteration + snapAccountTrieReadCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/account/trieread", nil) + // snapAccountSnapReadCounter measues time spent on the snapshot account iteration + snapAccountSnapReadCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/account/snapread", nil) + // snapAccountWriteCounter measures time spent on writing/updating/deleting accounts + snapAccountWriteCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/account/write", nil) + // snapStorageProveCounter measures time spent on storage proving + snapStorageProveCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/storage/prove", nil) + // snapStorageTrieReadCounter measures time spent on the storage trie iteration + snapStorageTrieReadCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/storage/trieread", nil) + // snapStorageSnapReadCounter measures time spent on the snapshot storage iteration + snapStorageSnapReadCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/storage/snapread", nil) + // snapStorageWriteCounter measures time spent on writing/updating/deleting storages + snapStorageWriteCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/storage/write", nil) +) diff --git a/ethdb/iterator.go b/ethdb/iterator.go index 2b49c93a96c4..be14fef39509 100644 --- a/ethdb/iterator.go +++ b/ethdb/iterator.go @@ -26,6 +26,10 @@ package ethdb // iterator until exhaustion. An iterator is not safe for concurrent use, but it // is safe to use multiple iterators concurrently. type Iterator interface { + // Prev moves the iterator to the previous key/value pair. It returns false + // if the iterator is exhausted. + Prev() bool + // Next moves the iterator to the next key/value pair. It returns whether the // iterator is exhausted. Next() bool diff --git a/ethdb/memorydb/memorydb.go b/ethdb/memorydb/memorydb.go index 95ec9bb8aa46..80757399c7f6 100644 --- a/ethdb/memorydb/memorydb.go +++ b/ethdb/memorydb/memorydb.go @@ -36,6 +36,10 @@ var ( // the provided memory database. errMemorydbNotFound = errors.New("not found") + // errIteratorReleased is returned if callers want to retrieve data from a + // released iterator. + errIteratorReleased = errors.New("iterator released") + // errSnapshotReleased is returned if callers want to retrieve data from a // released snapshot. errSnapshotReleased = errors.New("snapshot released") @@ -169,6 +173,7 @@ func (db *Database) NewIterator(prefix []byte, start []byte) ethdb.Iterator { values = append(values, db.db[key]) } return &iterator{ + index: -1, keys: keys, values: values, } @@ -279,25 +284,39 @@ func (b *batch) Replay(w ethdb.KeyValueWriter) error { // value store. Internally it is a deep copy of the entire iterated state, // sorted by keys. type iterator struct { - inited bool + index int keys []string values [][]byte } +// Prev moves the iterator to the previous key/value pair. It returns whether the +// iterator is exhausted. +func (it *iterator) Prev() bool { + // Short circuit if iterator has nothing to iterate. + if len(it.keys) == 0 { + return false + } + // Short circuit if the iterator is not yet initialized + if it.index == -1 { + return false + } + it.index -= 1 + return true +} + // Next moves the iterator to the next key/value pair. It returns whether the // iterator is exhausted. func (it *iterator) Next() bool { - // If the iterator was not yet initialized, do it now - if !it.inited { - it.inited = true - return len(it.keys) > 0 + // Short circuit if iterator has nothing to iterate. + if len(it.keys) == 0 { + return false } - // Iterator already initialize, advance it - if len(it.keys) > 0 { - it.keys = it.keys[1:] - it.values = it.values[1:] + // Short circuit if iterator is already exhausted. + if it.index == len(it.keys)-1 { + return false } - return len(it.keys) > 0 + it.index += 1 + return true } // Error returns any accumulated error. Exhausting all the key/value pairs @@ -310,26 +329,28 @@ func (it *iterator) Error() error { // should not modify the contents of the returned slice, and its contents may // change on the next call to Next. func (it *iterator) Key() []byte { - if len(it.keys) > 0 { - return []byte(it.keys[0]) + // Short circuit if iterator is not initialized yet. + if it.index == -1 { + return nil } - return nil + return []byte(it.keys[it.index]) } // Value returns the value of the current key/value pair, or nil if done. The // caller should not modify the contents of the returned slice, and its contents // may change on the next call to Next. func (it *iterator) Value() []byte { - if len(it.values) > 0 { - return it.values[0] + // Short circuit if iterator is not initialized yet. + if it.index == -1 { + return nil } - return nil + return it.values[it.index] } // Release releases associated resources. Release should always succeed and can // be called multiple times without causing error. func (it *iterator) Release() { - it.keys, it.values = nil, nil + it.index, it.keys, it.values = -1, nil, nil } // snapshot wraps a batch of key-value entries deep copied from the in-memory From f4a489d31a83b6b68cb3eb46874f81c50ff7f242 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Wed, 4 May 2022 10:39:16 +0800 Subject: [PATCH 10/30] core/state/snapshot: polish --- core/state/snapshot/context.go | 35 +++++++++++++++++---------------- core/state/snapshot/generate.go | 4 ++-- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/core/state/snapshot/context.go b/core/state/snapshot/context.go index bca280509d69..85c9777baeda 100644 --- a/core/state/snapshot/context.go +++ b/core/state/snapshot/context.go @@ -98,10 +98,10 @@ func newGeneratorContext(stats *generatorStats, db ethdb.KeyValueStore, accMarke // interrupted position. These iterators will be reopened from time // to time to avoid blocking leveldb compaction for a long time. // - // Note for the storage iterator, we use the interrupted account hash as - // the iteration start point instead of the storage interruption position, - // because this account can be deleted since last generation and storages - // need to wiped in this case. + // Note for the storage iterator, we use the account interruption + // position as the iteration start point, because this account can + // be deleted since last generation and storages need to wiped in + // this case. var ( acctIter = db.NewIterator(rawdb.SnapshotAccountPrefix, accMarker) storageIter = db.NewIterator(rawdb.SnapshotStoragePrefix, accMarker) @@ -132,8 +132,8 @@ func (ctx *generatorContext) iterator(kind string) ethdb.Iterator { return ctx.storage } -// reopen re-constructs the database iterator with given start position. -func (ctx *generatorContext) reopen(kind string, prefix []byte, start []byte) { +// seekIterator re-opens the database iterator with given start position. +func (ctx *generatorContext) seekIterator(kind string, prefix []byte, start []byte) { iter := ctx.iterator(kind) iter.Release() @@ -145,10 +145,11 @@ func (ctx *generatorContext) reopen(kind string, prefix []byte, start []byte) { } } -// removeStorageBefore starting from the current iterator position, iterate and -// delete all storage snapshots before the specified account. When the iterator -// touches the storage located in the account range, or the storage is larger -// than the account range, it stops and moves back the iterator a step. +// removeStorageBefore, iterates and deletes all storage snapshots starting +// from the current iterator position until the specified account. When the +// iterator touches the storage located in the given account range, or the +// storage is larger than the given account range, it stops and moves back +// the iterator a step. func (ctx *generatorContext) removeStorageBefore(account common.Hash) { iter := ctx.storage for iter.Next() { @@ -161,11 +162,11 @@ func (ctx *generatorContext) removeStorageBefore(account common.Hash) { } } -// removeStorageAt starting from the current iterator position, iterate and -// delete all storage snapshots located in the specified account range. When -// the iterator touches the storage is larger than the account range, it stops -// and moves back the iterator a step. An error will be returned if the initial -// position of iterator is not in the specified account range. +// removeStorageAt iterates and deletes all storage snapshots which are located +// in the specified account range. When the iterator touches the storage which +// is larger than the given account range, it stops and moves back the iterator +// a step. An error will be returned if the initial position of iterator is not +// in the given account range. func (ctx *generatorContext) removeStorageAt(account common.Hash) error { iter := ctx.iterator(snapStorage) for iter.Next() { @@ -183,9 +184,9 @@ func (ctx *generatorContext) removeStorageAt(account common.Hash) error { return nil } -// removeAllStorage starting from the current iterator position, iterate and +// removeStorageLeft starting from the current iterator position, iterate and // delete all storage snapshots left. -func (ctx *generatorContext) removeAllStorage() { +func (ctx *generatorContext) removeStorageLeft() { iter := ctx.iterator(snapStorage) for iter.Next() { ctx.batch.Delete(iter.Key()) diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 1f0b3cf82430..b3d6a4f600df 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -623,7 +623,7 @@ func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) er } // Shift iterator to the interruption position if storeMarker != nil { - ctx.reopen(snapStorage, accMarker, storeMarker) + ctx.seekIterator(snapStorage, accMarker, storeMarker) } if err := generateStorages(ctx, dl, accountHash, acc.Root, storeMarker); err != nil { return err @@ -644,7 +644,7 @@ func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) er // Last step, cleanup the storages after the last account. // All the left storages should be treated as dangling. if origin == nil || exhausted { - ctx.removeAllStorage() + ctx.removeStorageLeft() break } } From 5f37c25e71812b81b58bfe4611a6281ef70ece06 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Wed, 4 May 2022 14:37:20 +0800 Subject: [PATCH 11/30] cmd, core/state/snapshot: polish --- cmd/geth/snapshot.go | 63 +--------- core/state/snapshot/context.go | 98 +++++++++------ core/state/snapshot/dangling.go | 182 ++++++++++++++++----------- core/state/snapshot/generate.go | 24 ++-- core/state/snapshot/generate_test.go | 5 +- core/state/snapshot/journal.go | 75 ----------- 6 files changed, 187 insertions(+), 260 deletions(-) diff --git a/cmd/geth/snapshot.go b/cmd/geth/snapshot.go index 7582b4c4d18c..a9fc035db3ac 100644 --- a/cmd/geth/snapshot.go +++ b/cmd/geth/snapshot.go @@ -20,7 +20,6 @@ import ( "bytes" "encoding/json" "errors" - "fmt" "os" "time" @@ -32,7 +31,6 @@ import ( "github.com/ethereum/go-ethereum/core/state/snapshot" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" - "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/rlp" "github.com/ethereum/go-ethereum/trie" @@ -223,15 +221,7 @@ func verifyState(ctx *cli.Context) error { return err } log.Info("Verified the state", "root", root) - if err := checkDanglingDiskStorage(chaindb); err != nil { - log.Error("Dangling snap disk-storage check failed", "root", root, "err", err) - return err - } - if err := checkDanglingMemStorage(chaindb); err != nil { - log.Error("Dangling snap mem-storage check failed", "root", root, "err", err) - return err - } - return nil + return snapshot.CheckDanglingStorage(chaindb) } // checkDanglingStorage iterates the snap storage data, and verifies that all @@ -240,56 +230,7 @@ func checkDanglingStorage(ctx *cli.Context) error { stack, _ := makeConfigNode(ctx) defer stack.Close() - chaindb := utils.MakeChainDatabase(ctx, stack, true) - if err := checkDanglingDiskStorage(chaindb); err != nil { - return err - } - return checkDanglingMemStorage(chaindb) - -} - -// checkDanglingDiskStorage checks if there is any 'dangling' storage data in the -// disk-backed snapshot layer. -func checkDanglingDiskStorage(chaindb ethdb.Database) error { - log.Info("Checking dangling snapshot disk storage") - var ( - lastReport = time.Now() - start = time.Now() - lastKey []byte - it = rawdb.NewKeyLengthIterator(chaindb.NewIterator(rawdb.SnapshotStoragePrefix, nil), 1+2*common.HashLength) - ) - defer it.Release() - for it.Next() { - k := it.Key() - accKey := k[1:33] - if bytes.Equal(accKey, lastKey) { - // No need to look up for every slot - continue - } - lastKey = common.CopyBytes(accKey) - if time.Since(lastReport) > time.Second*8 { - log.Info("Iterating snap storage", "at", fmt.Sprintf("%#x", accKey), "elapsed", common.PrettyDuration(time.Since(start))) - lastReport = time.Now() - } - if data := rawdb.ReadAccountSnapshot(chaindb, common.BytesToHash(accKey)); len(data) == 0 { - log.Error("Dangling storage - missing account", "account", fmt.Sprintf("%#x", accKey), "storagekey", fmt.Sprintf("%#x", k)) - return fmt.Errorf("dangling snapshot storage account %#x", accKey) - } - } - log.Info("Verified the snapshot disk storage", "time", common.PrettyDuration(time.Since(start)), "err", it.Error()) - return nil -} - -// checkDanglingMemStorage checks if there is any 'dangling' storage in the journalled -// snapshot difflayers. -func checkDanglingMemStorage(chaindb ethdb.Database) error { - start := time.Now() - log.Info("Checking dangling snapshot difflayer journalled storage") - if err := snapshot.CheckJournalStorage(chaindb); err != nil { - return err - } - log.Info("Verified the snapshot journalled storage", "time", common.PrettyDuration(time.Since(start))) - return nil + return snapshot.CheckDanglingStorage(utils.MakeChainDatabase(ctx, stack, true)) } // traverseState is a helper function used for pruning verification. diff --git a/core/state/snapshot/context.go b/core/state/snapshot/context.go index 85c9777baeda..832634695955 100644 --- a/core/state/snapshot/context.go +++ b/core/state/snapshot/context.go @@ -41,6 +41,7 @@ type generatorStats struct { start time.Time // Timestamp when generation started accounts uint64 // Number of accounts indexed(generated or recovered) slots uint64 // Number of storage slots indexed(generated or recovered) + dangling uint64 // Number of dangling storage slots storage common.StorageSize // Total account and storage slot size(generation or recovery) } @@ -66,6 +67,7 @@ func (gs *generatorStats) Log(msg string, root common.Hash, marker []byte) { "accounts", gs.accounts, "slots", gs.slots, "storage", gs.storage, + "dangling", gs.dangling, "elapsed", common.PrettyDuration(time.Since(gs.start)), }...) // Calculate the estimated indexing time based on current stats @@ -93,28 +95,46 @@ type generatorContext struct { } // newGeneratorContext initializes the context for generation. -func newGeneratorContext(stats *generatorStats, db ethdb.KeyValueStore, accMarker []byte) *generatorContext { - // Construct global account and storage snapshot iterators at the - // interrupted position. These iterators will be reopened from time - // to time to avoid blocking leveldb compaction for a long time. - // - // Note for the storage iterator, we use the account interruption - // position as the iteration start point, because this account can - // be deleted since last generation and storages need to wiped in - // this case. - var ( - acctIter = db.NewIterator(rawdb.SnapshotAccountPrefix, accMarker) - storageIter = db.NewIterator(rawdb.SnapshotStoragePrefix, accMarker) - acctIt = rawdb.NewKeyLengthIterator(acctIter, 1+common.HashLength) - storageIt = rawdb.NewKeyLengthIterator(storageIter, 1+2*common.HashLength) - ) - return &generatorContext{ - stats: stats, - db: db, - account: acctIt, - storage: storageIt, - batch: db.NewBatch(), - logged: time.Now(), +func newGeneratorContext(stats *generatorStats, db ethdb.KeyValueStore, accMarker []byte, storageMarker []byte) *generatorContext { + ctx := &generatorContext{ + stats: stats, + db: db, + batch: db.NewBatch(), + logged: time.Now(), + } + ctx.openIterator(snapAccount, accMarker) + ctx.openIterator(snapStorage, storageMarker) + return ctx +} + +// openIterator constructs global account and storage snapshot iterators +// at the interrupted position. These iterators should be reopened from time +// to time to avoid blocking leveldb compaction for a long time. +func (ctx *generatorContext) openIterator(kind string, start []byte) { + if kind == snapAccount { + iter := ctx.db.NewIterator(rawdb.SnapshotAccountPrefix, start) + ctx.account = rawdb.NewKeyLengthIterator(iter, 1+common.HashLength) + return + } + iter := ctx.db.NewIterator(rawdb.SnapshotStoragePrefix, start) + ctx.storage = rawdb.NewKeyLengthIterator(iter, 1+2*common.HashLength) +} + +// reopenIterators releases the held two global database iterators and +// reopens them in the interruption position. It's aim for not blocking +// leveldb compaction. +func (ctx *generatorContext) reopenIterators() { + for i, iter := range []ethdb.Iterator{ctx.account, ctx.storage} { + var ( + key = iter.Key() + cur = key[1:] + ) + kind := snapAccount + if i == 1 { + kind = snapStorage + } + iter.Release() + ctx.openIterator(kind, cur) } } @@ -132,26 +152,16 @@ func (ctx *generatorContext) iterator(kind string) ethdb.Iterator { return ctx.storage } -// seekIterator re-opens the database iterator with given start position. -func (ctx *generatorContext) seekIterator(kind string, prefix []byte, start []byte) { - iter := ctx.iterator(kind) - iter.Release() - - iter = ctx.db.NewIterator(prefix, start) - if kind == snapStorage { - ctx.storage = rawdb.NewKeyLengthIterator(iter, 1+2*common.HashLength) - } else { - ctx.account = rawdb.NewKeyLengthIterator(iter, 1+common.HashLength) - } -} - // removeStorageBefore, iterates and deletes all storage snapshots starting // from the current iterator position until the specified account. When the // iterator touches the storage located in the given account range, or the // storage is larger than the given account range, it stops and moves back // the iterator a step. func (ctx *generatorContext) removeStorageBefore(account common.Hash) { - iter := ctx.storage + var ( + count uint64 + iter = ctx.storage + ) for iter.Next() { key := iter.Key() if bytes.Compare(key[1:1+common.HashLength], account.Bytes()) >= 0 { @@ -159,7 +169,9 @@ func (ctx *generatorContext) removeStorageBefore(account common.Hash) { return } ctx.batch.Delete(key) + count += 1 } + ctx.stats.dangling += count } // removeStorageAt iterates and deletes all storage snapshots which are located @@ -168,7 +180,10 @@ func (ctx *generatorContext) removeStorageBefore(account common.Hash) { // a step. An error will be returned if the initial position of iterator is not // in the given account range. func (ctx *generatorContext) removeStorageAt(account common.Hash) error { - iter := ctx.iterator(snapStorage) + var ( + count uint64 + iter = ctx.iterator(snapStorage) + ) for iter.Next() { key := iter.Key() cmp := bytes.Compare(key[1:1+common.HashLength], account.Bytes()) @@ -180,15 +195,22 @@ func (ctx *generatorContext) removeStorageAt(account common.Hash) error { break } ctx.batch.Delete(key) + count += 1 } + ctx.stats.dangling += count return nil } // removeStorageLeft starting from the current iterator position, iterate and // delete all storage snapshots left. func (ctx *generatorContext) removeStorageLeft() { - iter := ctx.iterator(snapStorage) + var ( + count uint64 + iter = ctx.iterator(snapStorage) + ) for iter.Next() { ctx.batch.Delete(iter.Key()) + count += 1 } + ctx.stats.dangling += count } diff --git a/core/state/snapshot/dangling.go b/core/state/snapshot/dangling.go index b5ae59f9b858..ca73da793f7a 100644 --- a/core/state/snapshot/dangling.go +++ b/core/state/snapshot/dangling.go @@ -18,108 +18,138 @@ package snapshot import ( "bytes" + "errors" "fmt" + "io" "time" "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/rlp" ) -// DanglingRange describes the range for detecting dangling storages. -type DanglingRange struct { - db ethdb.KeyValueStore // The database stores the snapshot data - start []byte // The start of the key range - limit []byte // The last of the key range - - result []common.Hash // The list of account hashes which have the dangling storages - duration time.Duration // Total time spent on the iteration -} - -// NewDanglingRange initializes a dangling storage scanner and detects all the -// dangling accounts out. -func NewDanglingRange(db ethdb.KeyValueStore, start, limit []byte, report bool) *DanglingRange { - r := &DanglingRange{ - db: db, - start: start, - limit: limit, +// CheckDanglingStorage iterates the snap storage data, and verifies that all +// storage also has corresponding account data. +func CheckDanglingStorage(chaindb ethdb.KeyValueStore) error { + if err := checkDanglingDiskStorage(chaindb); err != nil { + return err } - r.result, r.duration = r.detect(report) - - if len(r.result) > 0 { - log.Warn("Detected dangling storages", "number", len(r.result), "start", hexutil.Encode(start), "limit", hexutil.Encode(limit), "elapsed", common.PrettyDuration(r.duration)) - } else { - logger := log.Debug - if report { - logger = log.Info - } - logger("Verified snapshot storages", "start", hexutil.Encode(start), "limit", hexutil.Encode(limit), "elapsed", common.PrettyDuration(r.duration)) - } - return r + return checkDanglingMemStorage(chaindb) } -// detect iterates the storage snapshot in the specified key range and -// returns a list of account hash of the dangling storages. Note both -// start and limit are included for iteration. -func (r *DanglingRange) detect(report bool) ([]common.Hash, time.Duration) { +// checkDanglingDiskStorage checks if there is any 'dangling' storage data in the +// disk-backed snapshot layer. +func checkDanglingDiskStorage(chaindb ethdb.KeyValueStore) error { var ( - checked []byte - result []common.Hash - start = time.Now() lastReport = time.Now() + start = time.Now() + lastKey []byte + it = rawdb.NewKeyLengthIterator(chaindb.NewIterator(rawdb.SnapshotStoragePrefix, nil), 1+2*common.HashLength) ) - iter := rawdb.NewKeyLengthIterator(r.db.NewIterator(rawdb.SnapshotStoragePrefix, r.start), len(rawdb.SnapshotStoragePrefix)+2*common.HashLength) - defer iter.Release() - - for iter.Next() { - account := iter.Key()[len(rawdb.SnapshotStoragePrefix) : len(rawdb.SnapshotStoragePrefix)+common.HashLength] - if r.limit != nil && bytes.Compare(account, r.limit) > 0 { - break - } - // Skip unnecessary checks for checked storage. - if bytes.Equal(account, checked) { - continue - } - checked = common.CopyBytes(account) + log.Info("Checking dangling snapshot disk storage") - // Check the presence of the corresponding account. - accountHash := common.BytesToHash(account) - data := rawdb.ReadAccountSnapshot(r.db, accountHash) - if len(data) != 0 { + defer it.Release() + for it.Next() { + k := it.Key() + accKey := k[1:33] + if bytes.Equal(accKey, lastKey) { + // No need to look up for every slot continue } - result = append(result, accountHash) - - if report { - log.Warn("Dangling storage - missing account", "account", fmt.Sprintf("%#x", accountHash)) - } - if time.Since(lastReport) > time.Second*8 && report { - log.Info("Detecting dangling storage", "at", fmt.Sprintf("%#x", accountHash), "elapsed", common.PrettyDuration(time.Since(start))) + lastKey = common.CopyBytes(accKey) + if time.Since(lastReport) > time.Second*8 { + log.Info("Iterating snap storage", "at", fmt.Sprintf("%#x", accKey), "elapsed", common.PrettyDuration(time.Since(start))) lastReport = time.Now() } + if data := rawdb.ReadAccountSnapshot(chaindb, common.BytesToHash(accKey)); len(data) == 0 { + log.Warn("Dangling storage - missing account", "account", fmt.Sprintf("%#x", accKey), "storagekey", fmt.Sprintf("%#x", k)) + return fmt.Errorf("dangling snapshot storage account %#x", accKey) + } } - return result, time.Since(start) + log.Info("Verified the snapshot disk storage", "time", common.PrettyDuration(time.Since(start)), "err", it.Error()) + return nil } -// cleanup wipes the dangling storages which fall within the range before the given key. -func (r *DanglingRange) cleanup(limit []byte) error { +// checkDanglingMemStorage checks if there is any 'dangling' storage in the journalled +// snapshot difflayers. +func checkDanglingMemStorage(db ethdb.KeyValueStore) error { var ( - err error - wiped int + start = time.Now() + journal = rawdb.ReadSnapshotJournal(db) ) - for _, accountHash := range r.result { - if limit != nil && bytes.Compare(accountHash.Bytes(), limit) >= 0 { - break + if len(journal) == 0 { + log.Warn("Loaded snapshot journal", "diffs", "missing") + return nil + } + r := rlp.NewStream(bytes.NewReader(journal), 0) + // Firstly, resolve the first element as the journal version + version, err := r.Uint() + if err != nil { + log.Warn("Failed to resolve the journal version", "error", err) + return nil + } + if version != journalVersion { + log.Warn("Discarded the snapshot journal with wrong version", "required", journalVersion, "got", version) + return nil + } + // Secondly, resolve the disk layer root, ensure it's continuous + // with disk layer. Note now we can ensure it's the snapshot journal + // correct version, so we expect everything can be resolved properly. + var root common.Hash + if err := r.Decode(&root); err != nil { + return errors.New("missing disk layer root") + } + // The diff journal is not matched with disk, discard them. + // It can happen that Geth crashes without persisting the latest + // diff journal. + // Load all the snapshot diffs from the journal + if err := checkDanglingJournalStorage(r); err != nil { + return err + } + log.Info("Verified the snapshot journalled storage", "time", common.PrettyDuration(time.Since(start))) + return nil +} + +// loadDiffLayer reads the next sections of a snapshot journal, reconstructing a new +// diff and verifying that it can be linked to the requested parent. +func checkDanglingJournalStorage(r *rlp.Stream) error { + for { + // Read the next diff journal entry + var root common.Hash + if err := r.Decode(&root); err != nil { + // The first read may fail with EOF, marking the end of the journal + if err == io.EOF { + return nil + } + return fmt.Errorf("load diff root: %v", err) + } + var destructs []journalDestruct + if err := r.Decode(&destructs); err != nil { + return fmt.Errorf("load diff destructs: %v", err) + } + var accounts []journalAccount + if err := r.Decode(&accounts); err != nil { + return fmt.Errorf("load diff accounts: %v", err) + } + accountData := make(map[common.Hash][]byte) + for _, entry := range accounts { + if len(entry.Blob) > 0 { // RLP loses nil-ness, but `[]byte{}` is not a valid item, so reinterpret that + accountData[entry.Hash] = entry.Blob + } else { + accountData[entry.Hash] = nil + } + } + var storage []journalStorage + if err := r.Decode(&storage); err != nil { + return fmt.Errorf("load diff storage: %v", err) } - prefix := append(rawdb.SnapshotStoragePrefix, accountHash.Bytes()...) - keylen := len(rawdb.SnapshotStoragePrefix) + 2*common.HashLength - if err = wipeKeyRange(r.db, "storage", prefix, nil, nil, keylen, snapWipedStorageMeter, false); err != nil { - break + for _, entry := range storage { + if _, ok := accountData[entry.Hash]; !ok { + log.Error("Dangling storage - missing account", "account", fmt.Sprintf("%#x", entry.Hash), "root", root) + return fmt.Errorf("dangling journal snapshot storage account %#x", entry.Hash) + } } - wiped += 1 } - r.result = r.result[wiped:] - return err } diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index b3d6a4f600df..28d57e3d7525 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -621,10 +621,6 @@ func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) er if accMarker != nil && bytes.Equal(accountHash[:], accMarker) && len(dl.genMarker) > common.HashLength { storeMarker = dl.genMarker[common.HashLength:] } - // Shift iterator to the interruption position - if storeMarker != nil { - ctx.seekIterator(snapStorage, accMarker, storeMarker) - } if err := generateStorages(ctx, dl, accountHash, acc.Root, storeMarker); err != nil { return err } @@ -633,9 +629,18 @@ func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) er accMarker = nil return nil } - origin := common.CopyBytes(accMarker) + // Always reset the initial account range as 1 whenever recover from the + // interruption. TODO(rjl493456442) can we remove it? + var accountRange = accountCheckRange + if len(accMarker) > 0 { + accountRange = 1 + } + var ( + iterTime = time.Now() + origin = common.CopyBytes(accMarker) + ) for { - exhausted, last, err := dl.generateRange(ctx, dl.root, rawdb.SnapshotAccountPrefix, snapAccount, origin, accountCheckRange, onAccount, FullAccountRLP) + exhausted, last, err := dl.generateRange(ctx, dl.root, rawdb.SnapshotAccountPrefix, snapAccount, origin, accountRange, onAccount, FullAccountRLP) if err != nil { return err // The procedure it aborted, either by external signal or internal error. } @@ -647,6 +652,11 @@ func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) er ctx.removeStorageLeft() break } + // Don't hold iterator too long, re-open them to let compactor works. + if time.Since(iterTime) > time.Minute*3 { + iterTime = time.Now() + ctx.reopenIterators() + } } return nil } @@ -666,7 +676,7 @@ func (dl *diskLayer) generate(stats *generatorStats) { stats.Log("Resuming state snapshot generation", dl.root, dl.genMarker) // Initialize the global generator context - ctx := newGeneratorContext(stats, dl.diskdb, accMarker) + ctx := newGeneratorContext(stats, dl.diskdb, accMarker, dl.genMarker) defer ctx.close() if err := generateAccounts(ctx, dl, accMarker); err != nil { diff --git a/core/state/snapshot/generate_test.go b/core/state/snapshot/generate_test.go index 5196f267844d..7e1d2b96f596 100644 --- a/core/state/snapshot/generate_test.go +++ b/core/state/snapshot/generate_test.go @@ -170,9 +170,8 @@ func checkSnapRoot(t *testing.T, snap *diskLayer, trieRoot common.Hash) { if snapRoot != trieRoot { t.Fatalf("snaproot: %#x != trieroot #%x", snapRoot, trieRoot) } - scanner := NewDanglingRange(snap.diskdb, nil, nil, false) - if len(scanner.result) != 0 { - t.Fatalf("Detected dangling storages %d", len(scanner.result)) + if err := CheckDanglingStorage(snap.diskdb); err != nil { + t.Fatalf("Detected dangling storages %v", err) } } diff --git a/core/state/snapshot/journal.go b/core/state/snapshot/journal.go index 8acc441aa15e..6836a574090c 100644 --- a/core/state/snapshot/journal.go +++ b/core/state/snapshot/journal.go @@ -345,78 +345,3 @@ func (dl *diffLayer) Journal(buffer *bytes.Buffer) (common.Hash, error) { log.Debug("Journalled diff layer", "root", dl.root, "parent", dl.parent.Root()) return base, nil } - -// CheckJournalStorage performs consistency-checks on the journalled -// difflayers. -func CheckJournalStorage(db ethdb.KeyValueStore) error { - journal := rawdb.ReadSnapshotJournal(db) - if len(journal) == 0 { - log.Warn("Loaded snapshot journal", "diffs", "missing") - return nil - } - r := rlp.NewStream(bytes.NewReader(journal), 0) - // Firstly, resolve the first element as the journal version - version, err := r.Uint() - if err != nil { - log.Warn("Failed to resolve the journal version", "error", err) - return nil - } - if version != journalVersion { - log.Warn("Discarded the snapshot journal with wrong version", "required", journalVersion, "got", version) - return nil - } - // Secondly, resolve the disk layer root, ensure it's continuous - // with disk layer. Note now we can ensure it's the snapshot journal - // correct version, so we expect everything can be resolved properly. - var root common.Hash - if err := r.Decode(&root); err != nil { - return errors.New("missing disk layer root") - } - // The diff journal is not matched with disk, discard them. - // It can happen that Geth crashes without persisting the latest - // diff journal. - // Load all the snapshot diffs from the journal - return checkDanglingJournalStorage(r) -} - -// loadDiffLayer reads the next sections of a snapshot journal, reconstructing a new -// diff and verifying that it can be linked to the requested parent. -func checkDanglingJournalStorage(r *rlp.Stream) error { - for { - // Read the next diff journal entry - var root common.Hash - if err := r.Decode(&root); err != nil { - // The first read may fail with EOF, marking the end of the journal - if err == io.EOF { - return nil - } - return fmt.Errorf("load diff root: %v", err) - } - var destructs []journalDestruct - if err := r.Decode(&destructs); err != nil { - return fmt.Errorf("load diff destructs: %v", err) - } - var accounts []journalAccount - if err := r.Decode(&accounts); err != nil { - return fmt.Errorf("load diff accounts: %v", err) - } - accountData := make(map[common.Hash][]byte) - for _, entry := range accounts { - if len(entry.Blob) > 0 { // RLP loses nil-ness, but `[]byte{}` is not a valid item, so reinterpret that - accountData[entry.Hash] = entry.Blob - } else { - accountData[entry.Hash] = nil - } - } - var storage []journalStorage - if err := r.Decode(&storage); err != nil { - return fmt.Errorf("load diff storage: %v", err) - } - for _, entry := range storage { - if _, ok := accountData[entry.Hash]; !ok { - log.Error("Dangling storage - missing account", "account", fmt.Sprintf("%#x", entry.Hash), "root", root) - return fmt.Errorf("dangling journal snapshot storage account %#x", entry.Hash) - } - } - } -} From 0584fc6082e4717e4a8b3c3876f0aeabe909b81f Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Wed, 4 May 2022 15:12:22 +0800 Subject: [PATCH 12/30] core/state/snapshot: polish --- core/state/snapshot/generate.go | 2 +- core/state/snapshot/wipe.go | 91 -------------------------------- core/state/snapshot/wipe_test.go | 79 --------------------------- 3 files changed, 1 insertion(+), 171 deletions(-) delete mode 100644 core/state/snapshot/wipe.go delete mode 100644 core/state/snapshot/wipe_test.go diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 28d57e3d7525..43de4275e83a 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -177,7 +177,7 @@ func (dl *diskLayer) proveRange(ctx *generatorContext, root common.Hash, prefix min = append(prefix, origin...) ) for iter.Next() { - // Ensure the iterated item always larger than the given origin. + // Ensure the iterated item is always equal or larger than the given origin. key := iter.Key() if bytes.Compare(key, min) < 0 { return nil, errors.New("invalid iteration position") diff --git a/core/state/snapshot/wipe.go b/core/state/snapshot/wipe.go deleted file mode 100644 index b774c37a4b7c..000000000000 --- a/core/state/snapshot/wipe.go +++ /dev/null @@ -1,91 +0,0 @@ -// Copyright 2019 The go-ethereum Authors -// This file is part of the go-ethereum library. -// -// The go-ethereum library is free software: you can redistribute it and/or modify -// it under the terms of the GNU Lesser General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// The go-ethereum library is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Lesser General Public License for more details. -// -// You should have received a copy of the GNU Lesser General Public License -// along with the go-ethereum library. If not, see . - -package snapshot - -import ( - "bytes" - "time" - - "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/ethdb" - "github.com/ethereum/go-ethereum/log" - "github.com/ethereum/go-ethereum/metrics" -) - -// wipeKeyRange deletes a range of keys from the database starting with prefix -// and having a specific total key length. The start and limit is optional for -// specifying a particular key range for deletion. -// -// Origin is included for wiping and limit is excluded if they are specified. -func wipeKeyRange(db ethdb.KeyValueStore, kind string, prefix []byte, origin []byte, limit []byte, keylen int, meter metrics.Meter, report bool) error { - // Batch deletions together to avoid holding an iterator for too long - var ( - batch = db.NewBatch() - items int - ) - // Iterate over the key-range and delete all of them - start, logged := time.Now(), time.Now() - - it := db.NewIterator(prefix, origin) - var stop []byte - if limit != nil { - stop = append(prefix, limit...) - } - for it.Next() { - // Skip any keys with the correct prefix but wrong length (trie nodes) - key := it.Key() - if !bytes.HasPrefix(key, prefix) { - break - } - if len(key) != keylen { - continue - } - if stop != nil && bytes.Compare(key, stop) >= 0 { - break - } - // Delete the key and periodically recreate the batch and iterator - batch.Delete(key) - items++ - - if items%10000 == 0 { - // Batch too large (or iterator too long lived, flush and recreate) - it.Release() - if err := batch.Write(); err != nil { - return err - } - batch.Reset() - seekPos := key[len(prefix):] - it = db.NewIterator(prefix, seekPos) - - if time.Since(logged) > 8*time.Second && report { - log.Info("Deleting state snapshot leftovers", "kind", kind, "wiped", items, "elapsed", common.PrettyDuration(time.Since(start))) - logged = time.Now() - } - } - } - it.Release() - if err := batch.Write(); err != nil { - return err - } - if meter != nil { - meter.Mark(int64(items)) - } - if report { - log.Info("Deleted state snapshot leftovers", "kind", kind, "wiped", items, "elapsed", common.PrettyDuration(time.Since(start))) - } - return nil -} diff --git a/core/state/snapshot/wipe_test.go b/core/state/snapshot/wipe_test.go deleted file mode 100644 index c5b340136511..000000000000 --- a/core/state/snapshot/wipe_test.go +++ /dev/null @@ -1,79 +0,0 @@ -// Copyright 2019 The go-ethereum Authors -// This file is part of the go-ethereum library. -// -// The go-ethereum library is free software: you can redistribute it and/or modify -// it under the terms of the GNU Lesser General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// The go-ethereum library is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Lesser General Public License for more details. -// -// You should have received a copy of the GNU Lesser General Public License -// along with the go-ethereum library. If not, see . - -package snapshot - -import ( - "math/rand" - "testing" - - "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/core/rawdb" - "github.com/ethereum/go-ethereum/ethdb/memorydb" -) - -// Tests that given a database with random data content, all parts of a snapshot -// can be crrectly wiped without touching anything else. -func TestWipe(t *testing.T) { - // Create a database with some random snapshot data - db := memorydb.New() - for i := 0; i < 128; i++ { - rawdb.WriteAccountSnapshot(db, randomHash(), randomHash().Bytes()) - } - // Add some random non-snapshot data too to make wiping harder - for i := 0; i < 500; i++ { - // Generate keys with wrong length for a state snapshot item - keysuffix := make([]byte, 31) - rand.Read(keysuffix) - db.Put(append(rawdb.SnapshotAccountPrefix, keysuffix...), randomHash().Bytes()) - keysuffix = make([]byte, 33) - rand.Read(keysuffix) - db.Put(append(rawdb.SnapshotAccountPrefix, keysuffix...), randomHash().Bytes()) - } - count := func() (items int) { - it := db.NewIterator(rawdb.SnapshotAccountPrefix, nil) - defer it.Release() - for it.Next() { - if len(it.Key()) == len(rawdb.SnapshotAccountPrefix)+common.HashLength { - items++ - } - } - return items - } - // Sanity check that all the keys are present - if items := count(); items != 128 { - t.Fatalf("snapshot size mismatch: have %d, want %d", items, 128) - } - // Wipe the accounts - if err := wipeKeyRange(db, "accounts", rawdb.SnapshotAccountPrefix, nil, nil, - len(rawdb.SnapshotAccountPrefix)+common.HashLength, snapWipedAccountMeter, true); err != nil { - t.Fatal(err) - } - // Iterate over the database end ensure no snapshot information remains - if items := count(); items != 0 { - t.Fatalf("snapshot size mismatch: have %d, want %d", items, 0) - } - // Iterate over the database and ensure miscellaneous items are present - items := 0 - it := db.NewIterator(nil, nil) - defer it.Release() - for it.Next() { - items++ - } - if items != 1000 { - t.Fatalf("misc item count mismatch: have %d, want %d", items, 1000) - } -} From 546ce97a237390f616a3bf8afed9b97673235b13 Mon Sep 17 00:00:00 2001 From: rjl493456442 Date: Wed, 4 May 2022 21:09:14 +0800 Subject: [PATCH 13/30] Update core/state/snapshot/generate.go Co-authored-by: Martin Holst Swende --- core/state/snapshot/generate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 43de4275e83a..ee57395bf59b 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -346,7 +346,7 @@ func (dl *diskLayer) generateRange(ctx *generatorContext, root common.Hash, pref snapFailedRangeProofMeter.Mark(1) // Special case, the entire trie is missing. In the original trie scheme, - // all the duplicated subtries will be filtered ot(only one copy of data + // all the duplicated subtries will be filtered out (only one copy of data // will be stored). While in the snapshot model, all the storage tries // belong to different contracts will be kept even they are duplicated. // Track it to a certain extent remove the noise data used for statistics. From b88d7ac4a7553b897fa8cf86bcb2812d04eeb034 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Thu, 5 May 2022 13:14:12 +0800 Subject: [PATCH 14/30] ethdb: extend db test suite and fix memorydb iterator --- ethdb/dbtest/testsuite.go | 26 ++++++++++++++++++++++++-- ethdb/memorydb/memorydb.go | 18 +++++++++++++----- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/ethdb/dbtest/testsuite.go b/ethdb/dbtest/testsuite.go index 6b206af48d5e..b06ba6f977da 100644 --- a/ethdb/dbtest/testsuite.go +++ b/ethdb/dbtest/testsuite.go @@ -36,8 +36,8 @@ func TestDatabaseSuite(t *testing.T, New func() ethdb.KeyValueStore) { 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"}}, @@ -124,6 +124,28 @@ func TestDatabaseSuite(t *testing.T, New func() ethdb.KeyValueStore) { if idx != len(tt.order) { t.Errorf("test %d: iteration terminated prematurely: have %d, want %d", i, idx, len(tt.order)) } + + // Backward iteration since the last position + idx = len(tt.order) + for it.Prev() { + idx-- + if idx < 0 { + 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)) + break + } + if !bytes.Equal(it.Key(), []byte(tt.order[idx])) { + t.Errorf("test %d: item %d: key mismatch: have %s, want %s", i, idx, string(it.Key()), tt.order[idx]) + } + if !bytes.Equal(it.Value(), []byte(tt.content[tt.order[idx]])) { + t.Errorf("test %d: item %d: value mismatch: have %s, want %s", i, idx, string(it.Value()), tt.content[tt.order[idx]]) + } + } + if err := it.Error(); err != nil { + t.Errorf("test %d: iteration failed: %v", i, err) + } + if idx != 0 { + t.Errorf("test %d: iteration terminated prematurely: have %d, want %d", i, idx, len(tt.order)) + } db.Close() } }) diff --git a/ethdb/memorydb/memorydb.go b/ethdb/memorydb/memorydb.go index 80757399c7f6..dbc411b85d39 100644 --- a/ethdb/memorydb/memorydb.go +++ b/ethdb/memorydb/memorydb.go @@ -296,12 +296,12 @@ func (it *iterator) Prev() bool { if len(it.keys) == 0 { return false } - // Short circuit if the iterator is not yet initialized + // Short circuit if the iterator is already exhausted in the backward direction. if it.index == -1 { return false } it.index -= 1 - return true + return it.index != -1 } // Next moves the iterator to the next key/value pair. It returns whether the @@ -311,12 +311,12 @@ func (it *iterator) Next() bool { if len(it.keys) == 0 { return false } - // Short circuit if iterator is already exhausted. - if it.index == len(it.keys)-1 { + // Short circuit if iterator is already exhausted in the forward direction. + if it.index == len(it.keys) { return false } it.index += 1 - return true + return it.index != len(it.keys) } // Error returns any accumulated error. Exhausting all the key/value pairs @@ -333,6 +333,10 @@ func (it *iterator) Key() []byte { if it.index == -1 { return nil } + // Short circuit if iterator is already exhausted + if it.index >= len(it.keys) { + return nil + } return []byte(it.keys[it.index]) } @@ -344,6 +348,10 @@ func (it *iterator) Value() []byte { if it.index == -1 { return nil } + // Short circuit if iterator is already exhausted + if it.index >= len(it.keys) { + return nil + } return it.values[it.index] } From e00ff21abcb5ffae43904ed3cb9396f9b681396d Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Thu, 5 May 2022 13:16:23 +0800 Subject: [PATCH 15/30] ethdb/dbtest: rollback changes --- ethdb/dbtest/testsuite.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ethdb/dbtest/testsuite.go b/ethdb/dbtest/testsuite.go index b06ba6f977da..53f74863c868 100644 --- a/ethdb/dbtest/testsuite.go +++ b/ethdb/dbtest/testsuite.go @@ -36,8 +36,8 @@ func TestDatabaseSuite(t *testing.T, New func() ethdb.KeyValueStore) { 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"}}, From 54caa24fa35011b41cc0a8f14e719405263e86bb Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Thu, 5 May 2022 13:18:42 +0800 Subject: [PATCH 16/30] ethdb/memorydb: simplify iteration --- ethdb/memorydb/memorydb.go | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/ethdb/memorydb/memorydb.go b/ethdb/memorydb/memorydb.go index dbc411b85d39..60eb57ebbd40 100644 --- a/ethdb/memorydb/memorydb.go +++ b/ethdb/memorydb/memorydb.go @@ -329,12 +329,8 @@ func (it *iterator) Error() error { // should not modify the contents of the returned slice, and its contents may // change on the next call to Next. func (it *iterator) Key() []byte { - // Short circuit if iterator is not initialized yet. - if it.index == -1 { - return nil - } - // Short circuit if iterator is already exhausted - if it.index >= len(it.keys) { + // Short circuit if iterator is not in a valid position + if it.index < 0 || it.index >= len(it.keys) { return nil } return []byte(it.keys[it.index]) @@ -344,12 +340,8 @@ func (it *iterator) Key() []byte { // caller should not modify the contents of the returned slice, and its contents // may change on the next call to Next. func (it *iterator) Value() []byte { - // Short circuit if iterator is not initialized yet. - if it.index == -1 { - return nil - } - // Short circuit if iterator is already exhausted - if it.index >= len(it.keys) { + // Short circuit if iterator is not in a valid position + if it.index < 0 || it.index >= len(it.keys) { return nil } return it.values[it.index] From 7fca158f3c327427d58136783c3123270e9d648d Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Thu, 5 May 2022 21:33:20 +0800 Subject: [PATCH 17/30] core/state/snapshot: update dangling counter --- core/state/snapshot/context.go | 7 +------ core/state/snapshot/generate.go | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/core/state/snapshot/context.go b/core/state/snapshot/context.go index 832634695955..e5a9ead4d1ef 100644 --- a/core/state/snapshot/context.go +++ b/core/state/snapshot/context.go @@ -180,10 +180,7 @@ func (ctx *generatorContext) removeStorageBefore(account common.Hash) { // a step. An error will be returned if the initial position of iterator is not // in the given account range. func (ctx *generatorContext) removeStorageAt(account common.Hash) error { - var ( - count uint64 - iter = ctx.iterator(snapStorage) - ) + var iter = ctx.iterator(snapStorage) for iter.Next() { key := iter.Key() cmp := bytes.Compare(key[1:1+common.HashLength], account.Bytes()) @@ -195,9 +192,7 @@ func (ctx *generatorContext) removeStorageAt(account common.Hash) error { break } ctx.batch.Delete(key) - count += 1 } - ctx.stats.dangling += count return nil } diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index ee57395bf59b..93f9481e29db 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -705,7 +705,7 @@ func (dl *diskLayer) generate(stats *generatorStats) { ctx.batch.Reset() log.Info("Generated state snapshot", "accounts", stats.accounts, "slots", stats.slots, - "storage", stats.storage, "elapsed", common.PrettyDuration(time.Since(stats.start))) + "storage", stats.storage, "dangling", stats.dangling, "elapsed", common.PrettyDuration(time.Since(stats.start))) dl.lock.Lock() dl.genMarker = nil From e178af17b5350b60672ddc1f7a400dc0fe06ea26 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Sat, 7 May 2022 10:53:32 +0800 Subject: [PATCH 18/30] core/state/snapshot: release iterators --- core/state/snapshot/generate.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 93f9481e29db..3a8828f0395a 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -495,6 +495,8 @@ func (dl *diskLayer) checkAndFlush(ctx *generatorContext, current []byte) error ctx.stats.Log("Aborting state snapshot generation", dl.root, current) return newAbortErr(abort) // bubble up an error for interruption } + // Don't hold the iterators too long, release them to let compactor works + ctx.reopenIterators() } if time.Since(ctx.logged) > 8*time.Second { ctx.stats.Log("Generating state snapshot", dl.root, current) @@ -635,10 +637,7 @@ func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) er if len(accMarker) > 0 { accountRange = 1 } - var ( - iterTime = time.Now() - origin = common.CopyBytes(accMarker) - ) + origin := common.CopyBytes(accMarker) for { exhausted, last, err := dl.generateRange(ctx, dl.root, rawdb.SnapshotAccountPrefix, snapAccount, origin, accountRange, onAccount, FullAccountRLP) if err != nil { @@ -652,11 +651,6 @@ func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) er ctx.removeStorageLeft() break } - // Don't hold iterator too long, re-open them to let compactor works. - if time.Since(iterTime) > time.Minute*3 { - iterTime = time.Now() - ctx.reopenIterators() - } } return nil } From 3acad8d7d0a90b28361dc2b8ffafef205471686c Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Sat, 7 May 2022 13:32:54 +0800 Subject: [PATCH 19/30] core/state/snapshot: update metrics --- core/state/snapshot/context.go | 8 +++++++- core/state/snapshot/generate.go | 4 ++-- core/state/snapshot/metrics.go | 1 + 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/core/state/snapshot/context.go b/core/state/snapshot/context.go index e5a9ead4d1ef..4bf977baf2e9 100644 --- a/core/state/snapshot/context.go +++ b/core/state/snapshot/context.go @@ -180,7 +180,10 @@ func (ctx *generatorContext) removeStorageBefore(account common.Hash) { // a step. An error will be returned if the initial position of iterator is not // in the given account range. func (ctx *generatorContext) removeStorageAt(account common.Hash) error { - var iter = ctx.iterator(snapStorage) + var ( + count int64 + iter = ctx.iterator(snapStorage) + ) for iter.Next() { key := iter.Key() cmp := bytes.Compare(key[1:1+common.HashLength], account.Bytes()) @@ -192,7 +195,9 @@ func (ctx *generatorContext) removeStorageAt(account common.Hash) error { break } ctx.batch.Delete(key) + count += 1 } + snapWipedStorageMeter.Mark(count) return nil } @@ -208,4 +213,5 @@ func (ctx *generatorContext) removeStorageLeft() { count += 1 } ctx.stats.dangling += count + snapDanglingStorageMeter.Mark(int64(count)) } diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 3a8828f0395a..6560e55f5c78 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -219,13 +219,13 @@ func (dl *diskLayer) proveRange(ctx *generatorContext, root common.Hash, prefix } } // Update metrics for database iteration and merkle proving - if kind == "storage" { + if kind == snapStorage { snapStorageSnapReadCounter.Inc(time.Since(start).Nanoseconds()) } else { snapAccountSnapReadCounter.Inc(time.Since(start).Nanoseconds()) } defer func(start time.Time) { - if kind == "storage" { + if kind == snapStorage { snapStorageProveCounter.Inc(time.Since(start).Nanoseconds()) } else { snapAccountProveCounter.Inc(time.Since(start).Nanoseconds()) diff --git a/core/state/snapshot/metrics.go b/core/state/snapshot/metrics.go index 23b240c3e74f..4806e8f17537 100644 --- a/core/state/snapshot/metrics.go +++ b/core/state/snapshot/metrics.go @@ -28,6 +28,7 @@ var ( snapRecoveredStorageMeter = metrics.NewRegisteredMeter("state/snapshot/generation/storage/recovered", nil) snapWipedStorageMeter = metrics.NewRegisteredMeter("state/snapshot/generation/storage/wiped", nil) snapMissallStorageMeter = metrics.NewRegisteredMeter("state/snapshot/generation/storage/missall", nil) + snapDanglingStorageMeter = metrics.NewRegisteredMeter("state/snapshot/generation/storage/dangling", nil) snapSuccessfulRangeProofMeter = metrics.NewRegisteredMeter("state/snapshot/generation/proof/success", nil) snapFailedRangeProofMeter = metrics.NewRegisteredMeter("state/snapshot/generation/proof/failure", nil) From 9a1ccd9160313b3d2b6d51e8660f60139df021c4 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Sat, 7 May 2022 14:09:04 +0800 Subject: [PATCH 20/30] core/state/snapshot: update time metrics --- core/state/snapshot/context.go | 6 ++++++ core/state/snapshot/generate.go | 13 +++++-------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/core/state/snapshot/context.go b/core/state/snapshot/context.go index 4bf977baf2e9..718cae4f38d2 100644 --- a/core/state/snapshot/context.go +++ b/core/state/snapshot/context.go @@ -160,6 +160,7 @@ func (ctx *generatorContext) iterator(kind string) ethdb.Iterator { func (ctx *generatorContext) removeStorageBefore(account common.Hash) { var ( count uint64 + start = time.Now() iter = ctx.storage ) for iter.Next() { @@ -172,6 +173,7 @@ func (ctx *generatorContext) removeStorageBefore(account common.Hash) { count += 1 } ctx.stats.dangling += count + snapStorageWriteCounter.Inc(time.Since(start).Nanoseconds()) } // removeStorageAt iterates and deletes all storage snapshots which are located @@ -182,6 +184,7 @@ func (ctx *generatorContext) removeStorageBefore(account common.Hash) { func (ctx *generatorContext) removeStorageAt(account common.Hash) error { var ( count int64 + start = time.Now() iter = ctx.iterator(snapStorage) ) for iter.Next() { @@ -198,6 +201,7 @@ func (ctx *generatorContext) removeStorageAt(account common.Hash) error { count += 1 } snapWipedStorageMeter.Mark(count) + snapStorageWriteCounter.Inc(time.Since(start).Nanoseconds()) return nil } @@ -206,6 +210,7 @@ func (ctx *generatorContext) removeStorageAt(account common.Hash) error { func (ctx *generatorContext) removeStorageLeft() { var ( count uint64 + start = time.Now() iter = ctx.iterator(snapStorage) ) for iter.Next() { @@ -214,4 +219,5 @@ func (ctx *generatorContext) removeStorageLeft() { } ctx.stats.dangling += count snapDanglingStorageMeter.Mark(int64(count)) + snapStorageWriteCounter.Inc(time.Since(start).Nanoseconds()) } diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 6560e55f5c78..30a3d1d46cf2 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -556,19 +556,17 @@ func generateStorages(ctx *generatorContext, dl *diskLayer, account common.Hash, // from the given origin position. func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) error { onAccount := func(key []byte, val []byte, write bool, delete bool) error { - var ( - start = time.Now() - accountHash = common.BytesToHash(key) - ) // Make sure to clear all dangling storages before processing this account + accountHash := common.BytesToHash(key) ctx.removeStorageBefore(accountHash) + start := time.Now() if delete { rawdb.DeleteAccountSnapshot(ctx.batch, accountHash) snapWipedAccountMeter.Mark(1) + snapAccountWriteCounter.Inc(time.Since(start).Nanoseconds()) ctx.removeStorageAt(accountHash) - snapAccountWriteCounter.Inc(time.Since(start).Nanoseconds()) return nil } // Retrieve the current account and flatten it into the internal format @@ -611,14 +609,13 @@ func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) er if err := dl.checkAndFlush(ctx, marker); err != nil { return err } + snapAccountWriteCounter.Inc(time.Since(start).Nanoseconds()) + // If the iterated account is the contract, create a further loop to // verify or regenerate the contract storage. if acc.Root == emptyRoot { ctx.removeStorageAt(accountHash) - snapAccountWriteCounter.Inc(time.Since(start).Nanoseconds()) } else { - snapAccountWriteCounter.Inc(time.Since(start).Nanoseconds()) - var storeMarker []byte if accMarker != nil && bytes.Equal(accountHash[:], accMarker) && len(dl.genMarker) > common.HashLength { storeMarker = dl.genMarker[common.HashLength:] From 5df12259a0a1c67cb07b5700b4c2fd60cc6baa43 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Sat, 7 May 2022 14:34:34 +0800 Subject: [PATCH 21/30] metrics/influxdb: temp solution to present counter meaningfully, remove it --- metrics/influxdb/influxdb.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/metrics/influxdb/influxdb.go b/metrics/influxdb/influxdb.go index 52d00910343e..dac9e824775a 100644 --- a/metrics/influxdb/influxdb.go +++ b/metrics/influxdb/influxdb.go @@ -129,17 +129,15 @@ func (r *reporter) send() error { switch metric := i.(type) { case metrics.Counter: - v := metric.Count() - l := r.cache[name] + count := metric.Count() pts = append(pts, client.Point{ Measurement: fmt.Sprintf("%s%s.count", namespace, name), Tags: r.tags, Fields: map[string]interface{}{ - "value": v - l, + "value": count, }, Time: now, }) - r.cache[name] = v case metrics.Gauge: ms := metric.Snapshot() pts = append(pts, client.Point{ From d3fb32137b425b7933299e8d92c48dd93a959889 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Sat, 7 May 2022 19:55:33 +0800 Subject: [PATCH 22/30] add debug log, revert later --- core/state/snapshot/context.go | 19 ++++++++++++++++++- core/state/snapshot/generate.go | 6 +++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/core/state/snapshot/context.go b/core/state/snapshot/context.go index 718cae4f38d2..dfb4943e28af 100644 --- a/core/state/snapshot/context.go +++ b/core/state/snapshot/context.go @@ -43,6 +43,8 @@ type generatorStats struct { slots uint64 // Number of storage slots indexed(generated or recovered) dangling uint64 // Number of dangling storage slots storage common.StorageSize // Total account and storage slot size(generation or recovery) + reopens int // Counter of re-open iterators + reopen time.Duration // Total time spent on re-open iterations } // Log creates an contextual log with the given message and the context pulled @@ -69,6 +71,16 @@ func (gs *generatorStats) Log(msg string, root common.Hash, marker []byte) { "storage", gs.storage, "dangling", gs.dangling, "elapsed", common.PrettyDuration(time.Since(gs.start)), + "reopen", gs.reopens, + "reopen-time", common.PrettyDuration(gs.reopen), + "account-prove", common.PrettyDuration(snapAccountProveCounter.Count()), + "account-trieread", common.PrettyDuration(snapAccountTrieReadCounter.Count()), + "account-snapread", common.PrettyDuration(snapAccountSnapReadCounter.Count()), + "account-write", common.PrettyDuration(snapAccountWriteCounter.Count()), + "storage-prove", common.PrettyDuration(snapStorageProveCounter.Count()), + "storage-trieread", common.PrettyDuration(snapStorageTrieReadCounter.Count()), + "storage-snapread", common.PrettyDuration(snapStorageSnapReadCounter.Count()), + "storage-write", common.PrettyDuration(snapStorageWriteCounter.Count()), }...) // Calculate the estimated indexing time based on current stats if len(marker) > 0 { @@ -96,6 +108,7 @@ type generatorContext struct { // newGeneratorContext initializes the context for generation. func newGeneratorContext(stats *generatorStats, db ethdb.KeyValueStore, accMarker []byte, storageMarker []byte) *generatorContext { + start := time.Now() ctx := &generatorContext{ stats: stats, db: db, @@ -104,6 +117,7 @@ func newGeneratorContext(stats *generatorStats, db ethdb.KeyValueStore, accMarke } ctx.openIterator(snapAccount, accMarker) ctx.openIterator(snapStorage, storageMarker) + ctx.stats.reopen += time.Since(start) return ctx } @@ -124,9 +138,10 @@ func (ctx *generatorContext) openIterator(kind string, start []byte) { // reopens them in the interruption position. It's aim for not blocking // leveldb compaction. func (ctx *generatorContext) reopenIterators() { + start := time.Now() for i, iter := range []ethdb.Iterator{ctx.account, ctx.storage} { var ( - key = iter.Key() + key = iter.Key() // It will panic if iterator is exhausted cur = key[1:] ) kind := snapAccount @@ -136,6 +151,8 @@ func (ctx *generatorContext) reopenIterators() { iter.Release() ctx.openIterator(kind, cur) } + ctx.stats.reopens += 1 + ctx.stats.reopen += time.Since(start) } // close releases all the held resources. diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 30a3d1d46cf2..1e4f853c34ba 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -599,9 +599,11 @@ func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) er ctx.stats.storage += common.StorageSize(1 + common.HashLength + dataLen) ctx.stats.accounts++ } - marker := accountHash[:] + snapAccountWriteCounter.Inc(time.Since(start).Nanoseconds()) + // If the snap generation goes here after interrupted, genMarker may go backward // when last genMarker is consisted of accountHash and storageHash + marker := accountHash[:] if accMarker != nil && bytes.Equal(marker, accMarker) && len(dl.genMarker) > common.HashLength { marker = dl.genMarker[:] } @@ -609,8 +611,6 @@ func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) er if err := dl.checkAndFlush(ctx, marker); err != nil { return err } - snapAccountWriteCounter.Inc(time.Since(start).Nanoseconds()) - // If the iterated account is the contract, create a further loop to // verify or regenerate the contract storage. if acc.Root == emptyRoot { From 83f60afd5838bb4ed880247bff6b8a52fcd49c40 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Sat, 7 May 2022 20:20:14 +0800 Subject: [PATCH 23/30] core/state/snapshot: fix iterator panic --- core/state/snapshot/context.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/state/snapshot/context.go b/core/state/snapshot/context.go index dfb4943e28af..aac36f4b0ff8 100644 --- a/core/state/snapshot/context.go +++ b/core/state/snapshot/context.go @@ -140,16 +140,16 @@ func (ctx *generatorContext) openIterator(kind string, start []byte) { func (ctx *generatorContext) reopenIterators() { start := time.Now() for i, iter := range []ethdb.Iterator{ctx.account, ctx.storage} { - var ( - key = iter.Key() // It will panic if iterator is exhausted - cur = key[1:] - ) + key := iter.Key() + if len(key) == 0 { // nil or []byte{} + continue // the iterator may be already exhausted + } kind := snapAccount if i == 1 { kind = snapStorage } iter.Release() - ctx.openIterator(kind, cur) + ctx.openIterator(kind, key[1:]) } ctx.stats.reopens += 1 ctx.stats.reopen += time.Since(start) From 78ed54276269b459ef91a3aa6e3c4cba67efb4c6 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Sat, 7 May 2022 20:54:53 +0800 Subject: [PATCH 24/30] all: customized snapshot iterator for backward iteration --- core/rawdb/table.go | 6 -- core/state/snapshot/context.go | 38 ++++------ core/state/snapshot/generate.go | 5 +- core/state/snapshot/metrics.go | 4 +- core/state/snapshot/snap_iterator.go | 91 +++++++++++++++++++++++ core/state/snapshot/snap_iterator_test.go | 86 +++++++++++++++++++++ core/state/snapshot/snapshot.go | 5 +- ethdb/dbtest/testsuite.go | 22 ------ ethdb/iterator.go | 4 - ethdb/memorydb/memorydb.go | 17 +---- 10 files changed, 201 insertions(+), 77 deletions(-) create mode 100644 core/state/snapshot/snap_iterator.go create mode 100644 core/state/snapshot/snap_iterator_test.go diff --git a/core/rawdb/table.go b/core/rawdb/table.go index 9fb32cd182b4..6d6fa0555da9 100644 --- a/core/rawdb/table.go +++ b/core/rawdb/table.go @@ -270,12 +270,6 @@ type tableIterator struct { prefix string } -// Prev moves the iterator to the previous key/value pair. It returns false -// if the iterator is exhausted. -func (iter *tableIterator) Prev() bool { - return iter.iter.Prev() -} - // Next moves the iterator to the next key/value pair. It returns whether the // iterator is exhausted. func (iter *tableIterator) Next() bool { diff --git a/core/state/snapshot/context.go b/core/state/snapshot/context.go index aac36f4b0ff8..2a28d363813c 100644 --- a/core/state/snapshot/context.go +++ b/core/state/snapshot/context.go @@ -43,8 +43,6 @@ type generatorStats struct { slots uint64 // Number of storage slots indexed(generated or recovered) dangling uint64 // Number of dangling storage slots storage common.StorageSize // Total account and storage slot size(generation or recovery) - reopens int // Counter of re-open iterators - reopen time.Duration // Total time spent on re-open iterations } // Log creates an contextual log with the given message and the context pulled @@ -71,8 +69,6 @@ func (gs *generatorStats) Log(msg string, root common.Hash, marker []byte) { "storage", gs.storage, "dangling", gs.dangling, "elapsed", common.PrettyDuration(time.Since(gs.start)), - "reopen", gs.reopens, - "reopen-time", common.PrettyDuration(gs.reopen), "account-prove", common.PrettyDuration(snapAccountProveCounter.Count()), "account-trieread", common.PrettyDuration(snapAccountTrieReadCounter.Count()), "account-snapread", common.PrettyDuration(snapAccountSnapReadCounter.Count()), @@ -81,6 +77,7 @@ func (gs *generatorStats) Log(msg string, root common.Hash, marker []byte) { "storage-trieread", common.PrettyDuration(snapStorageTrieReadCounter.Count()), "storage-snapread", common.PrettyDuration(snapStorageSnapReadCounter.Count()), "storage-write", common.PrettyDuration(snapStorageWriteCounter.Count()), + "storage-clean", common.PrettyDuration(snapStorageCleanCounter.Count()), }...) // Calculate the estimated indexing time based on current stats if len(marker) > 0 { @@ -100,15 +97,14 @@ func (gs *generatorStats) Log(msg string, root common.Hash, marker []byte) { type generatorContext struct { stats *generatorStats // Generation statistic collection db ethdb.KeyValueStore // Key-value store containing the snapshot data - account ethdb.Iterator // Iterator of account snapshot data - storage ethdb.Iterator // Iterator of storage snapshot data + account *snapIter // Iterator of account snapshot data + storage *snapIter // Iterator of storage snapshot data batch ethdb.Batch // Database batch for writing batch data atomically logged time.Time // The timestamp when last generation progress was displayed } // newGeneratorContext initializes the context for generation. func newGeneratorContext(stats *generatorStats, db ethdb.KeyValueStore, accMarker []byte, storageMarker []byte) *generatorContext { - start := time.Now() ctx := &generatorContext{ stats: stats, db: db, @@ -117,7 +113,6 @@ func newGeneratorContext(stats *generatorStats, db ethdb.KeyValueStore, accMarke } ctx.openIterator(snapAccount, accMarker) ctx.openIterator(snapStorage, storageMarker) - ctx.stats.reopen += time.Since(start) return ctx } @@ -127,22 +122,21 @@ func newGeneratorContext(stats *generatorStats, db ethdb.KeyValueStore, accMarke func (ctx *generatorContext) openIterator(kind string, start []byte) { if kind == snapAccount { iter := ctx.db.NewIterator(rawdb.SnapshotAccountPrefix, start) - ctx.account = rawdb.NewKeyLengthIterator(iter, 1+common.HashLength) + ctx.account = newSnapIter(rawdb.NewKeyLengthIterator(iter, 1+common.HashLength)) return } iter := ctx.db.NewIterator(rawdb.SnapshotStoragePrefix, start) - ctx.storage = rawdb.NewKeyLengthIterator(iter, 1+2*common.HashLength) + ctx.storage = newSnapIter(rawdb.NewKeyLengthIterator(iter, 1+2*common.HashLength)) } // reopenIterators releases the held two global database iterators and // reopens them in the interruption position. It's aim for not blocking // leveldb compaction. func (ctx *generatorContext) reopenIterators() { - start := time.Now() for i, iter := range []ethdb.Iterator{ctx.account, ctx.storage} { key := iter.Key() if len(key) == 0 { // nil or []byte{} - continue // the iterator may be already exhausted + continue // the iterator may already be exhausted } kind := snapAccount if i == 1 { @@ -151,8 +145,6 @@ func (ctx *generatorContext) reopenIterators() { iter.Release() ctx.openIterator(kind, key[1:]) } - ctx.stats.reopens += 1 - ctx.stats.reopen += time.Since(start) } // close releases all the held resources. @@ -162,7 +154,7 @@ func (ctx *generatorContext) close() { } // iterator returns the corresponding iterator specified by the kind. -func (ctx *generatorContext) iterator(kind string) ethdb.Iterator { +func (ctx *generatorContext) iterator(kind string) *snapIter { if kind == snapAccount { return ctx.account } @@ -183,14 +175,14 @@ func (ctx *generatorContext) removeStorageBefore(account common.Hash) { for iter.Next() { key := iter.Key() if bytes.Compare(key[1:1+common.HashLength], account.Bytes()) >= 0 { - iter.Prev() - return + iter.Discard() + break } ctx.batch.Delete(key) count += 1 } ctx.stats.dangling += count - snapStorageWriteCounter.Inc(time.Since(start).Nanoseconds()) + snapStorageCleanCounter.Inc(time.Since(start).Nanoseconds()) } // removeStorageAt iterates and deletes all storage snapshots which are located @@ -202,7 +194,7 @@ func (ctx *generatorContext) removeStorageAt(account common.Hash) error { var ( count int64 start = time.Now() - iter = ctx.iterator(snapStorage) + iter = ctx.storage ) for iter.Next() { key := iter.Key() @@ -211,14 +203,14 @@ func (ctx *generatorContext) removeStorageAt(account common.Hash) error { return errors.New("invalid iterator position") } if cmp > 0 { - iter.Prev() + iter.Discard() break } ctx.batch.Delete(key) count += 1 } snapWipedStorageMeter.Mark(count) - snapStorageWriteCounter.Inc(time.Since(start).Nanoseconds()) + snapStorageCleanCounter.Inc(time.Since(start).Nanoseconds()) return nil } @@ -228,7 +220,7 @@ func (ctx *generatorContext) removeStorageLeft() { var ( count uint64 start = time.Now() - iter = ctx.iterator(snapStorage) + iter = ctx.storage ) for iter.Next() { ctx.batch.Delete(iter.Key()) @@ -236,5 +228,5 @@ func (ctx *generatorContext) removeStorageLeft() { } ctx.stats.dangling += count snapDanglingStorageMeter.Mark(int64(count)) - snapStorageWriteCounter.Inc(time.Since(start).Nanoseconds()) + snapStorageCleanCounter.Inc(time.Since(start).Nanoseconds()) } diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 1e4f853c34ba..88306e16a72d 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -187,14 +187,14 @@ func (dl *diskLayer) proveRange(ctx *generatorContext, root common.Hash, prefix // Move the iterator a step back since we iterate one extra element // out. if !bytes.Equal(key[:len(prefix)], prefix) { - iter.Prev() + iter.Discard() break } // Break if we've reached the max size, and signal that we're not // done yet. Move the iterator a step back since we iterate one // extra element out. if len(keys) == max { - iter.Prev() + iter.Discard() diskMore = true break } @@ -648,6 +648,7 @@ func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) er ctx.removeStorageLeft() break } + accountRange = accountCheckRange } return nil } diff --git a/core/state/snapshot/metrics.go b/core/state/snapshot/metrics.go index 4806e8f17537..42fa6fafaf38 100644 --- a/core/state/snapshot/metrics.go +++ b/core/state/snapshot/metrics.go @@ -46,6 +46,8 @@ var ( snapStorageTrieReadCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/storage/trieread", nil) // snapStorageSnapReadCounter measures time spent on the snapshot storage iteration snapStorageSnapReadCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/storage/snapread", nil) - // snapStorageWriteCounter measures time spent on writing/updating/deleting storages + // snapStorageWriteCounter measures time spent on writing/updating storages snapStorageWriteCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/storage/write", nil) + // snapStorageCleanCounter measures time spent on deleting storages + snapStorageCleanCounter = metrics.NewRegisteredCounter("state/snapshot/generation/duration/storage/clean", nil) ) diff --git a/core/state/snapshot/snap_iterator.go b/core/state/snapshot/snap_iterator.go new file mode 100644 index 000000000000..56c1fbd658a0 --- /dev/null +++ b/core/state/snapshot/snap_iterator.go @@ -0,0 +1,91 @@ +// Copyright 2022 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see + +package snapshot + +import ( + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/ethdb" +) + +// snapIter is a wrapper of underlying database iterator. It extends +// the basic iteration interface by adding Discard which can cache +// the element internally where the iterator is currently located. +type snapIter struct { + it ethdb.Iterator + key []byte + val []byte + checkLast bool +} + +// newSnapIter initializes the snapshot iterator. +func newSnapIter(it ethdb.Iterator) *snapIter { + return &snapIter{it: it} +} + +// Discard stores the element internally where the iterator is currently located. +func (it *snapIter) Discard() { + if it.it.Key() == nil { + return // nothing to cache + } + it.key = common.CopyBytes(it.it.Key()) + it.val = common.CopyBytes(it.it.Value()) + it.checkLast = false +} + +// Next moves the iterator to the next key/value pair. It returns whether the +// iterator is exhausted. +func (it *snapIter) Next() bool { + if !it.checkLast && it.key != nil { + it.checkLast = true + } else if it.checkLast { + it.checkLast = false + it.key = nil + it.val = nil + } + if it.key != nil { + return true // shift to discarded value + } + return it.it.Next() +} + +// Error returns any accumulated error. Exhausting all the key/value pairs +// is not considered to be an error. +func (it *snapIter) Error() error { return it.it.Error() } + +// Release releases associated resources. Release should always succeed and can +// be called multiple times without causing error. +func (it *snapIter) Release() { it.it.Release() } + +// Key returns the key of the current key/value pair, or nil if done. The caller +// should not modify the contents of the returned slice, and its contents may +// change on the next call to Next. +func (it *snapIter) Key() []byte { + if it.key != nil { + return it.key + } + return it.it.Key() +} + +// Value returns the value of the current key/value pair, or nil if done. The +// caller should not modify the contents of the returned slice, and its contents +// may change on the next call to Next. +func (it *snapIter) Value() []byte { + if it.val != nil { + return it.val + } + return it.it.Value() +} diff --git a/core/state/snapshot/snap_iterator_test.go b/core/state/snapshot/snap_iterator_test.go new file mode 100644 index 000000000000..1887bcd5ada5 --- /dev/null +++ b/core/state/snapshot/snap_iterator_test.go @@ -0,0 +1,86 @@ +// Copyright 2022 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see + +package snapshot + +import ( + "bytes" + "testing" + + "github.com/ethereum/go-ethereum/core/rawdb" +) + +func TestIteratorDiscard(t *testing.T) { + // Create the key-value data store + var ( + content = map[string]string{"k1": "v1", "k2": "v2", "k3": "v3"} + order = []string{"k1", "k2", "k3"} + db = rawdb.NewMemoryDatabase() + ) + for key, val := range content { + if err := db.Put([]byte(key), []byte(val)); err != nil { + t.Fatalf("failed to insert item %s:%s into database: %v", key, val, err) + } + } + // Iterate over the database with the given configs and verify the results + it, idx := newSnapIter(db.NewIterator(nil, nil)), 0 + + // Nothing should be affected for calling Discard on non-initialized iterator + it.Discard() + + for it.Next() { + if len(content) <= idx { + t.Errorf("more items than expected: checking idx=%d (key %q), expecting len=%d", idx, it.Key(), len(order)) + break + } + if !bytes.Equal(it.Key(), []byte(order[idx])) { + t.Errorf("item %d: key mismatch: have %s, want %s", idx, string(it.Key()), order[idx]) + } + if !bytes.Equal(it.Value(), []byte(content[order[idx]])) { + t.Errorf("item %d: value mismatch: have %s, want %s", idx, string(it.Value()), content[order[idx]]) + } + // Should be safe to call discard multiple times + it.Discard() + it.Discard() + + // Shift iterator to the discarded element + it.Next() + if !bytes.Equal(it.Key(), []byte(order[idx])) { + t.Errorf("item %d: key mismatch: have %s, want %s", idx, string(it.Key()), order[idx]) + } + if !bytes.Equal(it.Value(), []byte(content[order[idx]])) { + t.Errorf("item %d: value mismatch: have %s, want %s", idx, string(it.Value()), content[order[idx]]) + } + + // Discard/Next combo should work always + it.Discard() + it.Next() + if !bytes.Equal(it.Key(), []byte(order[idx])) { + t.Errorf("item %d: key mismatch: have %s, want %s", idx, string(it.Key()), order[idx]) + } + if !bytes.Equal(it.Value(), []byte(content[order[idx]])) { + t.Errorf("item %d: value mismatch: have %s, want %s", idx, string(it.Value()), content[order[idx]]) + } + idx++ + } + if err := it.Error(); err != nil { + t.Errorf("iteration failed: %v", err) + } + if idx != len(order) { + t.Errorf("iteration terminated prematurely: have %d, want %d", idx, len(order)) + } + db.Close() +} diff --git a/core/state/snapshot/snapshot.go b/core/state/snapshot/snapshot.go index 76200851e469..95a8ece51fa6 100644 --- a/core/state/snapshot/snapshot.go +++ b/core/state/snapshot/snapshot.go @@ -21,9 +21,6 @@ import ( "bytes" "errors" "fmt" - "sync" - "sync/atomic" - "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/ethdb" @@ -31,6 +28,8 @@ import ( "github.com/ethereum/go-ethereum/metrics" "github.com/ethereum/go-ethereum/rlp" "github.com/ethereum/go-ethereum/trie" + "sync" + "sync/atomic" ) var ( diff --git a/ethdb/dbtest/testsuite.go b/ethdb/dbtest/testsuite.go index 53f74863c868..6b206af48d5e 100644 --- a/ethdb/dbtest/testsuite.go +++ b/ethdb/dbtest/testsuite.go @@ -124,28 +124,6 @@ func TestDatabaseSuite(t *testing.T, New func() ethdb.KeyValueStore) { if idx != len(tt.order) { t.Errorf("test %d: iteration terminated prematurely: have %d, want %d", i, idx, len(tt.order)) } - - // Backward iteration since the last position - idx = len(tt.order) - for it.Prev() { - idx-- - if idx < 0 { - 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)) - break - } - if !bytes.Equal(it.Key(), []byte(tt.order[idx])) { - t.Errorf("test %d: item %d: key mismatch: have %s, want %s", i, idx, string(it.Key()), tt.order[idx]) - } - if !bytes.Equal(it.Value(), []byte(tt.content[tt.order[idx]])) { - t.Errorf("test %d: item %d: value mismatch: have %s, want %s", i, idx, string(it.Value()), tt.content[tt.order[idx]]) - } - } - if err := it.Error(); err != nil { - t.Errorf("test %d: iteration failed: %v", i, err) - } - if idx != 0 { - t.Errorf("test %d: iteration terminated prematurely: have %d, want %d", i, idx, len(tt.order)) - } db.Close() } }) diff --git a/ethdb/iterator.go b/ethdb/iterator.go index be14fef39509..2b49c93a96c4 100644 --- a/ethdb/iterator.go +++ b/ethdb/iterator.go @@ -26,10 +26,6 @@ package ethdb // iterator until exhaustion. An iterator is not safe for concurrent use, but it // is safe to use multiple iterators concurrently. type Iterator interface { - // Prev moves the iterator to the previous key/value pair. It returns false - // if the iterator is exhausted. - Prev() bool - // Next moves the iterator to the next key/value pair. It returns whether the // iterator is exhausted. Next() bool diff --git a/ethdb/memorydb/memorydb.go b/ethdb/memorydb/memorydb.go index 60eb57ebbd40..6a136b7ac573 100644 --- a/ethdb/memorydb/memorydb.go +++ b/ethdb/memorydb/memorydb.go @@ -289,21 +289,6 @@ type iterator struct { values [][]byte } -// Prev moves the iterator to the previous key/value pair. It returns whether the -// iterator is exhausted. -func (it *iterator) Prev() bool { - // Short circuit if iterator has nothing to iterate. - if len(it.keys) == 0 { - return false - } - // Short circuit if the iterator is already exhausted in the backward direction. - if it.index == -1 { - return false - } - it.index -= 1 - return it.index != -1 -} - // Next moves the iterator to the next key/value pair. It returns whether the // iterator is exhausted. func (it *iterator) Next() bool { @@ -407,4 +392,4 @@ func (snap *snapshot) Release() { defer snap.lock.Unlock() snap.db = nil -} +} \ No newline at end of file From 254666a1177aeaa2363fc5683f415a1d5fa76cea Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Mon, 9 May 2022 13:52:38 +0800 Subject: [PATCH 25/30] core, ethdb: polish --- core/state/snapshot/snap_iterator.go | 7 ++++++- core/state/snapshot/snapshot.go | 5 +++-- ethdb/memorydb/memorydb.go | 6 +----- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/core/state/snapshot/snap_iterator.go b/core/state/snapshot/snap_iterator.go index 56c1fbd658a0..790b40d3d5c7 100644 --- a/core/state/snapshot/snap_iterator.go +++ b/core/state/snapshot/snap_iterator.go @@ -68,7 +68,12 @@ func (it *snapIter) Error() error { return it.it.Error() } // Release releases associated resources. Release should always succeed and can // be called multiple times without causing error. -func (it *snapIter) Release() { it.it.Release() } +func (it *snapIter) Release() { + it.checkLast = false + it.key = nil + it.val = nil + it.it.Release() +} // Key returns the key of the current key/value pair, or nil if done. The caller // should not modify the contents of the returned slice, and its contents may diff --git a/core/state/snapshot/snapshot.go b/core/state/snapshot/snapshot.go index 95a8ece51fa6..76200851e469 100644 --- a/core/state/snapshot/snapshot.go +++ b/core/state/snapshot/snapshot.go @@ -21,6 +21,9 @@ import ( "bytes" "errors" "fmt" + "sync" + "sync/atomic" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/ethdb" @@ -28,8 +31,6 @@ import ( "github.com/ethereum/go-ethereum/metrics" "github.com/ethereum/go-ethereum/rlp" "github.com/ethereum/go-ethereum/trie" - "sync" - "sync/atomic" ) var ( diff --git a/ethdb/memorydb/memorydb.go b/ethdb/memorydb/memorydb.go index 6a136b7ac573..378ef7413600 100644 --- a/ethdb/memorydb/memorydb.go +++ b/ethdb/memorydb/memorydb.go @@ -36,10 +36,6 @@ var ( // the provided memory database. errMemorydbNotFound = errors.New("not found") - // errIteratorReleased is returned if callers want to retrieve data from a - // released iterator. - errIteratorReleased = errors.New("iterator released") - // errSnapshotReleased is returned if callers want to retrieve data from a // released snapshot. errSnapshotReleased = errors.New("snapshot released") @@ -392,4 +388,4 @@ func (snap *snapshot) Release() { defer snap.lock.Unlock() snap.db = nil -} \ No newline at end of file +} From 1f5442d04e194c8977fd0f4da25dcfdaf329018c Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Mon, 9 May 2022 13:58:00 +0800 Subject: [PATCH 26/30] core/state/snapshot: remove debug log --- core/state/snapshot/context.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/core/state/snapshot/context.go b/core/state/snapshot/context.go index 2a28d363813c..60c52645a041 100644 --- a/core/state/snapshot/context.go +++ b/core/state/snapshot/context.go @@ -69,15 +69,6 @@ func (gs *generatorStats) Log(msg string, root common.Hash, marker []byte) { "storage", gs.storage, "dangling", gs.dangling, "elapsed", common.PrettyDuration(time.Since(gs.start)), - "account-prove", common.PrettyDuration(snapAccountProveCounter.Count()), - "account-trieread", common.PrettyDuration(snapAccountTrieReadCounter.Count()), - "account-snapread", common.PrettyDuration(snapAccountSnapReadCounter.Count()), - "account-write", common.PrettyDuration(snapAccountWriteCounter.Count()), - "storage-prove", common.PrettyDuration(snapStorageProveCounter.Count()), - "storage-trieread", common.PrettyDuration(snapStorageTrieReadCounter.Count()), - "storage-snapread", common.PrettyDuration(snapStorageSnapReadCounter.Count()), - "storage-write", common.PrettyDuration(snapStorageWriteCounter.Count()), - "storage-clean", common.PrettyDuration(snapStorageCleanCounter.Count()), }...) // Calculate the estimated indexing time based on current stats if len(marker) > 0 { From 55577d0cbe106bac693450b4b7712d45744ed47c Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Tue, 10 May 2022 11:34:51 +0800 Subject: [PATCH 27/30] core/state/snapshot: address comments from peter --- core/state/snapshot/context.go | 44 ++++++++------- core/state/snapshot/generate.go | 4 +- ...{snap_iterator.go => holdable_iterator.go} | 53 ++++++++++--------- ...ator_test.go => holdable_iterator_test.go} | 10 ++-- 4 files changed, 55 insertions(+), 56 deletions(-) rename core/state/snapshot/{snap_iterator.go => holdable_iterator.go} (64%) rename core/state/snapshot/{snap_iterator_test.go => holdable_iterator_test.go} (96%) diff --git a/core/state/snapshot/context.go b/core/state/snapshot/context.go index 60c52645a041..645491f1b89d 100644 --- a/core/state/snapshot/context.go +++ b/core/state/snapshot/context.go @@ -88,8 +88,8 @@ func (gs *generatorStats) Log(msg string, root common.Hash, marker []byte) { type generatorContext struct { stats *generatorStats // Generation statistic collection db ethdb.KeyValueStore // Key-value store containing the snapshot data - account *snapIter // Iterator of account snapshot data - storage *snapIter // Iterator of storage snapshot data + account *holdableIterator // Iterator of account snapshot data + storage *holdableIterator // Iterator of storage snapshot data batch ethdb.Batch // Database batch for writing batch data atomically logged time.Time // The timestamp when last generation progress was displayed } @@ -113,15 +113,15 @@ func newGeneratorContext(stats *generatorStats, db ethdb.KeyValueStore, accMarke func (ctx *generatorContext) openIterator(kind string, start []byte) { if kind == snapAccount { iter := ctx.db.NewIterator(rawdb.SnapshotAccountPrefix, start) - ctx.account = newSnapIter(rawdb.NewKeyLengthIterator(iter, 1+common.HashLength)) + ctx.account = newHoldableIterator(rawdb.NewKeyLengthIterator(iter, 1+common.HashLength)) return } iter := ctx.db.NewIterator(rawdb.SnapshotStoragePrefix, start) - ctx.storage = newSnapIter(rawdb.NewKeyLengthIterator(iter, 1+2*common.HashLength)) + ctx.storage = newHoldableIterator(rawdb.NewKeyLengthIterator(iter, 1+2*common.HashLength)) } // reopenIterators releases the held two global database iterators and -// reopens them in the interruption position. It's aim for not blocking +// reopens them at the current position. It's aimed for not blocking // leveldb compaction. func (ctx *generatorContext) reopenIterators() { for i, iter := range []ethdb.Iterator{ctx.account, ctx.storage} { @@ -145,18 +145,17 @@ func (ctx *generatorContext) close() { } // iterator returns the corresponding iterator specified by the kind. -func (ctx *generatorContext) iterator(kind string) *snapIter { +func (ctx *generatorContext) iterator(kind string) *holdableIterator { if kind == snapAccount { return ctx.account } return ctx.storage } -// removeStorageBefore, iterates and deletes all storage snapshots starting -// from the current iterator position until the specified account. When the -// iterator touches the storage located in the given account range, or the -// storage is larger than the given account range, it stops and moves back -// the iterator a step. +// removeStorageBefore deletes all storage entries which are located before +// the specified account. When the iterator touches the storage entry which +// is located in or outside the given account, it stops and holds the current +// iterated element locally. func (ctx *generatorContext) removeStorageBefore(account common.Hash) { var ( count uint64 @@ -166,21 +165,20 @@ func (ctx *generatorContext) removeStorageBefore(account common.Hash) { for iter.Next() { key := iter.Key() if bytes.Compare(key[1:1+common.HashLength], account.Bytes()) >= 0 { - iter.Discard() + iter.Hold() break } ctx.batch.Delete(key) - count += 1 + count++ } ctx.stats.dangling += count snapStorageCleanCounter.Inc(time.Since(start).Nanoseconds()) } -// removeStorageAt iterates and deletes all storage snapshots which are located -// in the specified account range. When the iterator touches the storage which -// is larger than the given account range, it stops and moves back the iterator -// a step. An error will be returned if the initial position of iterator is not -// in the given account range. +// removeStorageAt deletes all storage entries which are located in the specified +// account. When the iterator touches the storage entry which is outside the given +// account, it stops and holds the current iterated element locally. An error will +// be returned if the initial position of iterator is not in the given account. func (ctx *generatorContext) removeStorageAt(account common.Hash) error { var ( count int64 @@ -194,19 +192,19 @@ func (ctx *generatorContext) removeStorageAt(account common.Hash) error { return errors.New("invalid iterator position") } if cmp > 0 { - iter.Discard() + iter.Hold() break } ctx.batch.Delete(key) - count += 1 + count++ } snapWipedStorageMeter.Mark(count) snapStorageCleanCounter.Inc(time.Since(start).Nanoseconds()) return nil } -// removeStorageLeft starting from the current iterator position, iterate and -// delete all storage snapshots left. +// removeStorageLeft deletes all storage entries which are located after +// the current iterator position. func (ctx *generatorContext) removeStorageLeft() { var ( count uint64 @@ -215,7 +213,7 @@ func (ctx *generatorContext) removeStorageLeft() { ) for iter.Next() { ctx.batch.Delete(iter.Key()) - count += 1 + count++ } ctx.stats.dangling += count snapDanglingStorageMeter.Mark(int64(count)) diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 88306e16a72d..9e64777b6a6e 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -187,14 +187,14 @@ func (dl *diskLayer) proveRange(ctx *generatorContext, root common.Hash, prefix // Move the iterator a step back since we iterate one extra element // out. if !bytes.Equal(key[:len(prefix)], prefix) { - iter.Discard() + iter.Hold() break } // Break if we've reached the max size, and signal that we're not // done yet. Move the iterator a step back since we iterate one // extra element out. if len(keys) == max { - iter.Discard() + iter.Hold() diskMore = true break } diff --git a/core/state/snapshot/snap_iterator.go b/core/state/snapshot/holdable_iterator.go similarity index 64% rename from core/state/snapshot/snap_iterator.go rename to core/state/snapshot/holdable_iterator.go index 790b40d3d5c7..c3ce4d6fc6dd 100644 --- a/core/state/snapshot/snap_iterator.go +++ b/core/state/snapshot/holdable_iterator.go @@ -21,55 +21,56 @@ import ( "github.com/ethereum/go-ethereum/ethdb" ) -// snapIter is a wrapper of underlying database iterator. It extends -// the basic iteration interface by adding Discard which can cache -// the element internally where the iterator is currently located. -type snapIter struct { - it ethdb.Iterator - key []byte - val []byte - checkLast bool +// holdableIterator is a wrapper of underlying database iterator. It extends +// the basic iterator interface by adding Hold which can hold the element +// locally where the iterator is currently located and serve it up next time. +type holdableIterator struct { + it ethdb.Iterator + key []byte + val []byte + atHeld bool } -// newSnapIter initializes the snapshot iterator. -func newSnapIter(it ethdb.Iterator) *snapIter { - return &snapIter{it: it} +// newHoldableIterator initializes the holdableIterator with the given iterator. +func newHoldableIterator(it ethdb.Iterator) *holdableIterator { + return &holdableIterator{it: it} } -// Discard stores the element internally where the iterator is currently located. -func (it *snapIter) Discard() { +// Hold holds the element locally where the iterator is currently located which +// can be served up next time. +func (it *holdableIterator) Hold() { if it.it.Key() == nil { - return // nothing to cache + return // nothing to hold } it.key = common.CopyBytes(it.it.Key()) it.val = common.CopyBytes(it.it.Value()) - it.checkLast = false + it.atHeld = false } // Next moves the iterator to the next key/value pair. It returns whether the // iterator is exhausted. -func (it *snapIter) Next() bool { - if !it.checkLast && it.key != nil { - it.checkLast = true - } else if it.checkLast { - it.checkLast = false +func (it *holdableIterator) Next() bool { + if !it.atHeld && it.key != nil { + it.atHeld = true + } else if it.atHeld { + it.atHeld = false it.key = nil it.val = nil } if it.key != nil { - return true // shift to discarded value + return true // shifted to locally held value } return it.it.Next() } // Error returns any accumulated error. Exhausting all the key/value pairs // is not considered to be an error. -func (it *snapIter) Error() error { return it.it.Error() } +func (it *holdableIterator) Error() error { return it.it.Error() } // Release releases associated resources. Release should always succeed and can // be called multiple times without causing error. -func (it *snapIter) Release() { - it.checkLast = false +func (it *holdableIterator) Release() { + it.atHeld = false it.key = nil it.val = nil it.it.Release() @@ -78,7 +79,7 @@ func (it *snapIter) Release() { // Key returns the key of the current key/value pair, or nil if done. The caller // should not modify the contents of the returned slice, and its contents may // change on the next call to Next. -func (it *snapIter) Key() []byte { +func (it *holdableIterator) Key() []byte { if it.key != nil { return it.key } @@ -88,7 +89,7 @@ func (it *snapIter) Key() []byte { // Value returns the value of the current key/value pair, or nil if done. The // caller should not modify the contents of the returned slice, and its contents // may change on the next call to Next. -func (it *snapIter) Value() []byte { +func (it *holdableIterator) Value() []byte { if it.val != nil { return it.val } diff --git a/core/state/snapshot/snap_iterator_test.go b/core/state/snapshot/holdable_iterator_test.go similarity index 96% rename from core/state/snapshot/snap_iterator_test.go rename to core/state/snapshot/holdable_iterator_test.go index 1887bcd5ada5..bd149bd64d46 100644 --- a/core/state/snapshot/snap_iterator_test.go +++ b/core/state/snapshot/holdable_iterator_test.go @@ -36,10 +36,10 @@ func TestIteratorDiscard(t *testing.T) { } } // Iterate over the database with the given configs and verify the results - it, idx := newSnapIter(db.NewIterator(nil, nil)), 0 + it, idx := newHoldableIterator(db.NewIterator(nil, nil)), 0 // Nothing should be affected for calling Discard on non-initialized iterator - it.Discard() + it.Hold() for it.Next() { if len(content) <= idx { @@ -53,8 +53,8 @@ func TestIteratorDiscard(t *testing.T) { t.Errorf("item %d: value mismatch: have %s, want %s", idx, string(it.Value()), content[order[idx]]) } // Should be safe to call discard multiple times - it.Discard() - it.Discard() + it.Hold() + it.Hold() // Shift iterator to the discarded element it.Next() @@ -66,7 +66,7 @@ func TestIteratorDiscard(t *testing.T) { } // Discard/Next combo should work always - it.Discard() + it.Hold() it.Next() if !bytes.Equal(it.Key(), []byte(order[idx])) { t.Errorf("item %d: key mismatch: have %s, want %s", idx, string(it.Key()), order[idx]) From 61dcb92bb3d0c4e4903edc5ee8b3867ac23d66de Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Tue, 10 May 2022 15:18:09 +0800 Subject: [PATCH 28/30] core/state/snapshot: reopen the iterator at the next position --- core/state/snapshot/context.go | 36 ++++----- core/state/snapshot/generate.go | 12 ++- core/state/snapshot/holdable_iterator_test.go | 79 ++++++++++++++++++- 3 files changed, 106 insertions(+), 21 deletions(-) diff --git a/core/state/snapshot/context.go b/core/state/snapshot/context.go index 645491f1b89d..0089c6dd03e8 100644 --- a/core/state/snapshot/context.go +++ b/core/state/snapshot/context.go @@ -120,22 +120,22 @@ func (ctx *generatorContext) openIterator(kind string, start []byte) { ctx.storage = newHoldableIterator(rawdb.NewKeyLengthIterator(iter, 1+2*common.HashLength)) } -// reopenIterators releases the held two global database iterators and -// reopens them at the current position. It's aimed for not blocking -// leveldb compaction. -func (ctx *generatorContext) reopenIterators() { - for i, iter := range []ethdb.Iterator{ctx.account, ctx.storage} { - key := iter.Key() - if len(key) == 0 { // nil or []byte{} - continue // the iterator may already be exhausted - } - kind := snapAccount - if i == 1 { - kind = snapStorage - } - iter.Release() - ctx.openIterator(kind, key[1:]) +// reopenIterator releases the specified snapshot iterator and re-open it +// in the next position. It's aimed for not blocking leveldb compaction. +func (ctx *generatorContext) reopenIterator(kind string) { + // Shift iterator one more step, so that we can reopen + // the iterator at the right position. + var iter = ctx.account + if kind == snapStorage { + iter = ctx.storage + } + hasNext := iter.Next() + if !hasNext { + return // iterator is exhausted now } + nextKey := iter.Key() + iter.Release() + ctx.openIterator(kind, nextKey[1:]) } // close releases all the held resources. @@ -168,7 +168,7 @@ func (ctx *generatorContext) removeStorageBefore(account common.Hash) { iter.Hold() break } - ctx.batch.Delete(key) + ctx.batch.Delete(key) // TODO(rjl493456442) avoid accumulating too much data count++ } ctx.stats.dangling += count @@ -195,7 +195,7 @@ func (ctx *generatorContext) removeStorageAt(account common.Hash) error { iter.Hold() break } - ctx.batch.Delete(key) + ctx.batch.Delete(key) // TODO(rjl493456442) avoid accumulating too much data count++ } snapWipedStorageMeter.Mark(count) @@ -212,7 +212,7 @@ func (ctx *generatorContext) removeStorageLeft() { iter = ctx.storage ) for iter.Next() { - ctx.batch.Delete(iter.Key()) + ctx.batch.Delete(iter.Key()) // TODO(rjl493456442) avoid accumulating too much data count++ } ctx.stats.dangling += count diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 9e64777b6a6e..4412f987eb04 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -496,7 +496,8 @@ func (dl *diskLayer) checkAndFlush(ctx *generatorContext, current []byte) error return newAbortErr(abort) // bubble up an error for interruption } // Don't hold the iterators too long, release them to let compactor works - ctx.reopenIterators() + ctx.reopenIterator(snapAccount) + ctx.reopenIterator(snapStorage) } if time.Since(ctx.logged) > 8*time.Second { ctx.stats.Log("Generating state snapshot", dl.root, current) @@ -667,7 +668,14 @@ func (dl *diskLayer) generate(stats *generatorStats) { } stats.Log("Resuming state snapshot generation", dl.root, dl.genMarker) - // Initialize the global generator context + // Initialize the global generator context. The snapshot iterators are + // opened at the interrupted position because the assumption is held + // that all the snapshot data are generated correctly before the marker. + // Even if the snapshot data is updated during the interruption (before + // or at the marker), the assumption is still held. + // For the account or storage slot at the interruption, they will be + // processed twice by the generator(they are already processed in the + // last run) but it's fine. ctx := newGeneratorContext(stats, dl.diskdb, accMarker, dl.genMarker) defer ctx.close() diff --git a/core/state/snapshot/holdable_iterator_test.go b/core/state/snapshot/holdable_iterator_test.go index bd149bd64d46..397dbf103796 100644 --- a/core/state/snapshot/holdable_iterator_test.go +++ b/core/state/snapshot/holdable_iterator_test.go @@ -20,10 +20,11 @@ import ( "bytes" "testing" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/rawdb" ) -func TestIteratorDiscard(t *testing.T) { +func TestIteratorHold(t *testing.T) { // Create the key-value data store var ( content = map[string]string{"k1": "v1", "k2": "v2", "k3": "v3"} @@ -84,3 +85,79 @@ func TestIteratorDiscard(t *testing.T) { } db.Close() } + +func TestReopenIterator(t *testing.T) { + var ( + content = map[common.Hash]string{ + common.HexToHash("a1"): "v1", + common.HexToHash("a2"): "v2", + common.HexToHash("a3"): "v3", + common.HexToHash("a4"): "v4", + common.HexToHash("a5"): "v5", + common.HexToHash("a6"): "v6", + } + order = []common.Hash{ + common.HexToHash("a1"), + common.HexToHash("a2"), + common.HexToHash("a3"), + common.HexToHash("a4"), + common.HexToHash("a5"), + common.HexToHash("a6"), + } + db = rawdb.NewMemoryDatabase() + ) + for key, val := range content { + rawdb.WriteAccountSnapshot(db, key, []byte(val)) + } + checkVal := func(it *holdableIterator, index int) { + if !bytes.Equal(it.Key(), append(rawdb.SnapshotAccountPrefix, order[index].Bytes()...)) { + t.Fatalf("Unexpected data entry key, want %v got %v", order[index], it.Key()) + } + if !bytes.Equal(it.Value(), []byte(content[order[index]])) { + t.Fatalf("Unexpected data entry key, want %v got %v", []byte(content[order[index]]), it.Value()) + } + } + // Iterate over the database with the given configs and verify the results + ctx, idx := newGeneratorContext(&generatorStats{}, db, nil, nil), -1 + + idx++ + ctx.account.Next() + checkVal(ctx.account, idx) + + ctx.reopenIterator(snapAccount) + idx++ + ctx.account.Next() + checkVal(ctx.account, idx) + + // reopen twice + ctx.reopenIterator(snapAccount) + ctx.reopenIterator(snapAccount) + idx++ + ctx.account.Next() + checkVal(ctx.account, idx) + + // reopen iterator with held value + ctx.account.Next() + ctx.account.Hold() + ctx.reopenIterator(snapAccount) + idx++ + ctx.account.Next() + checkVal(ctx.account, idx) + + // reopen twice iterator with held value + ctx.account.Next() + ctx.account.Hold() + ctx.reopenIterator(snapAccount) + ctx.reopenIterator(snapAccount) + idx++ + ctx.account.Next() + checkVal(ctx.account, idx) + + // shift to the end and reopen + ctx.account.Next() // the end + ctx.reopenIterator(snapAccount) + ctx.account.Next() + if ctx.account.Key() != nil { + t.Fatal("Unexpected iterated entry") + } +} From c80a059633ddd06aa0e4c35729b49026860c7bea Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Tue, 10 May 2022 19:17:33 +0800 Subject: [PATCH 29/30] ethdb, core/state/snapshot: address comment from peter --- core/state/snapshot/context.go | 22 +++++++++++++++++----- core/state/snapshot/generate.go | 26 +++++++++++++------------- ethdb/memorydb/memorydb.go | 8 ++------ 3 files changed, 32 insertions(+), 24 deletions(-) diff --git a/core/state/snapshot/context.go b/core/state/snapshot/context.go index 0089c6dd03e8..9a50e075b999 100644 --- a/core/state/snapshot/context.go +++ b/core/state/snapshot/context.go @@ -133,9 +133,9 @@ func (ctx *generatorContext) reopenIterator(kind string) { if !hasNext { return // iterator is exhausted now } - nextKey := iter.Key() + next := iter.Key() iter.Release() - ctx.openIterator(kind, nextKey[1:]) + ctx.openIterator(kind, next[1:]) } // close releases all the held resources. @@ -168,8 +168,12 @@ func (ctx *generatorContext) removeStorageBefore(account common.Hash) { iter.Hold() break } - ctx.batch.Delete(key) // TODO(rjl493456442) avoid accumulating too much data count++ + ctx.batch.Delete(key) + if ctx.batch.ValueSize() > ethdb.IdealBatchSize { + ctx.batch.Write() + ctx.batch.Reset() + } } ctx.stats.dangling += count snapStorageCleanCounter.Inc(time.Since(start).Nanoseconds()) @@ -195,8 +199,12 @@ func (ctx *generatorContext) removeStorageAt(account common.Hash) error { iter.Hold() break } - ctx.batch.Delete(key) // TODO(rjl493456442) avoid accumulating too much data count++ + ctx.batch.Delete(key) + if ctx.batch.ValueSize() > ethdb.IdealBatchSize { + ctx.batch.Write() + ctx.batch.Reset() + } } snapWipedStorageMeter.Mark(count) snapStorageCleanCounter.Inc(time.Since(start).Nanoseconds()) @@ -212,8 +220,12 @@ func (ctx *generatorContext) removeStorageLeft() { iter = ctx.storage ) for iter.Next() { - ctx.batch.Delete(iter.Key()) // TODO(rjl493456442) avoid accumulating too much data count++ + ctx.batch.Delete(iter.Key()) + if ctx.batch.ValueSize() > ethdb.IdealBatchSize { + ctx.batch.Write() + ctx.batch.Reset() + } } ctx.stats.dangling += count snapDanglingStorageMeter.Mark(int64(count)) diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 4412f987eb04..769989aec21c 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -557,17 +557,17 @@ func generateStorages(ctx *generatorContext, dl *diskLayer, account common.Hash, // from the given origin position. func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) error { onAccount := func(key []byte, val []byte, write bool, delete bool) error { - // Make sure to clear all dangling storages before processing this account - accountHash := common.BytesToHash(key) - ctx.removeStorageBefore(accountHash) + // Make sure to clear all dangling storages before this account + account := common.BytesToHash(key) + ctx.removeStorageBefore(account) start := time.Now() if delete { - rawdb.DeleteAccountSnapshot(ctx.batch, accountHash) + rawdb.DeleteAccountSnapshot(ctx.batch, account) snapWipedAccountMeter.Mark(1) snapAccountWriteCounter.Inc(time.Since(start).Nanoseconds()) - ctx.removeStorageAt(accountHash) + ctx.removeStorageAt(account) return nil } // Retrieve the current account and flatten it into the internal format @@ -581,7 +581,7 @@ func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) er log.Crit("Invalid account encountered during snapshot creation", "err", err) } // If the account is not yet in-progress, write it out - if accMarker == nil || !bytes.Equal(accountHash[:], accMarker) { + if accMarker == nil || !bytes.Equal(account[:], accMarker) { dataLen := len(val) // Approximate size, saves us a round of RLP-encoding if !write { if bytes.Equal(acc.CodeHash, emptyCode[:]) { @@ -594,17 +594,15 @@ func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) er } else { data := SlimAccountRLP(acc.Nonce, acc.Balance, acc.Root, acc.CodeHash) dataLen = len(data) - rawdb.WriteAccountSnapshot(ctx.batch, accountHash, data) + rawdb.WriteAccountSnapshot(ctx.batch, account, data) snapGeneratedAccountMeter.Mark(1) } ctx.stats.storage += common.StorageSize(1 + common.HashLength + dataLen) ctx.stats.accounts++ } - snapAccountWriteCounter.Inc(time.Since(start).Nanoseconds()) - // If the snap generation goes here after interrupted, genMarker may go backward // when last genMarker is consisted of accountHash and storageHash - marker := accountHash[:] + marker := account[:] if accMarker != nil && bytes.Equal(marker, accMarker) && len(dl.genMarker) > common.HashLength { marker = dl.genMarker[:] } @@ -612,16 +610,18 @@ func generateAccounts(ctx *generatorContext, dl *diskLayer, accMarker []byte) er if err := dl.checkAndFlush(ctx, marker); err != nil { return err } + snapAccountWriteCounter.Inc(time.Since(start).Nanoseconds()) // let's count flush time as well + // If the iterated account is the contract, create a further loop to // verify or regenerate the contract storage. if acc.Root == emptyRoot { - ctx.removeStorageAt(accountHash) + ctx.removeStorageAt(account) } else { var storeMarker []byte - if accMarker != nil && bytes.Equal(accountHash[:], accMarker) && len(dl.genMarker) > common.HashLength { + if accMarker != nil && bytes.Equal(account[:], accMarker) && len(dl.genMarker) > common.HashLength { storeMarker = dl.genMarker[common.HashLength:] } - if err := generateStorages(ctx, dl, accountHash, acc.Root, storeMarker); err != nil { + if err := generateStorages(ctx, dl, account, acc.Root, storeMarker); err != nil { return err } } diff --git a/ethdb/memorydb/memorydb.go b/ethdb/memorydb/memorydb.go index 378ef7413600..e94570cb3f0e 100644 --- a/ethdb/memorydb/memorydb.go +++ b/ethdb/memorydb/memorydb.go @@ -288,16 +288,12 @@ type iterator struct { // Next moves the iterator to the next key/value pair. It returns whether the // iterator is exhausted. func (it *iterator) Next() bool { - // Short circuit if iterator has nothing to iterate. - if len(it.keys) == 0 { - return false - } // Short circuit if iterator is already exhausted in the forward direction. - if it.index == len(it.keys) { + if it.index >= len(it.keys) { return false } it.index += 1 - return it.index != len(it.keys) + return it.index < len(it.keys) } // Error returns any accumulated error. Exhausting all the key/value pairs From 25b03928af3a6e9c4bbf1bb12dc1526253f733ce Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Mon, 23 May 2022 17:32:38 +0800 Subject: [PATCH 30/30] core/state/snapshot: reopen exhausted iterators --- core/state/snapshot/context.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/core/state/snapshot/context.go b/core/state/snapshot/context.go index 9a50e075b999..67d7e41a03ca 100644 --- a/core/state/snapshot/context.go +++ b/core/state/snapshot/context.go @@ -26,6 +26,7 @@ import ( "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/ethdb" + "github.com/ethereum/go-ethereum/ethdb/memorydb" "github.com/ethereum/go-ethereum/log" ) @@ -131,7 +132,14 @@ func (ctx *generatorContext) reopenIterator(kind string) { } hasNext := iter.Next() if !hasNext { - return // iterator is exhausted now + // Iterator exhausted, release forever and create an already exhausted virtual iterator + iter.Release() + if kind == snapAccount { + ctx.account = newHoldableIterator(memorydb.New().NewIterator(nil, nil)) + return + } + ctx.storage = newHoldableIterator(memorydb.New().NewIterator(nil, nil)) + return } next := iter.Key() iter.Release()