Skip to content

db: atomic per-key prune deletion, prune consistency assertion - #20503

Closed
mh0lt wants to merge 8 commits into
mainfrom
fix/prune-atomicity-clean
Closed

mh0lt wants to merge 8 commits into
mainfrom
fix/prune-atomicity-clean

Conversation

@mh0lt

@mh0lt mh0lt commented Apr 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • prune.go: Ensure all in-range DupSort entries for a key are deleted atomically — ctx/throttle checks happen between keys, not between dups of the same key. Prevents transient stale state if prune is interrupted mid-key.
  • domain.go: Add assertPruneConsistency function (enabled via AGG_ASSERT_PRUNE=true env var) that verifies file/DB value consistency before pruning. Catches collation/merge errors at prune time before they become silent state corruption.
  • aggregator.go: Add diagnostic warning when BuildFilesInBackground sees lastInDB > execStep.
  • memory_mutation.go: Delegate ViewID() to underlying tx instead of panicking. Unblocks AGG_ASSERTS diagnostics.

Background

Block 24,857,060 on mainnet fails with gas mismatch (-31,400) due to stale storage value in snapshot file. Investigation traced the root cause to domain values prune where DupSort entries can be partially deleted, leaving stale older values as authoritative after the newer entry is removed.

Test plan

  • Fresh sync from genesis past block 24,857,060 — no gas mismatch, nonce errors, or assertion warnings
  • go test ./db/state/ — all pass
  • go test ./db/kv/prune/ — all pass
  • go test ./db/kv/membatchwithdb/ — all pass
  • Re-run with AGG_ASSERT_PRUNE=true to verify no prune consistency violations

🤖 Generated with Claude Code

Mark Holt and others added 7 commits April 12, 2026 16:27
The preverified snapshot set may include state domain files whose
committed block number extends past the block file boundary. This
causes "behind commitment" errors on fresh sync because SeekCommitment
returns a block number that TxNums doesn't cover.

After downloading snapshots and opening the aggregator files, read the
commitment "state" key to get the committed block number. If it exceeds
FrozenBlocks(), iteratively remove the highest state files (across all
dirs: domain, idx, history, accessor) and strip them from
preverified.toml so the downloader doesn't re-fetch them.

The alignment loop removes one step range per iteration, reopening the
aggregator each time until the commitment block is within bounds.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use existing tx instead of opening nested transaction (Copilot review)
- Return (step, found bool) from findHighestStateFileStartStep to handle
  step 0 correctly (Copilot review)
- Filter to state domain files only via isStateDomainFile() to avoid
  accidentally deleting block snapshot indices (Copilot review)
- Handle errors from os.Chmod/os.WriteFile on preverified.toml (Copilot)
- Skip directory entries with e.IsDir() (Copilot review)
- Update comment to match actual algorithm (Copilot review)
- Remove fixed iteration cap — loop until aligned or no progress
- De-register deleted files from downloader via seeder.Delete()

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The alignment was running on every restart, not just fresh starts.
When the downloader re-downloads state files between restarts, the
alignment would cascade — removing re-downloaded files, then the ones
below, until all state snapshot files were gone.

Fix: check stages.Execution progress before aligning. If > 0, the
node already processed past any snapshot misalignment and the
alignment is unnecessary.

Also added debug logging to trace alignment decisions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tests for parseFileStepRange, isStateDomainFile,
findHighestStateFileStartStep, removeStateFilesFromStep, and the
stageProgress guard that prevents alignment on restart.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the stageProgress > 0 guard with a TxNums.Max(frozenBlocks)
check. This is a lower-level check that directly verifies the TxNums
index covers all frozen blocks — meaning the node already indexed
past the block boundary. More reliable than stageProgress which could
be set from a different execution context.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The preverified snapshot set may include state domain files whose
committed block number extends past the block file boundary. This
causes "behind commitment" errors on fresh sync because SeekCommitment
returns a block number that TxNums doesn't cover.

After downloading snapshots and opening the aggregator files, read the
commitment "state" key via a temporary temporal RO tx. If the committed
block exceeds FrozenBlocks(), iteratively remove the highest state files
(across all dirs: domain, idx, history, accessor), de-register them from
the downloader via seeder.Delete(), and strip from preverified.toml.

The alignment only runs on fresh start — if TxNums already covers all
frozen blocks (checked via a temporal RO tx), the node previously
indexed past the boundary and alignment is skipped. This prevents the
cascade bug where re-downloaded files on restart would trigger repeated
alignment until all state files were removed.

Includes:
- isStateDomainFile() filter to avoid deleting block snapshot files
- findHighestStateFileStartStep returns (step, found) for step-0 safety
- Debug logging for alignment decisions
- Unit tests for all helper functions

Tested end-to-end:
1. Fresh start: alignment removes one misaligned step (44 files),
   execution starts from aligned commitment point
2. Restart: alignment skips entirely, execution resumes, no files removed

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mh0lt
mh0lt force-pushed the fix/prune-atomicity-clean branch 6 times, most recently from 287532d to 51798e9 Compare April 12, 2026 23:04
// already-open aggregator) and compare with cfg.blockReader.FrozenBlocks().
// If ahead, remove the highest state files (all domains), de-register them
// from the downloader, strip from preverified.toml, reopen, and repeat.
func alignStateToBlockSnapshots(ctx context.Context, agg *state.Aggregator, cfg SnapshotsCfg, logPrefix string, logger log.Logger) error {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

let's move it to another PR

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

updated sub-modules by mistake

Comment thread db/state/domain.go Outdated
return fmt.Errorf("prune assertion failed: domain=%s key=%x dbVal=%x dbStep=%d — file has no entry",
dt.name, k, dbVal, dbStep)
}
if foundInFile && !bytes.Equal(fileVal, dbVal) {

@AskAlexSharov AskAlexSharov Apr 13, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i don't understand assert logic: if foundInFile && !bytes.Equal(fileVal, dbVal) { why values in db and file must be equal?

@AskAlexSharov AskAlexSharov left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

prune assertion4 failed: domain=commitment key=00000157 dbStep=8813 fileStep=8814 — file/DB value mismatch (stale collation) - unclear why values from different steps expected to be equal. i guess need add dbStep == fileStep to assert

Comment thread db/kv/prune/prune.go
}
stat.PruneCountValues++
}
// Check throttle/ctx only AFTER all in-range dups for this key are deleted

@AskAlexSharov AskAlexSharov Apr 13, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we can try. but it will lead to unpredictable prune time on chain-tip.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

also unclear - what will happen with non-dupsort tables (they can have exactly same keys/values amount like dup-sort tables - means non-dupsort tables still can have garbage)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we can try. but it will lead to unpredictable prune time on chain-tip. -- yes, we did have atomic per-key prune deletion; but moved away from it because we couldn't prune commitment history this way.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we did have atomic per-key prune deletion - we had another sort of "atomic per key" where erigon did delete "all domains for given txNum" and it way more data than "just domain per-key atomicity".

@AskAlexSharov

Copy link
Copy Markdown
Collaborator

extracted state ahead of files to another pr: #20508

@mh0lt
mh0lt force-pushed the fix/prune-atomicity-clean branch 4 times, most recently from f60f41d to 180abe9 Compare April 14, 2026 18:59
Fix state corruption (gas mismatch / nonce errors) caused by partial prune
of DupSort entries leaving stale values in the DB.

**prune.go**: Move ctx/throttle checks to after all in-range dups for a key
are deleted. Previously, these checks happened between individual dup
deletions within the same key. If interrupted mid-key, the DB could
transiently hold only an older stale value after the newer entry was
deleted.

**aggregator.go**: Add diagnostic warning when BuildFilesInBackground sees
lastInDB > execStep (the step execution is currently writing to). This
invariant should always hold — a violation would indicate a data
consistency issue.

**memory_mutation.go**: Delegate ViewID() to the underlying tx instead of
panicking. This unblocks AGG_ASSERTS diagnostics which use ViewID for
cursor cache validation.

Verified: fresh sync from genesis past block 24,857,060 (the originally
failing block) with no gas mismatch, nonce errors, or assertion warnings.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mh0lt
mh0lt force-pushed the fix/prune-atomicity-clean branch from 180abe9 to 4af2f83 Compare April 14, 2026 19:04
@AskAlexSharov

Copy link
Copy Markdown
Collaborator

seems initial problem was fixed. closing. feel free to reopen if need

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants