Skip to content

Commit

Permalink
core, cmd, trie: pbss fix release v1.13.6 and v1.13.8 (#618)
Browse files Browse the repository at this point in the history
* trie: remove inconsistent trie nodes during sync in path mode (#28595)

This fixes a database corruption issue that could occur during state healing.
When sync is aborted while certain modifications were already committed, and a reorg occurs, the database would contain incorrect trie nodes stored by path.
These nodes need to detected/deleted in order to obtain a complete and fully correct state after state healing.

---------

Co-authored-by: Felix Lange <[email protected]>

* core, cmd, trie: fix the condition of pathdb initialization (#28718)

Original problem was caused by #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 <[email protected]>

---------

Co-authored-by: rjl493456442 <[email protected]>
Co-authored-by: Felix Lange <[email protected]>
Co-authored-by: Martin HS <[email protected]>
  • Loading branch information
4 people authored and huyngopt1994 committed Nov 21, 2024
1 parent 8b1fa06 commit 935aa9d
Show file tree
Hide file tree
Showing 8 changed files with 350 additions and 130 deletions.
99 changes: 56 additions & 43 deletions cmd/ronin/dbcmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,60 +248,73 @@ WARNING: This is a low-level operation which may cause database corruption!`,
func removeDB(ctx *cli.Context) error {
stack, config := makeConfigNode(ctx)

// Remove the full node state database
path := stack.ResolvePath("chaindata")
if common.FileExist(path) {
confirmAndRemoveDB(path, "full node state database")
} else {
log.Info("Full node state database missing", "path", path)
}
// Remove the full node ancient database
path = config.Eth.DatabaseFreezer
// Resolve folder paths.
var (
rootDir = stack.ResolvePath("chaindata")
ancientDir = config.Eth.DatabaseFreezer
)
switch {
case path == "":
path = filepath.Join(stack.ResolvePath("chaindata"), "ancient")
case !filepath.IsAbs(path):
path = config.Node.ResolvePath(path)
}
if common.FileExist(path) {
confirmAndRemoveDB(path, "full node ancient database")
} else {
log.Info("Full node ancient database missing", "path", path)
}
// Remove the light node database
path = stack.ResolvePath("lightchaindata")
if common.FileExist(path) {
confirmAndRemoveDB(path, "light node database")
} else {
log.Info("Light node database missing", "path", path)
}
case ancientDir == "":
ancientDir = filepath.Join(stack.ResolvePath("chaindata"), "ancient")
case !filepath.IsAbs(ancientDir):
ancientDir = config.Node.ResolvePath(ancientDir)
}
// Delete state data
statePaths := []string{rootDir, filepath.Join(ancientDir, rawdb.StateFreezerName)}
confirmAndRemoveDB(statePaths, "state data")

// Delete ancient chain
chainPaths := []string{filepath.Join(ancientDir, rawdb.ChainFreezerName)}
confirmAndRemoveDB(chainPaths, "ancient chain")
return nil
}

// removeFolder deletes all files (not folders) inside the directory 'dir' (but
// not files in subfolders).
func removeFolder(dir string) {
filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
// If we're at the top level folder, recurse into
if path == dir {
return nil
}
// Delete all the files, but not subfolders
if !info.IsDir() {
os.Remove(path)
return nil
}
return filepath.SkipDir
})
}

// confirmAndRemoveDB prompts the user for a last confirmation and removes the
// folder if accepted.
func confirmAndRemoveDB(database string, kind string) {
confirm, err := prompt.Stdin.PromptConfirm(fmt.Sprintf("Remove %s (%s)?", kind, database))
// list of folders if accepted.
func confirmAndRemoveDB(paths []string, kind string) {
msg := fmt.Sprintf("Location(s) of '%s': \n", kind)
for _, path := range paths {
msg += fmt.Sprintf("\t- %s\n", path)
}
fmt.Println(msg)

confirm, err := prompt.Stdin.PromptConfirm(fmt.Sprintf("Remove '%s'?", kind))
switch {
case err != nil:
utils.Fatalf("%v", err)
case !confirm:
log.Info("Database deletion skipped", "path", database)
log.Info("Database deletion skipped", "kind", kind, "paths", paths)
default:
start := time.Now()
filepath.Walk(database, func(path string, info os.FileInfo, err error) error {
// If we're at the top level folder, recurse into
if path == database {
return nil
var (
deleted []string
start = time.Now()
)
for _, path := range paths {
if common.FileExist(path) {
removeFolder(path)
deleted = append(deleted, path)
} else {
log.Info("Folder is not existent", "path", path)
}
// Delete all the files, but not subfolders
if !info.IsDir() {
os.Remove(path)
return nil
}
return filepath.SkipDir
})
log.Info("Database successfully deleted", "path", database, "elapsed", common.PrettyDuration(time.Since(start)))
}
log.Info("Database successfully deleted", "kind", kind, "paths", deleted, "elapsed", common.PrettyDuration(time.Since(start)))
}
}

Expand Down
8 changes: 4 additions & 4 deletions core/rawdb/ancient_scheme.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,16 @@ var chainFreezerNoSnappy = map[string]bool{

// The list of identifiers of ancient stores. It can split more in the futures.
var (
chainFreezerName = "chain" // the folder name of chain segment ancient store.
stateFreezerName = "state" // the folder name of reverse diff ancient store.
ChainFreezerName = "chain" // the folder name of chain segment ancient store.
StateFreezerName = "state" // the folder name of reverse diff ancient store.
)

// freezers the collections of all builtin freezers.
var freezers = []string{chainFreezerName, stateFreezerName}
var freezers = []string{ChainFreezerName, StateFreezerName}

// NewStateFreezer initializes the freezer for state history.
func NewStateFreezer(ancientDir string, readOnly bool) (*ResettableFreezer, error) {
return NewResettableFreezer(
filepath.Join(ancientDir, stateFreezerName), namespace, readOnly,
filepath.Join(ancientDir, StateFreezerName), namespace, readOnly,
stateHistoryTableSize, stateFreezerNoSnappy)
}
12 changes: 6 additions & 6 deletions core/rawdb/ancient_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,14 @@ func inspectFreezers(db ethdb.Database) ([]freezerInfo, error) {
var infos []freezerInfo
for _, freezer := range freezers {
switch freezer {
case chainFreezerName:
info, err := inspect(chainFreezerName, chainFreezerNoSnappy, db)
case ChainFreezerName:
info, err := inspect(ChainFreezerName, chainFreezerNoSnappy, db)
if err != nil {
return nil, err
}
infos = append(infos, info)

case stateFreezerName:
case StateFreezerName:
if ReadStateScheme(db) != PathScheme {
log.Info("Skip inspecting state freezer", "reason", "state freezer is supported for PathScheme only")
continue
Expand All @@ -104,7 +104,7 @@ func inspectFreezers(db ethdb.Database) ([]freezerInfo, error) {
}
defer f.Close()

info, err := inspect(stateFreezerName, stateFreezerNoSnappy, f)
info, err := inspect(StateFreezerName, stateFreezerNoSnappy, f)
if err != nil {
return nil, err
}
Expand All @@ -128,9 +128,9 @@ func InspectFreezerTable(ancient string, freezerName string, tableName string, s
)

switch freezerName {
case chainFreezerName:
case ChainFreezerName:
path, tables = resolveChainFreezerDir(ancient), chainFreezerNoSnappy
case stateFreezerName:
case StateFreezerName:
path, tables = filepath.Join(ancient, freezerName), stateFreezerNoSnappy
default:
return fmt.Errorf("unknown freezer, supported ones: %v", freezers)
Expand Down
2 changes: 1 addition & 1 deletion core/rawdb/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func resolveChainFreezerDir(ancient string) string {
// - chain freezer is not initialized
// - it's legacy location, chain freezer is present in the root ancient folder

freezer := path.Join(ancient, chainFreezerName)
freezer := path.Join(ancient, ChainFreezerName)
if !common.FileExist(freezer) {
if !common.FileExist(ancient) {
// The entire ancient store is not initialized, still use the sub
Expand Down
6 changes: 5 additions & 1 deletion ethdb/dbtest/testsuite.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,9 +272,13 @@ func TestDatabaseSuite(t *testing.T, New func() ethdb.KeyValueStore) {
b.Put([]byte("5"), nil)
b.Delete([]byte("1"))
b.Put([]byte("6"), nil)
b.Delete([]byte("3"))

b.Delete([]byte("3")) // delete then put
b.Put([]byte("3"), nil)

b.Put([]byte("7"), nil) // put then delete
b.Delete([]byte("7"))

if err := b.Write(); err != nil {
t.Fatal(err)
}
Expand Down
Loading

0 comments on commit 935aa9d

Please sign in to comment.