Skip to content

db/downloader: kept-snapshot seeding fans out unbounded and abandon() joins it uncancellably #23448

Description

@awskii

#23350 added a seeding step for snapshots kept on the complete path. On main today:

// db/downloader/download-batch.go:47
if keptLocal {
	me.all.Go(func() { me.d.seedKeptSnapshot(me.d.ctx, item.Name) })
}

Two problems compound.

Unbounded fan-out. all is a plain sync.WaitGroup (download-batch.go:18), so this is one goroutine per preverified item. Each runs seedKeptSnapshotAddNewSeedableFileBuildTorrentIfNeedBuildFromFilePath, which SHA-1s the whole data file, holding an open fd and a sequential read for the duration. The package's own whole-file hashing loop bounds itself — util.go:200 does g.SetLimit(runtime.GOMAXPROCS(-1) * 16).

Wrong context. The task captures me.d.ctx, and d.ctx is context.WithCancel(context.Background()) (downloader.go:331) — not derived from the caller. abandon() fires me.cancel(...) then blocks in me.all.Wait() (download-batch.go:91-93), but that cancel is batchCtx's and these tasks never observe it. BuildFromFilePath takes no context, and BuildTorrentIfNeed checks ctx.Done() once on entry. The batch.all.Go at downloader.go:847 passes batchCtx for exactly this reason, one line after it is created at 845 — same batch, same WaitGroup.

Trigger

A datadir with preverified.toml and the snapshot data but no .torrent files — rsync'd or restored without them, which is the case the keep behaviour exists for. Every item takes the kept-local branch: prepareLocalDataForDownload finds no metainfo, complete regime, data exists, not unbacked → keptLocal=true.

Both production callers reach it:

  • cmd/downloader --seedbox issues one Download with the entire preverified set (cmd/downloader/main.go:362-373), synchronously, before the gRPC server starts.
  • A node reaches it more easily than the seedbox: AddTorrentsFromDisk runs only after the download on that path (node/eth/backend.go:919-933 via afterSnapshotDownload, execution/stagedsync/stage_snapshots.go:252), so torrentsByName is empty when Download arrives, and db/snapshotsync/snapshotsync.go:557 requests the whole filtered preverified set regardless of what is on disk.

Effect

Thousands of goroutines each hashing a full file: open descriptors, N concurrent sequential reads and N CPU-bound SHA-1s, plus disk thrash. Ctrl-C then makes it worse — wait(ctx) returns the context error and defer me.abandon() blocks in all.Wait() until every file has been fully hashed, with the tasks' own context still live. An uninterruptible hang. Failed seed jobs are logged at warn while the Download RPC still reports success.

Fix

Bound the fan-out the way BuildTorrentFilesIfNeed does — an errgroup or semaphore at runtime.GOMAXPROCS(-1)*16, shared across the batch — and pass batchCtx instead of me.d.ctx so abandon's cancel drops queued seed tasks at BuildTorrentIfNeed's entry check.

Keep the tasks on me.all: TestKeptLocalSnapshotIsSeeded (downloader_test.go:664-679) depends on abandon()'s join for determinism. Coalescing seed work by snapshot name would also stop duplicate request items double-hashing.

Where

main at db/downloader/download-batch.go:47, and the same line in the release/3.6 cherry-pick #23446.


Corrected: an earlier revision of this issue claimed ~2 MiB of piece buffers per in-flight hash. There is no piece-sized buffer — GeneratePieces does io.CopyN(h, r, pieceLength), and since hash.Hash does not implement ReaderFrom nor LimitReader WriterTo, io.copyBuffer allocates a fresh 32 KiB buffer per call. The pressure is fd, IO and CPU, not memory. The unbounded fan-out itself is unaffected.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions