Skip to content

fix: harden checkpoint downloader for startup-critical fetches#29

Open
JasonWildMe wants to merge 1 commit into
mainfrom
fix/harden-checkpoint-download
Open

fix: harden checkpoint downloader for startup-critical fetches#29
JasonWildMe wants to merge 1 commit into
mainfrom
fix/harden-checkpoint-download

Conversation

@JasonWildMe

Copy link
Copy Markdown

What

Hardens checkpoint_utils.download_checkpoint — the mechanism behind every URL-configured model weight, which runs during service startup:

  • Wall-clock deadline (15 min) for the whole fetch, enforced by running the download in a worker thread and bounding the wait — a stalled connection or a drip-feeding server can no longer block readiness forever. A socket-level (10, 60) connect/read timeout backs it up. On expiry, the response is closed and a cancellation flag (checked after header receipt, per chunk, and under a commit lock before the final rename) guarantees no checkpoint is ever installed after the caller has raised TimeoutError.
  • Atomic cache writes: stream to a temp file, os.replace() into place — a concurrent reader or mid-download crash never sees a partial file at the cache path.
  • Cross-process download lock: per-cache-key flock so multiple workers/replicas sharing a cache volume download once and the rest reuse the result.
  • Collision-proof cache key: sha256(url)_basename instead of basename alone — two stores serving .../model.pt no longer collide. (Existing old-format cache entries in /tmp/checkpoints are simply ignored and re-fetched once.)

Why

#26 makes MODEL_BASE=https://... a first-class deployment mode, which turns this downloader into the critical path for every cold start on serverless GPU platforms. These failure modes (unbounded startup hang, partial-file reads, duplicate concurrent downloads) go from theoretical to routine in that world. The hardening is equally valid for the URL weights prod already uses.

Testing

Written test-first (TDD): tests/test_checkpoint_utils.py, 11 tests — each hardening behavior had a failing test before the code. Includes a genuinely stalling fake server for the deadline path, a threaded flock contention test for the lock path, and a late-header race test. Full suite: 53 passed.

Provenance

Carved out of #26 as an atomic change. Adversarially reviewed by Codex 5.6 (gpt-5.6-terra) over 5 rounds; it drove the wall-clock deadline (vs naive between-chunk checks), the cross-process lock, the cancellation flag, and the commit lock, and explicitly confirmed convergence: no interleaving permits an install after cancellation.

🤖 Generated with Claude Code

download_checkpoint runs during service startup (and is the mechanism
behind URL-based model weights), so:

- add a (connect, read) socket timeout plus a wall-clock deadline for
  the whole download, so neither a stalled connection nor a
  drip-feeding server can block readiness forever
- stream to a temp file and os.replace() into the cache path so a
  concurrent reader or mid-download crash never sees a partial file
- serialize concurrent downloaders with a per-key flock so multiple
  workers sharing a cache volume fetch once and reuse the result
- key the cache by sha256(url) + basename so two stores serving the
  same filename (e.g. .../model.pt) cannot collide

Written test-first: tests/test_checkpoint_utils.py.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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