Final downloader cleanup by anacrolix · Pull Request #18056 · erigontech/erigon

anacrolix · 2025-11-25T22:58:42Z

Fixes a few things that bothered me that weren't right in 3.1. #16532, #17664, #17310, #16654.

#16698 is not addressed here (not sure if I will, but testing this might make it worthwhile).

List of changes:

Downloader.Add is split into Seed and Download. This prevents allowing downloads to occur in parts of code that expect to only Seed (like in the pruning code). It also prevents incomplete/partial torrents being added. Download now blocks and logs synchronously until all requested snapshots are complete. This simplifies the API back and forth with the caller. Snapshot requests now include an appropriate name for the request set.
Download.Completed, SetLogPrefix are no longer required. These don't work well since Downloader can be shared, and those concepts don't support sharing.
A few logs that weren't clear they weren't related to the downloader/BitTorrent were adjusted.
A bunch of metrics in webseed requesting that aren't needed anymore were removed.
Download.AddTorrentsFromDisk can be called without breaking existing downloads. It only adds completed snapshots, it's assumed anything else would be a mistake. It's no longer the default anywhere.
torrent.TorrentSpec is not used directly anymore. It's been deprecated for a long time.
I fixed STOP_BEFORE_STAGE and STOP_AFTER_STAGE.
There's no longer snapshot "validation" on completion. We're trusting the torrent client to do the right thing, but also not asking it to download things that should already be complete.
Data is always rechecked if an infohash changes.
Torrent/metainfo files are retained on disk before initial sync completes. This means significantly faster start-up times if a sync is resumed midway.

anacrolix · 2025-12-01T07:24:00Z

Oops I was trying to figure out how to get linting to work and modified it. I'll restore it.

Doesn't actually help yet because nothing uses it further up the stack.

…te Seed and Download

This finishes what is required for snapshot reset to do the right thing. If data is complete but the infohash for which it is complete is unknown, we can't trust it.

I think it regenerated stuff because we moved to a newer grpc version.

@Giulio2002

## Summary - Moves `AddTorrentsFromDisk` call from `initDownloader` (at startup, before any sync) to an `afterSnapshotDownload` callback invoked once the snapshot sync stage completes on the first sync cycle. - This eliminates false-positive incomplete torrent reports that occurred because we were registering on-disk snapshots before the sync stage had a chance to complete them. - The `afterDownload` callback is threaded through `StageSnapshotsCfg`, `NewDefaultStages`, and `NewPipelineStages` with a `nil` default so callers that don't need it are unaffected. - Incomplete snapshot warnings after this callback are now flagged as unexpected (sync just finished), with a comment noting the preverified-set edge case that remains unresolved. ## Background Issue #19435 first identified this problem: `AddTorrentsFromDisk` was being called before initial sync status was checked, causing old or stale `.torrent` files on disk to interfere with the sync stage's ability to fetch the correct (preverified) versions of snapshots. PR #19316 by @Giulio2002 addressed a related symptom — converting the fatal "snapshot already loaded with different infohash" error in `addPreverifiedSnapshotForDownload` into a warning — but this was treating a symptom rather than the root cause. Other related changes to this area: - PR #18056 — "Final downloader cleanup": Changed `AddTorrentsFromDisk` to only add completed snapshots (anything incomplete is assumed a mistake), and removed it as a default at startup. - PR #18270 — "Quick fix for --downloader.verify assertion": Fixed a panic in `addCompleteTorrentFromMetainfo` when called from `AddTorrentsFromDisk` on already-completed files (#18165). - PR #15043 — "Fix a collection of downloader, snapshot sync and torrent related issues": Earlier broad fix addressing how disk-loaded torrents interact with preverified snapshots. - PR #16655 — "Require torrents added from disk to complete on `downloader.verify`": Enforced completion check for disk-loaded torrents during verify. @mh0lt — please take a look, particularly at the scenario where extra snapshots land on disk that weren't part of the sync set (e.g. after an upgrade or downgrade that changes the preverified set). --------- Co-authored-by: Alex Sharov <AskAlexSharov@gmail.com>

@Giulio2002

## Summary - Moves `AddTorrentsFromDisk` call from `initDownloader` (at startup, before any sync) to an `afterSnapshotDownload` callback invoked once the snapshot sync stage completes on the first sync cycle. - This eliminates false-positive incomplete torrent reports that occurred because we were registering on-disk snapshots before the sync stage had a chance to complete them. - The `afterDownload` callback is threaded through `StageSnapshotsCfg`, `NewDefaultStages`, and `NewPipelineStages` with a `nil` default so callers that don't need it are unaffected. - Incomplete snapshot warnings after this callback are now flagged as unexpected (sync just finished), with a comment noting the preverified-set edge case that remains unresolved. ## Background Issue #19435 first identified this problem: `AddTorrentsFromDisk` was being called before initial sync status was checked, causing old or stale `.torrent` files on disk to interfere with the sync stage's ability to fetch the correct (preverified) versions of snapshots. PR #19316 by @Giulio2002 addressed a related symptom — converting the fatal "snapshot already loaded with different infohash" error in `addPreverifiedSnapshotForDownload` into a warning — but this was treating a symptom rather than the root cause. Other related changes to this area: - PR #18056 — "Final downloader cleanup": Changed `AddTorrentsFromDisk` to only add completed snapshots (anything incomplete is assumed a mistake), and removed it as a default at startup. - PR #18270 — "Quick fix for --downloader.verify assertion": Fixed a panic in `addCompleteTorrentFromMetainfo` when called from `AddTorrentsFromDisk` on already-completed files (#18165). - PR #15043 — "Fix a collection of downloader, snapshot sync and torrent related issues": Earlier broad fix addressing how disk-loaded torrents interact with preverified snapshots. - PR #16655 — "Require torrents added from disk to complete on `downloader.verify`": Enforced completion check for disk-loaded torrents during verify. @mh0lt — please take a look, particularly at the scenario where extra snapshots land on disk that weren't part of the sync set (e.g. after an upgrade or downgrade that changes the preverified set). --------- Co-authored-by: Alex Sharov <AskAlexSharov@gmail.com>

anacrolix force-pushed the anacrolix/downlaoder-final branch 3 times, most recently from e5c2935 to dbbcab2 Compare November 25, 2025 23:41

AskAlexSharov approved these changes Nov 26, 2025

View reviewed changes

Comment thread db/downloader/downloader.go Outdated

anacrolix force-pushed the anacrolix/downlaoder-final branch from 7f7f75d to 4b91260 Compare December 1, 2025 05:26

This was referenced Dec 1, 2025

Rework snapshot/downloader interface #15514

Closed

Same file size but different hash may not be checked by downloader #17664

Closed

anacrolix marked this pull request as ready for review December 1, 2025 07:03

anacrolix requested review from Giulio2002, domiwei, mh0lt, sudeepdino008, taratorio and yperbasis as code owners December 1, 2025 07:03

anacrolix force-pushed the anacrolix/downlaoder-final branch from 4b91260 to 5dbec1b Compare December 1, 2025 07:30

anacrolix added 8 commits December 2, 2025 00:06

Cleaning up Downloader

c0817ce

Return stage failure error too

fc6e8a6

Doesn't actually help yet because nothing uses it further up the stack.

Fix tests, remove unused stuff from requestHandler, explicitly separa…

1f8dc5d

…te Seed and Download

Update pruning, exit on sync stop, background log when not downloading

7489304

Apply lints

c991207

Do background logging more lazily

5fb4daf

Also invalidate data if metainfo is missing

048acea

This finishes what is required for snapshot reset to do the right thing. If data is complete but the infohash for which it is complete is unknown, we can't trust it.

go mod tidy for lint

44e195e

anacrolix force-pushed the anacrolix/downlaoder-final branch from 0984914 to 44e195e Compare December 1, 2025 13:06

anacrolix mentioned this pull request Dec 2, 2025

db/downloader: remove unused rate allocations in stats helpers #18125

Closed

AskAlexSharov merged commit cc44877 into main Dec 2, 2025
19 checks passed

AskAlexSharov deleted the anacrolix/downlaoder-final branch December 2, 2025 02:11

This was referenced Dec 2, 2025

downloader: re-visit validate method #16532

Closed

Downloader.Add: follow up #16654

Closed

This was referenced Dec 2, 2025

Downloader fetches metainfo for incomplete snapshots each time on initial sync #16698

Closed

Changes to Downloader GRPC erigontech/interfaces#273

Merged

anacrolix added a commit that referenced this pull request Dec 5, 2025

Forgot to update the interfaces submodule in #18056

6668a7f

anacrolix added a commit that referenced this pull request Dec 5, 2025

Forgot to update the interfaces submodule in #18056

5ccf7a8

AskAlexSharov pushed a commit that referenced this pull request Dec 5, 2025

Forgot to update the interfaces submodule in #18056 (#18185)

e5a445d

I think it regenerated stuff because we moved to a newer grpc version.

anacrolix mentioned this pull request Mar 10, 2026

downloader: add torrents from disk after snapshot sync stage #19712

Merged

anacrolix mentioned this pull request Mar 18, 2026

"staticpeers" param is not supported in toml config #17388

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Final downloader cleanup#18056

Final downloader cleanup#18056
AskAlexSharov merged 8 commits intomainfrom
anacrolix/downlaoder-final

anacrolix commented Nov 25, 2025 •

edited

Loading

Uh oh!

Uh oh!

anacrolix commented Dec 1, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

anacrolix commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

anacrolix commented Dec 1, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

anacrolix commented Nov 25, 2025 •

edited

Loading