erigontech / erigontech/erigon
db/snapshotsync: RemoveOverlaps deletes every .tmp in its dir, racing concurrent merges
- 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
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