Skip to content

Fall back to fastq-dump for heavily-aligned SRA runs - #342

Closed
tsackton wants to merge 2 commits into
mainfrom
fix/sra-aligned-run-extraction
Closed

Fall back to fastq-dump for heavily-aligned SRA runs#342
tsackton wants to merge 2 commits into
mainfrom
fix/sra-aligned-run-extraction

Conversation

@tsackton

@tsackton tsackton commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Problem

download_sra cannot extract some reference-aligned (cSRA) accessions at all. On a Pseudophryne corroboree run (GCA_028390025.1, 8.87 Gbp), 11 of 20 accessions burned two 48 h walls and ~2,100 CPU-hours without producing a single FASTQ.

The cause is fasterq-dump's lookup phase. For reference-aligned submissions it re-pairs reads by building a spot-id lookup over the entire PRIMARY_ALIGNMENT table and sorting it externally. Forensics on the leftover temp dirs show it emitting one 244 MB sorted run every ~2 hours, steadily, for 18.6 h — ~122 MB/h, still in lookup construction, nowhere near done.

Measurements

accession aligned reads (PRIM) reference fasterq-dump fastq-dump
SRR000001 0 (unaligned) 3 s 6 s
SRR28349113 54.2M 0.69 Gbp 46.8 min 87.9 min
SRR28349114 1,108.7M 8.88 Gbp ~577 h (est.) ~3.3 h (est.)

The blow-up is not just "more alignments". The third run has 20× more, but is ~90× slower per alignment, so cost tracks alignments × f(reference size) rather than alignment count alone.

fastq-dump streams in spot order and builds no lookup, so it is unaffected — but it is ~2× slower everywhere else, so it must not become the default.

What was ruled out

  • Buffer tuning (--curcache 4G --bufsize 64M --mem 8G): 1.46× at best, against the ~29× needed. Measured head-to-head at 45 min per arm.
  • Node-local temp: fasterq-dump sizes the lookup upfront (~176 GB) and refuses to start with disk-limit exeeded.
  • sam-dump: viable (~13.8 h) but 4× slower than fastq-dump and needs collate to re-pair.
  • ENA mirrors: this BioProject has no fastq_ftp, submitted_ftp or sra_ftp at ENA, so the rule's existing ffq fallback could not have helped.

Changes

1. fastq-dump as a fallback, never the default.

  • Log SEQ/PRIM/REF/SEC on every download, so thresholds can be recalibrated from real data later.
  • Skip fasterq-dump outright only when a run has both many alignments and a large reference. Both are required because the two axes are confounded in the only dataset measured — the shortcut fires only inside the region actually observed to fail.
  • Otherwise run fasterq-dump under a wall-clock budget (default 6 h) and fall back if it expires. The budget cannot mis-classify, which the thresholds can.
  • A timed-out fasterq-dump leaves a partial FASTQ; it is deleted before the fallback runs, since pigz would otherwise compress a truncated file and hand silently corrupt reads to fastp.

2. Downloads into a job-private directory.

prefetch ran without -O, writing one <accession>/ dir into the workflow root with jobs running up to 500-wide, and the rule bracketed itself with rm -rf <accession>. If two sample rows reference one accession, either job's teardown can delete the other's in-flight download. Each job now gets its own directory under resources.tmpdir, removed by an EXIT trap so it cleans up on timeout or cancellation too.

Verification

  • Output equivalence checked, not assumed. Given --split-3 --skip-technical (both needed — fastq-dump's defaults differ from fasterq-dump's on each), the two produce byte-identical FASTQ. Confirmed on SRR000001 and on all 307 GB of SRR28349113: 400,318,389 reads per mate, cmp-clean.
  • Routing exercised against real vdb-dump output for all three shapes, including a flat run where PRIM/REF lines are absent and the parse degrades safely to 0.
  • Both branches run end-to-end on SRR000001: normal budget (fasterq-dump succeeds) and a 1-second budget (forced timeout → partial cleanup → fallback). Same reads either way; no accession dir left behind; scratch removed.
  • Full DAG builds (75 jobs) with the workflow profile.
  • Passing a full .sra path still resolves a run's sibling reference object, and both tools name output from the basename, so downstream globs are unchanged.

Notes for reviewers

  • No CI test pins the byte-identity claim, which is the entire safety argument for the fallback. A test config with a low fasterq_budget_minutes would force the path and assert parity — worth adding, and I'm happy to.
  • The thresholds (3e8 alignments, 3 Gbp) are calibrated on three accessions from one project. The commit message and an fastq.smk header comment say so explicitly, and the logging exists so they can be revisited.
  • test_markdup fails on this machine (sambamba on macOS/arm64), identically on an unmodified tree — pre-existing, unrelated.

🤖 Generated with Claude Code

Tim Sackton and others added 2 commits September 2, 2026 09:25
fasterq-dump re-pairs reads from reference-aligned (cSRA) submissions by
building a spot-id lookup over the PRIMARY_ALIGNMENT table and sorting it
externally. That phase does not scale, and on VGP-scale data it does not
finish at all.

Measured on Pseudophryne corroboree (GCA_028390025.1):

  accession     aligned reads   reference    fasterq-dump    fastq-dump
  SRR000001     0 (unaligned)   -                     3 s           6 s
  SRR28349113   54.2M           0.69 Gbp          47 min        88 min
  SRR28349114   1,108.7M        8.88 Gbp    ~577 h (est.)  ~3.3 h (est.)

The cost is not merely proportional to alignment count: the third run has
20x more alignments but is ~90x slower per alignment, so total cost tracks
alignments x f(reference size). Buffer tuning does not recover it -- raising
--curcache/--bufsize/--mem gained at most 1.46x against the ~29x needed.

fastq-dump streams in spot order and builds no lookup, so it is unaffected.
It is also ~2x slower everywhere else, so it must not become the default;
this adds it as a fallback instead.

Output equivalence was verified rather than assumed: given --split-3 and
--skip-technical (fastq-dump's defaults differ from fasterq-dump's on both),
the two produce byte-identical FASTQ. Checked on SRR000001 and on all 307 GB
of SRR28349113 -- 400,318,389 reads per mate, cmp-clean.

Routing:
  - Skip fasterq-dump outright only when a run has many alignments AND a
    large reference. Both conditions are required because the two axes are
    confounded in the only dataset measured, so the shortcut fires only
    inside the region actually observed to fail.
  - Otherwise run fasterq-dump under a wall-clock budget and fall back if it
    expires. The budget cannot mis-classify, which the thresholds can.
  - Log SEQ/PRIM/REF/SEC on every download so the thresholds can later be
    recalibrated from real data.

A timed-out fasterq-dump leaves a partial FASTQ behind; it is deleted before
the fallback runs, since pigz would otherwise compress a truncated file and
hand silently corrupt reads to fastp.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
prefetch was invoked without -O, so it wrote into the Snakemake working
directory: one <accession>/ directory per download in the workflow root,
created and removed by jobs running up to 500-wide. The rule bracketed itself
with `rm -rf <accession>`, which is safe only while no two jobs touch the same
name -- but nothing prevents two sample rows from referencing one accession,
and in that case either job's teardown could delete the other's in-flight
download or its .sra mid-extraction.

Give each job its own directory under resources.tmpdir instead, and remove it
with an EXIT trap so it is cleaned up however the job ends, including on a
timeout or a cancelled Slurm allocation. The fasterq-dump scratch moves inside
it, which drops a second mktemp and its matching cleanup.

Extractors now take the .sra path rather than the bare accession, so
resolution no longer depends on the working directory. Verified that a full
path still resolves a run's sibling reference object -- fastq-dump reconstructs
real bases from a cSRA run given an absolute path from an unrelated cwd -- and
that both tools name output files from the basename, so downstream globs are
unchanged.

Exercised end-to-end on SRR000001, both branches: normal budget (fasterq-dump
succeeds) and a 1-second budget (forced timeout, partial cleanup, fastq-dump
fallback). Both produce the same reads, the working directory is left with no
accession directory, and the scratch directory is gone afterwards.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tsackton
tsackton marked this pull request as draft September 4, 2026 16:21
@tsackton

tsackton commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Marking draft: the performance comparison in the description is being re-tested. The fastq-dump timings were extrapolated from a partial run and do not account for extraction rate varying with position in the file, so the claimed speedup is not established. The prefetch isolation fix in 5408621 is unaffected and will be split out.

@tsackton

tsackton commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Closing this in favour of a new branch off main.

The performance argument here does not hold. The fastq-dump timings were extrapolated from the first 22.6% of a file, and extraction rate degrades sharply with position — a later investigation measured fastq-dump decaying from 5.4 GB/h to 0.08 GiB/h on this data, projecting ~52 days for a single accession. fastq-dump is not a viable fallback, so the branch this PR adds is pointless.

The replacement routes aligned (cSRA) runs through sam-dump | samtools collate | samtools fastq, which reads in reference order rather than spot order, gated on PRIM > 0 instead of tunable thresholds. See #343 for the full rationale and measurements.

The job-private download directory from 5408621 is unaffected and carries over to the new branch — it fixes a real bug where concurrent download_sra jobs shared accession directories in the workflow root. It could not be split out by reverting here, because the two commits touch the same lines.

Superseded by #343.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant