[r3.6] db/downloader: keep local snapshot data once the initial download is complete

[AskAlexSharov]

Cherry-pick of #23350 to release/3.6.

[awskii]

Four points, one of them major and line-anchored below. The cherry-pick itself is faithful — git range-diff against the merged commit shows = — so none of this is a porting defect, and the fan-out at download-batch.go:47 is already on main from #23350 with the same me.d.ctx. A fix scoped to this backport would leave main carrying it.

Two things not line-anchored:

• maybeLoadMetainfoFromDisk's docstring at downloader.go:786 — "Loads metainfo from disk, removing it if it's invalid" — is stale; nothing is removed on any path, and the removal went in #18056 when loadSpecFromDisk was replaced. Pre-existing, but the signature directly beneath it is renamed here, and the new comment at downloader.go:1142 states the opposite ("An unreadable metainfo is logged and then treated as missing"). Two comments in one change disagreeing about whether a corrupt .torrent survives on disk.
• downloader_test.go:455 — require.Contains(logs.String(), name) cannot fail: the warn at downloader.go:1150-1153 already put name in the buffer before the assertion runs. The message assertion on the line above does bite. Worth a look because #23350's review recorded this exact assertion as fixed and verified to bite, and this commit is byte-identical to what merged.

Noting for the record that the README block reviewed below was rewritten at ec87f511/ec152a20, after the approval on #23350 at 56ce2d41 — so this is its first review in its current form.

[AskAlexSharov]

All six points verified and addressed on main in #23459 — none in this backport, for the reason you gave: the cherry-pick is faithful and a fix scoped here would leave main carrying it.

The two not line-anchored:

• maybeLoadMetainfoFromDisk's docstring — right, nothing is removed on any path. Rewritten to say what it does: valid metainfo as Some, missing as None, invalid returned as an error and left on disk. That no longer contradicts the comment at prepareLocalDataForDownload.
• downloader_test.go:455 — confirmed, the earlier warn puts name in the buffer before that line runs. The three local-snapshot tests now use a helper that requires the message and the name on the same log line, so the assertion bites again.

Of the four line-anchored, the fan-out was the one with real consequences; the rest are prose, a comment rationale, and a test hole. One deviation, noted in the thread: scoping the seeding wait to batchCtx drops seeding on the success path, so it is scoped to d.ctx.

[awskii]

Holding this one — not on the diff's own merits, but on sequencing. The cherry-pick is faithful to #23350; the problem is what #23350 is currently known to contain.

The backport would put the unbounded kept-snapshot seeding bug onto release/3.6 while the fixes for it are still unmerged on main:

• #23350 merged to main 2026-08-20
• #23459 (AskAlexSharov) — "bound kept-snapshot seeding and rebuild malformed metainfo" — open, against main
• #23472 (yours) — "bound kept-snapshot seeding and make abandonment observable" — open, against main

I checked: neither 9052d38e73 nor 58ed10b87b is an ancestor of origin/main, origin/release/3.6, or this PR's head. A review passed to me described them as "later main-branch fixes"; they are not on main, they are the two open PRs above. Flagging the correction because it changes the recommendation — this is not "backport is missing a fix that already shipped", it is "the fix does not exist anywhere yet".

Suggest landing #23459 and #23472 on main first, then cherry-picking the set to 3.6 together. A release branch is the one place I would not take the bug ahead of its fix.

Two majors inline, both describing what would land on 3.6.

Five minors, not worth blocking on individually but they travel with the same code — the README block at db/downloader/README.md:5 makes two claims about the complete path that the code contradicts; a kept snapshot with a malformed .torrent is never seeded and the corrupt file is never repaired (downloader.go:1198); the "Must run without d.lock" comments give the hashing cost rather than the re-entrancy that actually deadlocks (downloader.go:1196); TestDownloadsLocalSnapshotNotMatchingItsMetainfo has a dead assertion (downloader_test.go:455); and nothing covers the kept-snapshot seeding outcome, so the unreadable-.torrent case silently never seeds (downloader_test.go:524). Several of these look like they are already addressed in #23459 — worth diffing rather than fixing twice.

[awskii]

Blocking on a release branch. One unbounded, uncancellable whole-file-hashing goroutine per kept item.

me.all is a plain sync.WaitGroup alias, so .Go has no bound. The task captures me.d.ctx, which is context.WithCancel(context.Background()) from downloader.go:331 and is not derived from the caller — so abandon()'s me.cancel(...) (line 92) never reaches it, and abandon() then blocks in me.all.Wait().

What it waits on: seedKeptSnapshot -> AddNewSeedableFile -> BuildTorrentIfNeed -> info.BuildFromFilePath -> GeneratePieces, which opens and SHA-1s the entire data file and takes no context at all. BuildTorrentIfNeed checks ctx.Done() once on entry (util.go:158) and never again. Each in-flight hash holds an open fd plus a 2 MiB DefaultPieceSize buffer.

Trigger: a datadir with preverified.toml and snapshot data but no .torrent files — rsync'd or restored. Nothing is in torrentsByName, so every item resolves to data-present / not-unbacked / download=false, keptLocal=true. The request carries the whole filtered preverified set (6,687 entries on the mainnet fixture), so that is thousands of concurrent goroutines each streaming a whole snapshot file: fd exhaustion and heavy disk contention while hundreds of GB hash at once. On interrupt, wait(ctx) returns but defer me.abandon() hangs until every remaining file finishes hashing — and in the seedbox command d.Close() cannot cancel d.ctx until that returns. Individual seed failures only log at warn (downloader.go:1199-1201) while the Download RPC still reports success.

My reviewers split on whether erigon nodes reach this or only cmd/downloader --seedbox (SyncSnapshots skips the request when snapCfg.Local is set, snapshotsync.go:411). I did not settle it — but the restored-datadir seedbox case is the one this change exists to serve, so it is in scope either way.

[AskAlexSharov]

Agreed on the unbounded/uncancellable hashing — that is the same defect #23472 is written against, so it gets fixed on main.

One correction on what that fix currently buys, from reviewing #23472 at its current head: the bound lands, but the uninterruptible wait does not go away. wait() samples context.Cause(ctx) once and then abandon blocks in all.Wait(), so a cancel arriving during the drain is never observed — and for exactly the trigger you describe (preverified.toml + data, no .torrent) every item is kept-local, so me.torrents is empty, the loop returns nil immediately, and abandon(nil) waits for every file to finish hashing. Tasks already past the semaphore also still run under d.ctx, so up to seedConcurrency whole-file hashes remain uncancellable.

So your "on interrupt, d.Close() cannot cancel d.ctx until that returns" still holds after the bound. That needs settling on main before any of this reaches a release branch.

[awskii]

Kept torrents get registered without trackers, so nothing announces them.

AddNewSeedableFile calls addCompleteTorrent and discards the torrent it returns — _, _, err = d.addCompleteTorrent(name) — so afterAdd never runs, and afterAdd (downloader.go:1882-1884) is where t.AddTrackers(Trackers) happens. addTorrentFromMetainfo only applies info bytes and piece layers. The freshly generated .torrent does carry the tracker list on disk, but the live torrent never consumes that outer metainfo.

Reachable in cmd/downloader --seedbox: AddTorrentsFromDisk runs before the changed Download path, so a restored datadir with no .torrent files has nothing loaded, and that command does not run the node's TorrentPeerManager. Net effect is a seedbox that believes it is seeding and that no peer can discover.

The omission inside AddNewSeedableFile is pre-existing; what is new is that this change makes it the sole seeding route for every kept file without metainfo.

Single-source (conf 93) — I confirmed the afterAdd/AddTrackers mechanics above, but not the end-to-end "peers cannot discover it" behaviour. Worth a check before acting.

[AskAlexSharov]

Checked this, since you flagged it as single-source. Confirmed, and it is worse than "no trackers".

makeAddTorrentOpts (downloader.go:1812-1818) sets ts.DisallowDataUpload = true on every torrent it adds, and the only place that lifts it is afterAdd -> t.AllowDataUpload() (downloader.go:1885), which is also the sole t.AddTrackers(Trackers) call site in the file. AddNewSeedableFile -> addCompleteTorrent -> addCompleteTorrentFromMetainfo -> addTorrentFromMetainfo never reaches afterAdd, so a kept snapshot is registered with data upload disallowed.

So it is not only that no peer can discover it — even a peer that finds it via DHT/PEX or a manual add gets nothing, because the torrent will not upload. The seeding is a no-op, and the Download RPC still reports success.

Agreed the omission inside AddNewSeedableFile predates #23350; what #23350 changes is that this becomes the only seeding route for kept files without metainfo. Fix belongs on main, not on this cherry-pick.

[AskAlexSharov]

Agreed on the sequencing — holding this one. Re-checked the facts just now and they stand:

• #23350 merged to main on 2026-08-20
• #23459 — still open against main
• #23472 — still open against main

So the fix genuinely does not exist anywhere yet, and this cherry-pick would put the bug on release/3.6 ahead of it. Your correction on 9052d38e73 / 58ed10b87b is right: those are the two open PR heads, not merged main commits.

Both inline majors are answered in thread. Short version:

• Unbounded/uncancellable hashing — real, and #23472 does not fully close it at its current head: the bound lands, but wait() samples the cancel cause once and then blocks in all.Wait(), and the all-kept-local case (your exact trigger) has an empty me.torrents, so it waits for every file regardless. Needs another round on main.
• Kept torrents not seeded — confirmed, and stronger than stated: makeAddTorrentOpts sets DisallowDataUpload = true on every torrent, and only afterAdd lifts it. AddNewSeedableFile never reaches afterAdd, so the kept file has no trackers and cannot upload. Seeding is a no-op while Download reports success.

Plan, no changes to this branch: land the fix on main first (deciding between #23459 and #23472, which overlap heavily — they should not both land), add the missing afterAdd on the AddNewSeedableFile path there, then cherry-pick the whole set to 3.6 as one series and re-request review here. Fixes for a backport belong on main, not patched into the cherry-pick.

The five minors travel with the same code and get picked up in that main-side change rather than fixed twice here.

[AskAlexSharov]

Sequencing status, since this is still held on it.

The two open main-branch PRs are complementary, not duplicates:

• db/downloader: bound kept-snapshot seeding and make abandonment observable #23472 (awskii) bounds the fan-out with a downloader-wide semaphore and — the part nothing else has — fixes the uninterruptible wait: a seedCtx cancelled from the caller's context via context.AfterFunc, plus wait() surfacing context.Cause(ctx) for an all-kept-local batch that never samples ctx.
• db/downloader: bound kept-snapshot seeding and rebuild malformed metainfo #23459 (mine) bounds the fan-out per batch, which db/downloader: bound kept-snapshot seeding and make abandonment observable #23472's downloader-wide semaphore supersedes, and separately repairs malformed .torrent metainfo before AddNewSeedableFile so the kept data can regenerate it. That repair, and the README/docstring corrections, exist in neither db/downloader: bound kept-snapshot seeding and make abandonment observable #23472 nor main.

So the order that gets release/3.6 the whole fix is: land #23472, then land #23459 reduced to the metainfo repair and the docs, then cherry-pick both here on top of #23350.

Nothing on this PR's own diff has changed — the cherry-pick is still faithful.