Skip to content

Never assemble a download at its destination path - #341

Merged
nuwang merged 1 commit into
mainfrom
atomic-ranged-download
Aug 2, 2026
Merged

Never assemble a download at its destination path#341
nuwang merged 1 commit into
mainfrom
atomic-ranged-download

Conversation

@nuwang

@nuwang nuwang commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

The bug

A Galaxy deployment on OpenStack Swift hit this downloading a 4.3 GB dataset:

File ".../cloudbridge/base/resources.py", line 961, in fetch_one
    with open(path, 'r+b') as f:
FileNotFoundError: [Errno 2] No such file or directory: '.../dataset_85cb....dat.tmp'

download_to_file built the object at the destination path: the generic ranged driver created the file up front and reopened it by path for every range, and the Azure downloader wrote in place. Callers commonly download every copy of an object to one well-known path — Galaxy's dataset cache uses a fixed <dataset>.tmp per dataset — so a second download of the same object shares that path. When one download finished and renamed the path into place, the other's next range reopened a path that no longer existed and blew up; before that, its open(path, 'wb') had already truncated the first download's in-progress file. A failed transfer also os.removed the destination, destroying whatever was previously downloaded there.

Only GCP and OpenStack Swift were exposed to the crash: AWS overrides the download with boto3's TransferManager, which already assembles in its own temp file — which is why this never showed up in the mock/MinIO tests (they run the AWS provider) and only surfaced against Swift. Azure was exposed to the partial-file and destroyed-destination variants.

The fix

download_to_file assembles into a private sibling file and os.replaces it into place once complete, so the destination only ever holds a whole object: an existing file is replaced atomically, a failed transfer leaves it untouched, and concurrent downloads to one path are safe with the last to complete winning. Providers now implement _download_to_path (filling a caller-owned path) instead of overriding download_to_file, so boto3, Azure and the generic driver all inherit the guarantee.

The ranged driver additionally writes through the single handle it opened rather than reopening the path per range — a range can no longer land in a file that has since been replaced, and 80+ opens become one. Writes are serialized with a lock, which costs little next to the network fetches and keeps the memory bound at ~concurrency × part_size.

The interface docstring now states the guarantee explicitly.

Tests

Three new cases in tests/test_download_driver.py, all red before the fix:

  • the destination never exists while ranges are in flight (nothing partial is visible)
  • a concurrent downloader renaming the destination away mid-transfer — the reported crash — completes fine
  • a failed transfer leaves an existing destination intact and no scratch files behind

Verification

  • tox -e py3.13-mock green; tox -e mypy and tox -e lint clean
  • Verified end-to-end through Galaxy's cloud object store against MinIO: 400 MB object, two concurrent downloads racing for one cache path, SHA256 verified

@nuwang
nuwang had a problem deploying to cloud-integration July 31, 2026 07:33 — with GitHub Actions Failure
@nuwang
nuwang temporarily deployed to cloud-integration July 31, 2026 07:33 — with GitHub Actions Inactive
@nuwang
nuwang had a problem deploying to cloud-integration July 31, 2026 07:33 — with GitHub Actions Failure
@nuwang
nuwang temporarily deployed to cloud-integration July 31, 2026 07:33 — with GitHub Actions Inactive
download_to_file built the object at the destination: the generic ranged
driver (GCP and OpenStack Swift) created the file up front and reopened
it by path for every range, and the Azure downloader wrote in place.
Callers commonly download every copy of an object to one well-known path
- a download cache keyed by object, say - so a second download of the
same object could truncate the first's file, and renaming that path into
place mid-transfer left the other download reopening a path that no
longer existed, failing with FileNotFoundError. A failed transfer also
deleted whatever was already at the destination.

Assemble into a private sibling file and rename it into place once
complete, so the destination only ever holds a whole object and a failed
transfer leaves an existing one untouched. Providers now fill a caller-
owned path via _download_to_path and inherit that guarantee. Ranges are
written through the single handle the driver opened, so a range can
never land in a file that has since been replaced.
@nuwang
nuwang force-pushed the atomic-ranged-download branch from 15c0bfd to 836da4e Compare August 1, 2026 18:38
@nuwang
nuwang temporarily deployed to cloud-integration August 1, 2026 18:39 — with GitHub Actions Inactive
@nuwang
nuwang temporarily deployed to cloud-integration August 1, 2026 18:39 — with GitHub Actions Inactive
@nuwang
nuwang temporarily deployed to cloud-integration August 1, 2026 18:39 — with GitHub Actions Inactive
@nuwang
nuwang deployed to cloud-integration August 1, 2026 18:39 — with GitHub Actions Active
@nuwang
nuwang merged commit 8fabc1e into main Aug 2, 2026
9 checks passed
@nuwang
nuwang deleted the atomic-ranged-download branch August 2, 2026 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant