State trie garbage collection #15903
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,7 @@ import ( | |
| "github.com/ethereum/go-ethereum/common" | ||
| "github.com/ethereum/go-ethereum/common/mclock" | ||
| "github.com/ethereum/go-ethereum/consensus" | ||
| "github.com/ethereum/go-ethereum/core/hashtree" | ||
| "github.com/ethereum/go-ethereum/core/state" | ||
| "github.com/ethereum/go-ethereum/core/types" | ||
| "github.com/ethereum/go-ethereum/core/vm" | ||
|
|
@@ -105,14 +106,18 @@ type BlockChain struct { | |
| quit chan struct{} // blockchain quit channel | ||
| running int32 // running must be called atomically | ||
| // procInterrupt must be atomically called | ||
| processing int32 | ||
| procInterrupt int32 // interrupt signaler for block processing | ||
| wg sync.WaitGroup // chain processing wait group for shutting down | ||
| writeCounter uint64 | ||
|
|
||
| engine consensus.Engine | ||
| processor Processor // block processor interface | ||
| validator Validator // block and state validator interface | ||
| vmConfig vm.Config | ||
|
|
||
| gc *hashtree.GarbageCollector | ||
|
|
||
| badBlocks *lru.Cache // Bad block cache | ||
| } | ||
|
|
||
|
|
@@ -126,6 +131,7 @@ func NewBlockChain(chainDb ethdb.Database, config *params.ChainConfig, engine co | |
| futureBlocks, _ := lru.New(maxFutureBlocks) | ||
| badBlocks, _ := lru.New(badBlockLimit) | ||
|
|
||
| //hashtree.Print(chainDb, []byte(state.DbPrefix)) | ||
| bc := &BlockChain{ | ||
| config: config, | ||
| chainDb: chainDb, | ||
|
|
@@ -167,11 +173,29 @@ func NewBlockChain(chainDb ethdb.Database, config *params.ChainConfig, engine co | |
| } | ||
| } | ||
| } | ||
|
|
||
| bc.gc = hashtree.NewGarbageCollector(chainDb, []byte(state.DbPrefix), bc.hasDataCallback) | ||
|
|
||
| /*headBlock := bc.currentBlock.NumberU64() | ||
| if headBlock > 1000 { | ||
| bc.gc.FullGC(headBlock - 1000) | ||
| }*/ | ||
|
|
||
| bc.gc.BackgroundGC(bc.CurrentBlock, &bc.processing, &bc.procInterrupt, &bc.wg) | ||
|
|
||
| // Take ownership of this particular state | ||
| go bc.update() | ||
| return bc, nil | ||
| } | ||
|
|
||
| func (bc *BlockChain) hasDataCallback(version uint64) func(position, hash []byte) bool { | ||
| header := bc.GetHeaderByNumber(version) | ||
| if header == nil { | ||
| return nil | ||
| } | ||
| return state.HasDataCallback(header.Root, bc.chainDb) | ||
| } | ||
|
|
||
| func (bc *BlockChain) getProcInterrupt() bool { | ||
| return atomic.LoadInt32(&bc.procInterrupt) == 1 | ||
| } | ||
|
|
@@ -292,7 +316,7 @@ func (bc *BlockChain) FastSyncCommitHead(hash common.Hash) error { | |
| if block == nil { | ||
| return fmt.Errorf("non existent block [%x…]", hash[:4]) | ||
| } | ||
| if _, err := trie.NewSecure(block.Root(), hashtree.NewReader(bc.chainDb, state.DbPrefix), 0); err != nil { | ||
| return err | ||
| } | ||
| // If all checks out, manually set the head block | ||
|
|
@@ -808,7 +832,7 @@ func (bc *BlockChain) WriteBlockAndState(block *types.Block, receipts []*types.R | |
| if err := WriteBlock(batch, block); err != nil { | ||
| return NonStatTy, err | ||
| } | ||
| if _, err := state.CommitTo(batch, block.NumberU64(), bc.gc, bc.config.IsEIP158(block.Number())); err != nil { | ||
| return NonStatTy, err | ||
| } | ||
| if err := WriteBlockReceipts(batch, block.Hash(), block.NumberU64(), receipts); err != nil { | ||
|
|
@@ -842,9 +866,13 @@ func (bc *BlockChain) WriteBlockAndState(block *types.Block, receipts []*types.R | |
| } else { | ||
| status = SideStatTy | ||
| } | ||
|
|
||
| bc.gc.LockWrite() | ||
| if err := batch.Write(); err != nil { | ||
| bc.gc.UnlockWrite() | ||
| return NonStatTy, err | ||
| } | ||
| bc.gc.UnlockWrite() | ||
|
There was a problem hiding this comment. This is a bit playing with fire; holding on to one mutex (bc.mu) while obtaining another mutex. Could lead to race conditions. For example, the There was a problem hiding this comment. You mean deadlock? It cannot cause a deadlock because the only thing we do under this lock is write or delete state data. Chain mutex is never used while holding this one. Still, I know that using such a lock in Blockchain is critical, but so is the GC. If we keep using it like this, we should document it very well what it does and why it does that. We should somehow avoid deleting trie nodes that reappeared just when GC removed the old entry. I am open to other suggestions though. Note: my original proposal had an inherently safe db structure: |
||
|
|
||
| // Set new head. | ||
| if status == CanonStatTy { | ||
|
|
@@ -888,6 +916,9 @@ func (bc *BlockChain) insertChain(chain types.Blocks) (int, []interface{}, []*ty | |
| bc.chainmu.Lock() | ||
| defer bc.chainmu.Unlock() | ||
|
|
||
| atomic.StoreInt32(&bc.processing, 1) | ||
| defer atomic.StoreInt32(&bc.processing, 0) | ||
|
|
||
| // A queued approach to delivering events. This is generally | ||
| // faster than direct delivery and requires much less mutex | ||
| // acquiring. | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I don't yet understand the full context of how this method is used, I do find it a bit odd that
numberis used to resolve a header, since anumbercan be ambiguous.So what I'm wondering is if whatever uses this method, does it handle reorgs without breaking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely not. This function verifies a piece of data (identified by hash) being present at the given position in a certain block. This block is the "GC block" which is the oldest block whose state we still want to remember. This block is also the earliest one where we can roll back to. If a longer reorg happens, we should resync the entire chain from the beginning (or do a fast sync). Handling this corner case is not implemented yet.