From 1607adcaaba04c561d3f75a7d8d550d1cc21561c Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Thu, 5 Oct 2023 16:48:17 +0000 Subject: [PATCH 1/7] garbage collect not commited tries --- core/blockchain.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index 79d97d9d0d..85b342cc20 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -1451,7 +1451,8 @@ func (bc *BlockChain) writeBlockWithState(block *types.Block, receipts []*types. // If MaxNumberOfBlocksToSkipStateSaving or MaxAmountOfGasToSkipStateSaving is not zero, then flushing of some blocks will be skipped: // * at most MaxNumberOfBlocksToSkipStateSaving block state commits will be skipped // * sum of gas used in skipped blocks will be at most MaxAmountOfGasToSkipStateSaving - if bc.cacheConfig.TrieDirtyDisabled { + archiveNode := bc.cacheConfig.TrieDirtyDisabled + if archiveNode { var maySkipCommiting, blockLimitReached, gasLimitReached bool if bc.cacheConfig.MaxNumberOfBlocksToSkipStateSaving != 0 { maySkipCommiting = true @@ -1474,10 +1475,10 @@ func (bc *BlockChain) writeBlockWithState(block *types.Block, receipts []*types. bc.amountOfGasInBlocksToSkipStateSaving = bc.cacheConfig.MaxAmountOfGasToSkipStateSaving return bc.triedb.Commit(root, false) } - return nil + // we are skipping saving the trie to diskdb, so we need to keep the trie in memory and garbage collect it later } - // Full but not archive node, do proper garbage collection + // Full node or archive node that's not keeping all states, do proper garbage collection bc.triedb.Reference(root, common.Hash{}) // metadata reference to keep trie alive bc.triegc.Push(trieGcEntry{root, block.Header().Time}, -int64(block.NumberU64())) @@ -1510,7 +1511,8 @@ func (bc *BlockChain) writeBlockWithState(block *types.Block, receipts []*types. } flushInterval := time.Duration(bc.flushInterval.Load()) // If we exceeded out time allowance, flush an entire trie to disk - if bc.gcproc > flushInterval && prevEntry != nil { + // In case of archive node that skips some trie commits we don't flush tries here + if bc.gcproc > flushInterval && prevEntry != nil && !archiveNode { // If the header is missing (canonical chain behind), we're reorging a low // diff sidechain. Suspend committing until this operation is completed. header := bc.GetHeaderByNumber(prevNum) From 2b1eb7e3bcecba01b99d7d0cee8ba869105f7705 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Mon, 9 Oct 2023 17:34:49 +0000 Subject: [PATCH 2/7] lock db lock when accessing dirties map in commit method --- trie/triedb/hashdb/database.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/trie/triedb/hashdb/database.go b/trie/triedb/hashdb/database.go index 8ebcff9f1f..60f18e8151 100644 --- a/trie/triedb/hashdb/database.go +++ b/trie/triedb/hashdb/database.go @@ -460,7 +460,9 @@ func (db *Database) Commit(node common.Hash, report bool) error { // commit is the private locked version of Commit. func (db *Database) commit(hash common.Hash, batch ethdb.Batch, uncacher *cleaner) error { // If the node does not exist, it's a previously committed node + db.lock.RLock() node, ok := db.dirties[hash] + db.lock.RUnlock() if !ok { return nil } From f5e52cb1e34bd0bf76d17477202c2848a27edbd7 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Tue, 10 Oct 2023 17:55:39 +0200 Subject: [PATCH 3/7] add safty checks to BlockChain.Stop --- core/blockchain.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index 85b342cc20..f6cf6814d4 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -1041,13 +1041,13 @@ func (bc *BlockChain) Stop() { for _, offset := range []uint64{0, 1, bc.cacheConfig.TriesInMemory - 1, math.MaxUint64} { if number := bc.CurrentBlock().Number.Uint64(); number > offset { var recent *types.Block - if offset == math.MaxUint { + if offset == math.MaxUint && !bc.triegc.Empty() { _, latest := bc.triegc.Peek() recent = bc.GetBlockByNumber(uint64(-latest)) } else { recent = bc.GetBlockByNumber(number - offset) } - if recent.Root() == (common.Hash{}) { + if recent == nil || recent.Root() == (common.Hash{}) { continue } From f0840474b8f2c629fd1c7b79a891c1a5e0a13c79 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Wed, 11 Oct 2023 01:02:47 +0200 Subject: [PATCH 4/7] fix commiting oldest node from triegc in BlockChain.Stop --- core/blockchain.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index f6cf6814d4..2ccc178de8 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -1039,9 +1039,9 @@ func (bc *BlockChain) Stop() { triedb := bc.triedb for _, offset := range []uint64{0, 1, bc.cacheConfig.TriesInMemory - 1, math.MaxUint64} { - if number := bc.CurrentBlock().Number.Uint64(); number > offset { + if number := bc.CurrentBlock().Number.Uint64(); number > offset || offset == math.MaxUint64 { var recent *types.Block - if offset == math.MaxUint && !bc.triegc.Empty() { + if offset == math.MaxUint64 && !bc.triegc.Empty() { _, latest := bc.triegc.Peek() recent = bc.GetBlockByNumber(uint64(-latest)) } else { From 89540b1440179a851e5e29672a700851c4237d83 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Wed, 11 Oct 2023 18:57:42 +0200 Subject: [PATCH 5/7] add locking aroung db.dirties access --- trie/triedb/hashdb/database.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/trie/triedb/hashdb/database.go b/trie/triedb/hashdb/database.go index 60f18e8151..b93764ae9e 100644 --- a/trie/triedb/hashdb/database.go +++ b/trie/triedb/hashdb/database.go @@ -329,6 +329,7 @@ func (db *Database) Cap(limit common.StorageSize) error { // outside code doesn't see an inconsistent state (referenced data removed from // memory cache during commit but not yet in persistent storage). This is ensured // by only uncaching existing data when the database write finalizes. + db.lock.RLock() nodes, storage, start := len(db.dirties), db.dirtiesSize, time.Now() batch := db.diskdb.NewBatch() @@ -337,12 +338,15 @@ func (db *Database) Cap(limit common.StorageSize) error { // counted. size := db.dirtiesSize + common.StorageSize(len(db.dirties)*cachedNodeSize) size += db.childrenSize + db.lock.RUnlock() // Keep committing nodes from the flush-list until we're below allowance oldest := db.oldest for size > limit && oldest != (common.Hash{}) { // Fetch the oldest referenced node and push into the batch + db.lock.RLock() node := db.dirties[oldest] + db.lock.RUnlock() rawdb.WriteLegacyTrieNode(batch, oldest, node.node) // If we exceeded the ideal batch size, commit and reset @@ -418,7 +422,9 @@ func (db *Database) Commit(node common.Hash, report bool) error { batch := db.diskdb.NewBatch() // Move the trie itself into the batch, flushing if enough data is accumulated + db.lock.RLock() nodes, storage := len(db.dirties), db.dirtiesSize + db.lock.RUnlock() uncacher := &cleaner{db} if err := db.commit(node, batch, uncacher); err != nil { From 3573250adec48511a0bc45647061ed6d25c4ae35 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Wed, 11 Oct 2023 19:09:30 +0200 Subject: [PATCH 6/7] move time.Now outside critical section --- trie/triedb/hashdb/database.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/trie/triedb/hashdb/database.go b/trie/triedb/hashdb/database.go index b93764ae9e..2f4e40111a 100644 --- a/trie/triedb/hashdb/database.go +++ b/trie/triedb/hashdb/database.go @@ -329,8 +329,9 @@ func (db *Database) Cap(limit common.StorageSize) error { // outside code doesn't see an inconsistent state (referenced data removed from // memory cache during commit but not yet in persistent storage). This is ensured // by only uncaching existing data when the database write finalizes. + start := time.Now() db.lock.RLock() - nodes, storage, start := len(db.dirties), db.dirtiesSize, time.Now() + nodes, storage := len(db.dirties), db.dirtiesSize batch := db.diskdb.NewBatch() // db.dirtiesSize only contains the useful data in the cache, but when reporting From 202caf5bf944835ef78b4d7c4ba8b48bdc54d65d Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Wed, 11 Oct 2023 22:34:39 +0200 Subject: [PATCH 7/7] move NewBatch() call outside of critical section --- trie/triedb/hashdb/database.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trie/triedb/hashdb/database.go b/trie/triedb/hashdb/database.go index 2f4e40111a..da1339e07f 100644 --- a/trie/triedb/hashdb/database.go +++ b/trie/triedb/hashdb/database.go @@ -330,9 +330,9 @@ func (db *Database) Cap(limit common.StorageSize) error { // memory cache during commit but not yet in persistent storage). This is ensured // by only uncaching existing data when the database write finalizes. start := time.Now() + batch := db.diskdb.NewBatch() db.lock.RLock() nodes, storage := len(db.dirties), db.dirtiesSize - batch := db.diskdb.NewBatch() // db.dirtiesSize only contains the useful data in the cache, but when reporting // the total memory consumption, the maintenance metadata is also needed to be