-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: fix multi-db env #2755
fix: fix multi-db env #2755
Conversation
@@ -210,7 +210,11 @@ func New(stack *node.Node, config *ethconfig.Config) (*Ethereum, error) { | |||
} | |||
|
|||
// startup ancient freeze | |||
if err = chainDb.SetupFreezerEnv(ðdb.FreezerEnv{ | |||
freezeDb := chainDb | |||
if stack.CheckIfMultiDataBase() { |
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.
if stack.CheckIfMultiDataBase() && chainDb.BlockStore() != nil
may be better
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.
It is not appropriate to use chainDb.BlockStore()
for validation because:
- When multi-database mode is enabled,
chainDb.BlockStore()
will return blockDb. - When multi-database mode is disabled,
chainDb.BlockStore()
will return chainDb.
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.
LGTM
@@ -74,6 +74,9 @@ func newChainFreezer(datadir string, namespace string, readonly bool, offset uin | |||
Freezer: freezer, | |||
quit: make(chan struct{}), | |||
trigger: make(chan chan struct{}), | |||
// After enabling pruneAncient, the ancient data is not retained. In some specific scenarios where it is | |||
// necessary to roll back to blocks prior to the finalized block, it is mandatory to keep the most recent 90,000 blocks in the database to ensure proper functionality and rollback capability. | |||
multiDatabase: false, |
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.
Why not remove the multiDatabase field & param? It's useless now.
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.
I'm not entirely sure whether the logic of using the finalized block for processing will be used later. For now, to minimize changes, I disable it.
Description
When multi-database (multi-db) mode is enabled, the freezer's environment (env) is not properly set for the multi-db configuration. This prevents the multi-db chain_freezer from moving block data from the database (db) to the ancient storage (ancient).
Rationale
tell us why we need these changes...
Example
add an example CLI or API response...
Changes
Notable changes: