From 3853f500824080b17183633fc4dad2e16cd68574 Mon Sep 17 00:00:00 2001 From: rjl493456442 Date: Mon, 23 Oct 2023 21:46:39 +0800 Subject: [PATCH] trie/triedb/pathdb, core/rawdb: enhance error message in freezer (#28198) This PR adds more error message for debugging purpose. --- core/rawdb/freezer_resettable.go | 2 ++ core/rawdb/freezer_table.go | 21 ++++++++++---- trie/triedb/pathdb/history.go | 22 +++++++++++++-- trie/triedb/pathdb/history_test.go | 44 ++++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 7 deletions(-) diff --git a/core/rawdb/freezer_resettable.go b/core/rawdb/freezer_resettable.go index 0a3892bcd..1df6411a3 100644 --- a/core/rawdb/freezer_resettable.go +++ b/core/rawdb/freezer_resettable.go @@ -22,6 +22,7 @@ import ( "sync" "github.com/ethereum/go-ethereum/ethdb" + "github.com/ethereum/go-ethereum/log" ) const tmpSuffix = ".tmp" @@ -224,6 +225,7 @@ func cleanup(path string) error { } for _, name := range names { if name == filepath.Base(path)+tmpSuffix { + log.Info("Removed leftover freezer directory", "name", name) return os.RemoveAll(filepath.Join(parent, name)) } } diff --git a/core/rawdb/freezer_table.go b/core/rawdb/freezer_table.go index cb32d61ae..e3353cc7d 100644 --- a/core/rawdb/freezer_table.go +++ b/core/rawdb/freezer_table.go @@ -257,6 +257,12 @@ func (t *freezerTable) repair() error { t.index.ReadAt(buffer, offsetsSize-indexEntrySize) lastIndex.unmarshalBinary(buffer) } + // Print an error log if the index is corrupted due to an incorrect + // last index item. While it is theoretically possible to have a zero offset + // by storing all zero-size items, it is highly unlikely to occur in practice. + if lastIndex.offset == 0 && offsetsSize%indexEntrySize > 1 { + log.Error("Corrupted index file detected", "lastOffset", lastIndex.offset, "items", offsetsSize%indexEntrySize-1) + } if t.readonly { t.head, err = t.openFile(lastIndex.filenum, openFreezerFileForReadOnly) } else { @@ -349,7 +355,7 @@ func (t *freezerTable) repair() error { return err } if verbose { - t.logger.Info("Chain freezer table opened", "items", t.items.Load(), "size", t.headBytes) + t.logger.Info("Chain freezer table opened", "items", t.items.Load(), "deleted", t.itemOffset.Load(), "hidden", t.itemHidden.Load(), "tailId", t.tailId, "headId", t.headId, "size", t.headBytes) } else { t.logger.Debug("Chain freezer table opened", "items", t.items.Load(), "size", common.StorageSize(t.headBytes)) } @@ -522,6 +528,10 @@ func (t *freezerTable) truncateTail(items uint64) error { if err := t.meta.Sync(); err != nil { return err } + // Close the index file before shorten it. + if err := t.index.Close(); err != nil { + return err + } // Truncate the deleted index entries from the index file. err = copyFrom(t.index.Name(), t.index.Name(), indexEntrySize*(newDeleted-deleted+1), func(f *os.File) error { tailIndex := indexEntry{ @@ -535,13 +545,14 @@ func (t *freezerTable) truncateTail(items uint64) error { return err } // Reopen the modified index file to load the changes - if err := t.index.Close(); err != nil { - return err - } t.index, err = openFreezerFileForAppend(t.index.Name()) if err != nil { return err } + // Sync the file to ensure changes are flushed to disk + if err := t.index.Sync(); err != nil { + return err + } // Release any files before the current tail t.tailId = newTailId t.itemOffset.Store(newDeleted) @@ -774,7 +785,7 @@ func (t *freezerTable) retrieveItems(start, count, maxBytes uint64) ([]byte, []i return fmt.Errorf("missing data file %d", fileId) } if _, err := dataFile.ReadAt(output[len(output)-length:], int64(start)); err != nil { - return err + return fmt.Errorf("%w, fileid: %d, start: %d, length: %d", err, fileId, start, length) } return nil } diff --git a/trie/triedb/pathdb/history.go b/trie/triedb/pathdb/history.go index ce8253250..6f33b6185 100644 --- a/trie/triedb/pathdb/history.go +++ b/trie/triedb/pathdb/history.go @@ -581,7 +581,16 @@ func truncateFromHead(db ethdb.Batcher, freezer *rawdb.ResettableFreezer, nhead if err != nil { return 0, err } - if ohead <= nhead { + otail, err := freezer.Tail() + if err != nil { + return 0, err + } + // Ensure that the truncation target falls within the specified range. + if ohead < nhead || nhead < otail { + return 0, fmt.Errorf("out of range, tail: %d, head: %d, target: %d", otail, ohead, nhead) + } + // Short circuit if nothing to truncate. + if ohead == nhead { return 0, nil } // Load the meta objects in range [nhead+1, ohead] @@ -610,11 +619,20 @@ func truncateFromHead(db ethdb.Batcher, freezer *rawdb.ResettableFreezer, nhead // truncateFromTail removes the extra state histories from the tail with the given // parameters. It returns the number of items removed from the tail. func truncateFromTail(db ethdb.Batcher, freezer *rawdb.ResettableFreezer, ntail uint64) (int, error) { + ohead, err := freezer.Ancients() + if err != nil { + return 0, err + } otail, err := freezer.Tail() if err != nil { return 0, err } - if otail >= ntail { + // Ensure that the truncation target falls within the specified range. + if otail > ntail || ntail > ohead { + return 0, fmt.Errorf("out of range, tail: %d, head: %d, target: %d", otail, ohead, ntail) + } + // Short circuit if nothing to truncate. + if otail == ntail { return 0, nil } // Load the meta objects in range [otail+1, ntail] diff --git a/trie/triedb/pathdb/history_test.go b/trie/triedb/pathdb/history_test.go index 677103e2b..a3257441d 100644 --- a/trie/triedb/pathdb/history_test.go +++ b/trie/triedb/pathdb/history_test.go @@ -224,6 +224,50 @@ func TestTruncateTailHistories(t *testing.T) { } } +func TestTruncateOutOfRange(t *testing.T) { + var ( + hs = makeHistories(10) + db = rawdb.NewMemoryDatabase() + freezer, _ = openFreezer(t.TempDir(), false) + ) + defer freezer.Close() + + for i := 0; i < len(hs); i++ { + accountData, storageData, accountIndex, storageIndex := hs[i].encode() + rawdb.WriteStateHistory(freezer, uint64(i+1), hs[i].meta.encode(), accountIndex, storageIndex, accountData, storageData) + rawdb.WriteStateID(db, hs[i].meta.root, uint64(i+1)) + } + truncateFromTail(db, freezer, uint64(len(hs)/2)) + + // Ensure of-out-range truncations are rejected correctly. + head, _ := freezer.Ancients() + tail, _ := freezer.Tail() + + cases := []struct { + mode int + target uint64 + expErr error + }{ + {0, head, nil}, // nothing to delete + {0, head + 1, fmt.Errorf("out of range, tail: %d, head: %d, target: %d", tail, head, head+1)}, + {0, tail - 1, fmt.Errorf("out of range, tail: %d, head: %d, target: %d", tail, head, tail-1)}, + {1, tail, nil}, // nothing to delete + {1, head + 1, fmt.Errorf("out of range, tail: %d, head: %d, target: %d", tail, head, head+1)}, + {1, tail - 1, fmt.Errorf("out of range, tail: %d, head: %d, target: %d", tail, head, tail-1)}, + } + for _, c := range cases { + var gotErr error + if c.mode == 0 { + _, gotErr = truncateFromHead(db, freezer, c.target) + } else { + _, gotErr = truncateFromTail(db, freezer, c.target) + } + if !reflect.DeepEqual(gotErr, c.expErr) { + t.Errorf("Unexpected error, want: %v, got: %v", c.expErr, gotErr) + } + } +} + // openFreezer initializes the freezer instance for storing state histories. func openFreezer(datadir string, readOnly bool) (*rawdb.ResettableFreezer, error) { return rawdb.NewStateFreezer(datadir, readOnly)