use StateAtBlock and reference states when recreating missing state #277
Conversation
magicxyyz
force-pushed
the
better-recreate-state-for-rpc
branch
from
567e6ce
to
8d5951a
Compare
There already is a system test on nitro side, that runs this partial-archive-node, stops it, restarts it and then checks if If needed, I can add some more specific tests for the changes, also in geth repo. |
magicxyyz
force-pushed
the
better-recreate-state-for-rpc
branch
from
814e540
to
b158011
Compare
As suggested, I added a specific test on nitro side for getting state for RPCs: |
|
I am still doing some testing with nitro-testnode. I have to yet confirm that, but it seems that finalizers are not good solution. |
going with upstream geth and disabling fastcache in StateAtBlock
| @@ -70,7 +80,7 @@ func (eth *Ethereum) hashState(ctx context.Context, block *types.Block, reexec u | |||
| database = state.NewDatabaseWithConfig(eth.chainDb, trie.HashDefaults) | |||
| if statedb, err = state.New(block.Root(), database, nil); err == nil { | |||
| log.Info("Found disk backend for state trie", "root", block.Root(), "number", block.Number()) | |||
| return statedb, func() { database.TrieDB().Close() }, nil | |||
| return statedb, noopReleaser, nil | |||
There was a problem hiding this comment.
why is that changed to noop? where will that temporarye stateDatabase be released?
There was a problem hiding this comment.
-
Referencing here is not needed, as if the state is available it was read from disk.
-
I've removed some of our changes here to minimize the diff to upstream, ia. we went with how upstream solved cleans cache memory leak by just disabling the cleans cache for the temporary dbs (trie.HashDefaults has cleans = 0) => we don't need to close the db here. If there will be need for cleans cache for better performance, we can enable cleans cache - we can do that in separate PR, but if needed I can enable it in this PR.
| @@ -93,7 +103,7 @@ func (eth *Ethereum) hashState(ctx context.Context, block *types.Block, reexec u | |||
| if !readOnly { | |||
| statedb, err = state.New(current.Root(), database, nil) | |||
| if err == nil { | |||
| return statedb, func() { database.TrieDB().Close() }, nil | |||
| return statedb, noopReleaser, nil | |||
There was a problem hiding this comment.
here also if state is available then it was read from disk + same as above we don't need to close the triedb so we can remove the diff.
This PR is addressing following issues:
hashdbdirties cache if at all. It means that the state could potentially be garbage collected in another thread and further operations on state would fail.That is fine for normal archive node that keeps all the state and doesn't use dirties cache. It is also ok for a full node as we don't require it to keep all the state but only some recent, so RPCs are expected to fail when requesting too old state.
However, that is an issue for "hybrid" node that persists only some states and keeps recent states in dirties cache - we expect it to always be able to process RPC for any state (to emulate normal archive node functionality).
AdvanceStateUpToBlock), which we use for some RPCs (e.g.eth_call,etc_getBalance), accumulated changes to state in oneStateDBobject, what may cause some issues i.a. excessive memory usage by caches insideStateDB.This PR:
state.StateDBeth.StateAtBlockwhich is upstream implementation for recreation of state for tracers and should be a more robust solution