-
Notifications
You must be signed in to change notification settings - Fork 20.1k
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
core, cmd, trie: fix the condition of pathdb initialization #28718
Conversation
79df09b
to
7d22bea
Compare
7d22bea
to
a399105
Compare
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.
Minor nits
Co-authored-by: Martin HS <martin@swende.se>
Co-authored-by: Martin HS <martin@swende.se>
return nil | ||
} | ||
|
||
// removeFolder deletes all files (not folders) inside the directory 'dir' (but not files in subfolders). | ||
func removeFolder(dir string) { |
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.
Perhaps a more apt name?
func removeFolder(dir string) { | |
func removeFilesInFolder(dir string) { |
Assuming this is accepted, can the fix be released ASAP? |
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
…#28718) Original problem was caused by ethereum#28595, where we made it so that as soon as we start to sync, the root of the disk layer is deleted. That is not wrong per se, but another part of the code uses the "presence of the root" as an init-check for the pathdb. And, since the init-check now failed, the code tried to re-initialize it which failed since a sync was already ongoing. The total impact being: after a state-sync has begun, if the node for some reason is is shut down, it will refuse to start up again, with the error message: `Fatal: Failed to register the Ethereum service: waiting for sync.`. This change also modifies how `geth removedb` works, so that the user is prompted for two things: `state data` and `ancient chain`. The former includes both the chaindb aswell as any state history stored in ancients. --------- Co-authored-by: Martin HS <martin@swende.se>
…#28718) Original problem was caused by ethereum#28595, where we made it so that as soon as we start to sync, the root of the disk layer is deleted. That is not wrong per se, but another part of the code uses the "presence of the root" as an init-check for the pathdb. And, since the init-check now failed, the code tried to re-initialize it which failed since a sync was already ongoing. The total impact being: after a state-sync has begun, if the node for some reason is is shut down, it will refuse to start up again, with the error message: `Fatal: Failed to register the Ethereum service: waiting for sync.`. This change also modifies how `geth removedb` works, so that the user is prompted for two things: `state data` and `ancient chain`. The former includes both the chaindb aswell as any state history stored in ancients. --------- Co-authored-by: Martin HS <martin@swende.se>
…#28718) Original problem was caused by ethereum#28595, where we made it so that as soon as we start to sync, the root of the disk layer is deleted. That is not wrong per se, but another part of the code uses the "presence of the root" as an init-check for the pathdb. And, since the init-check now failed, the code tried to re-initialize it which failed since a sync was already ongoing. The total impact being: after a state-sync has begun, if the node for some reason is is shut down, it will refuse to start up again, with the error message: `Fatal: Failed to register the Ethereum service: waiting for sync.`. This change also modifies how `geth removedb` works, so that the user is prompted for two things: `state data` and `ancient chain`. The former includes both the chaindb aswell as any state history stored in ancients. --------- Co-authored-by: Martin HS <martin@swende.se>
Alternative to #28715
Fix for #28713
Original problem was caused by #28595 . In this PR, we made it so that as soon as we start to sync, we delete the root of the disk layer. That is not wrong per se, but, another part of the code uses the "presence of the root" as an init-check for the pathdb. And, since the init-check now failed, it tried to initialize it, which failed since a sync was already ongoing.
Impact: after a state-sync has begun, if the node is restarted, it will refuse to restart, with the error message:
Fatal: Failed to register the Ethereum service: waiting for sync
.