erigontech / erigontech/erigon

db/snapshotsync: RemoveOverlaps deletes every .tmp in its dir, racing concurrent merges

Open
#23,470 0 comments 0 reactions 0 assignees View on GitHub
ErigonDB
Dominant language
Go
Stars
3.6k
Forks
1.5k
Avg merge
1d 16h
Merged PRs (30d)
455

Description

`BaseRoSnapshots.RemoveOverlaps` ends by unlinking every `*.tmp` in `s.dir` (`db/snapshotsync/snapshots.go:1613-1621`). The TODO already sitting on that block flags the hazard.

`seg.Compressor` writes its output through `dir.CreateTemp(c.outputFile)` (`db/seg/compress.go:321`), and `dir.CreateTemp` creates `..tmp` in the output file's own directory (`common/dir/rw_dir.go:235-242`) — for EL segments, `dirs.Snap` — then renames it into place at `compress.go:389`.

Today the sweep is safe only by accident. Its sole caller is `BlockRetire.MergeBlocks` (`db/snapshotsync/freezeblocks/block_snapshots.go:333`), which runs after `merger.Merge` returns on the same goroutine, so it never overlaps a live compressor.

That stops holding as soon as a second, independently scheduled collection sharing `dirs.Snap` calls it. `CaplinSnapshots` is constructed over `dirs.Snap` (`db/snapshotsync/freezeblocks/caplin_snapshots.go:78`), and the caplin antiquary runs on its own 12s ticker (`cl/antiquary/antiquary.go:229-249`). A caplin-side call landing mid-merge unlinks the EL merge's temp file; on Linux the compressor keeps writing into the unlinked inode and `os.Rename` then fails with ENOENT, aborting the merge. The source segments survive, so this is a failed-and-retried merge rather than corruption.

This is why #23412 limited its node-path wiring to `CaplinStateSnapshots`. Note that a separate directory is **not** what makes that safe, contrary to what this issue first said: `dir.CreateTemp` creates the temp in the output file's own directory (`common/dir/rw_dir.go:236`), so `db/seg/compress.go:321` and `db/recsplit/recsplit.go:947` put in-flight `.tmp` files into `dirs.SnapCaplin` too. It is safe only because `DumpCaplinState` and `RemoveOverlaps` run sequentially on the single `loopStates` goroutine — parallelising the per-table dump would break it.

Two ways out: scope the sweep to `.tmp` names the collection's own types could have produced, or drop it from `RemoveOverlaps` and leave temp cleanup to the startup sweep it duplicates.

Part of #23024.

Contributor guide

Open the contributing guide

Research direction

Start with RemoveOverlaps in db/snapshotsync/snapshots.go and trace its caller in db/snapshotsync/freezeblocks/block_snapshots.go, then read CreateTemp in common/dir/rw_dir.go and the compressor flow in db/seg/compress.go. Compare the Caplin call path in caplin_snapshots.go and antiquary.go; done means concurrent collection work no longer removes another merge's temporary file or causes the merge to fail with ENOENT.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
database
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.