Skip to content

pruner: fall back to disk snapshot root when journal is missing #300

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.

Sign up for GitHub

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

Merged
curryxbo merged 3 commits into from
Mar 18, 2026
Merged

pruner: fall back to disk snapshot root when journal is missing #300

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
56ae344
pruner: fall back to disk snapshot root when journal is missing
Mar 18, 2026
185463c
pruner: fix Cap panic on disk-layer-only tree and add generation wait…
Mar 18, 2026
83b1bd5
pruner: rename diskRoot to snapDiskRoot to avoid confusion with diskS…
Mar 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 41 additions & 12 deletions core/state/pruner/pruner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ type Pruner struct {
trieCachePath string
headHeader *types.Header
snaptree *snapshot.Tree
// diskRoot is set when the snapshot journal was missing and we fell back
// to the persisted disk snapshot root. Prune() uses it as the pruning
// target when no explicit root is provided.
diskRoot common.Hash
}

// NewPruner creates the pruner instance.
Expand All @@ -90,8 +94,22 @@ func NewPruner(db ethdb.Database, datadir, trieCachePath string, bloomSize uint6
return nil, errors.New("Failed to load head block")
}
snaptree, err := snapshot.New(db, trie.NewDatabase(db), 256, headBlock.Root(), false, false, false)
var diskRoot common.Hash
if err != nil {
return nil, err // The relevant snapshot(s) might not exist
// The snapshot journal may be missing because geth was not shut down
// cleanly (SIGKILL before BlockChain.Stop could write the journal).
// Fall back: initialise the snapshot tree with the persisted disk
// snapshot root so that Prune() can still target that state.
diskRoot = rawdb.ReadSnapshotRoot(db)
if diskRoot == (common.Hash{}) {
return nil, err // No snapshot at all — nothing we can do.
}
log.Warn("Snapshot journal missing, falling back to disk snapshot root",
"diskRoot", diskRoot, "chainHead", headBlock.Root())
snaptree, err = snapshot.New(db, trie.NewDatabase(db), 256, diskRoot, false, false, false)
if err != nil {
return nil, err
}
}
Comment on lines 98 to 119

@coderabbitai coderabbitai Bot Mar 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don’t treat every snapshot init failure as “missing journal”.

At Line 98, fallback is triggered for any snapshot.New(...) error. That can hide unrelated snapshot failures and route pruning through a stale/incorrect recovery path. Gate fallback to the journal-missing case only, and log the original init error.

Suggested hardening 
 snaptree, err := snapshot.New(db, trie.NewDatabase(db), 256, headBlock.Root(), false, false, false)
 var snapDiskRoot common.Hash
 if err != nil {
+	// Only fallback when journal is missing; keep other init errors fatal.
+	// Prefer a typed/sentinel error check if snapshot package exposes one.
+	if !strings.Contains(strings.ToLower(err.Error()), "journal") {
+		return nil, err
+	}
 	snapDiskRoot = rawdb.ReadSnapshotRoot(db)
 	if snapDiskRoot == (common.Hash{}) {
 		return nil, err // No snapshot at all — nothing we can do.
 	}
 	log.Warn("Snapshot journal missing, falling back to snapshot disk-layer root",
-		"snapDiskRoot", snapDiskRoot, "chainHead", headBlock.Root())
+		"snapDiskRoot", snapDiskRoot, "chainHead", headBlock.Root(), "initErr", err)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@core/state/pruner/pruner.go` around lines 98 - 119, The snapshot
initialization currently treats any error from snapshot.New(...) as a missing
journal and falls back to rawdb.ReadSnapshotRoot; change this so the fallback to
reading snapDiskRoot only happens when the original error indicates a missing
journal (detect that specific error or sentinel), otherwise return the original
error from snapshot.New; ensure you capture and log the original init error
(from snapshot.New) with context (e.g. include the error when calling
log.Warn/log.Info), and keep the existing flow that uses rawdb.ReadSnapshotRoot,
snaptree assignment, and Prune() only in the genuine journal-missing branch (use
functions snapshot.New, rawdb.ReadSnapshotRoot, snaptree, headBlock.Root() as
anchors).

@curryxbo curryxbo Mar 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fallback is intentionally broad — in practice, the only scenario where snapshot.New(headBlock.Root()) fails but snapshot.New(snapDiskRoot) succeeds is the journal-missing case. For other failures (e.g. DB corruption), the second snapshot.New call will also fail and the error will surface. No behavior change is needed.

@coderabbitai coderabbitai Bot Mar 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@curryxbo, that's a fair point — the second snapshot.New call acts as a natural filter for non-journal failures (DB corruption, missing trie, etc.), and the error will surface from there.

The one residual risk worth noting: if the first call fails for a reason unrelated to the journal (e.g., a partial/inconsistent state around headBlock.Root()), but snapDiskRoot is an older yet structurally intact root, the second call could succeed and pruning would proceed silently against that stale target rather than surfacing the original error. This is admittedly low-probability in practice.

If you're comfortable accepting that edge case given the simplicity of the current approach, that's a reasonable call. Acknowledged — no change needed.

// Sanitize the bloom filter size if it's too small.
if bloomSize < 256 {
Expand All @@ -109,6 +127,7 @@ func NewPruner(db ethdb.Database, datadir, trieCachePath string, bloomSize uint6
trieCachePath: trieCachePath,
headHeader: headBlock.Header(),
snaptree: snaptree,
diskRoot: diskRoot,
}, nil
}

Expand Down Expand Up @@ -249,18 +268,28 @@ func (p *Pruner) Prune(root common.Hash) error {
// - the probability of this layer being reorg is very low
var layers []snapshot.Snapshot
if root == (common.Hash{}) {
// Retrieve all snapshot layers from the current HEAD.
// In theory there are 128 difflayers + 1 disk layer present,
// so 128 diff layers are expected to be returned.
layers = p.snaptree.Snapshots(p.headHeader.Root, 128, true)
if len(layers) != 128 {
// Reject if the accumulated diff layers are less than 128. It
// means in most of normal cases, there is no associated state
// with bottom-most diff layer.
return fmt.Errorf("snapshot not old enough yet: need %d more blocks", 128-len(layers))
// When the snapshot journal was missing (unclean shutdown), we fell
// back to the persisted disk snapshot root in NewPruner. Use that
// root directly as the pruning target instead of requiring 128 diff
// layers that don't exist.
if p.diskRoot != (common.Hash{}) {
log.Info("Using disk snapshot root as pruning target (journal was missing)",
"diskRoot", p.diskRoot)
root = p.diskRoot
} else {
// Retrieve all snapshot layers from the current HEAD.
// In theory there are 128 difflayers + 1 disk layer present,
// so 128 diff layers are expected to be returned.
layers = p.snaptree.Snapshots(p.headHeader.Root, 128, true)
if len(layers) != 128 {
// Reject if the accumulated diff layers are less than 128. It
// means in most of normal cases, there is no associated state
// with bottom-most diff layer.
return fmt.Errorf("snapshot not old enough yet: need %d more blocks", 128-len(layers))
}
// Use the bottom-most diff layer as the target
root = layers[len(layers)-1].Root()
}
// Use the bottom-most diff layer as the target
root = layers[len(layers)-1].Root()
}
// Ensure the root is really present. The weak assumption
// is the presence of root can indicate the presence of the
Expand Down
Loading