Warm checkpoint weights at spawn: fill prewarm_paths from manifest records (#178) - #215
Merged
Conversation
Member
Author
|
Applied fixes from a 10-finding review pass (b51f056). Highlights:
🤖 Generated with Claude Code |
…cords (#178) The spawn-time prewarm covers the env tree, but a checkpoint's model weights load via mmap *after* it ends and fault in at network-round-trip speed — on Delta, uma's 14 GB fairchem cache structurally blew every warm-up timeout its siblings fit (2026-07-29). #177 started recording each checkpoint's weight files in the manifest; this consumes those records at spawn, tiered: 1. Manifest record -> warm exactly those files (cache-root-joined). 2. No record -> best-effort heuristic: shared-cache dirs whose names match a package the env ships ("_"-boundary prefix, cache/orb <-> orb_models) or the env name. The huggingface/torch hub trees are never matched — they hold every family's weights, and over-warming under a memory cgroup evicts the pages the worker needs. 3. Nothing -> today's behavior. :custom ids skip the heuristic (the user's weights already ride checkpoint_path into the prewarm). The lookup lives in spawn_in_env, so every spawn path (calculator, verify, serve, add) gets it; it reads manifest.json raw — never printing migration notes, locking, or refusing newer schemas on the calculator path — and any failure degrades to a spawn without the hint. The prewarm summary now tags the weights tier ("weights: 14.2 GB (manifest)") as field data for retiring the heuristic, and warns up front when the expected cold working set exceeds the job's cgroup memory limit — the eviction footgun from the 2026-07-29 uma timeout, reduced to one visible line. Ships client-side: deployed envs benefit from a client pin bump, no rebuilds. Closes #178. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…honesty Review findings on the #178 implementation, most-severe first: - A weight_files entry with a non-string truthy path ({"path": 5}) raised TypeError outside _recorded_weight_files' fallback net, so the spawn shipped no hint instead of falling back a tier. Entries now require a non-empty str path. - Download spawns ran the fill too, and since a checkpoint's record is only written *after* its download, a first-time `rootstock add` always fell through to the heuristic and cold-read the whole family cache dir (typically on a login node) for weights the download was about to (re)write. Downloads now resolve from the record tier only. - An intact record whose files were all purged (scratch sweeps) won the tier and logged "weights: 0 MB (manifest)" while the load cold-faulted — misattributing exactly the stall the telemetry exists to catch. The manifest tier now requires at least one recorded path to exist (one stat in the common case) and falls through otherwise. - Caller-supplied prewarm_paths were labeled "(custom)" by elimination; they now tag "(caller)" so the tier telemetry can't lie about provenance. - The heuristic's whole-family over-warm (one unrecorded checkpoint warms every cached sibling, e.g. all of cache/mace) is now named in the docstring as the transitional cost it is. - Cleanups: cgroup walk probes only its hierarchy's filename; the summary total uses the same byte formatter as the weights portion; heuristic scan runs string filters before the per-entry stat; "-" is normalized to "_" in dir-name matching. - Tests: wrong-typed path entries; dead/partial record fallbacks; download-spawn tiering; a reader test pinned against a manifest written by the production save path (hand-rolled JSON would stay green across a schema relocation); hardcoded schema versions now use SCHEMA_VERSION; corrected the false claim that smoke-test records :custom weight files (#204 writes only verification records). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
OwenPriceSkelly
force-pushed
the
weights-prewarm-lookup
branch
from
August 14, 2026 15:10
b51f056 to
82dbeb9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Consumes the #177 manifest weight records at spawn time — ladder rung 2 of the cold-start work (#160 → #167/#171 lineage). Closes #178.
What
The spawn-time prewarm covers the env tree, but a checkpoint's model weights load via mmap after it ends and fault in at ~0.35 MB/s on cold Lustre — on Delta, uma's 14 GB fairchem cache structurally blew every warm-up timeout its siblings fit (2026-07-29 field data). This fills
spec["prewarm_paths"](reserved since #171) so the existing prewarm machinery streams the weights too, tiered:{cache_root}/cache,home/.cache,homewhose names match a package the env ships (_-boundary prefix match:cache/orb↔orb_models,home/.dgl↔dgl) or the env's name. Thehuggingface/torchhub trees are never matched — they hold every family's weights side by side, and over-warming under a job's memory cgroup evicts exactly the pages the worker needs. Known transitional cost: the heuristic emits whole per-family dirs, so one unrecorded checkpoint over-warms its cached siblings (all mace ids sharecache/mace) until one add/verify/smoke-test pass records it — the tier tag is the telemetry for retiring it. The manifest tier also requires at least one recorded path to still exist, so a scratch-purged record falls back a tier instead of loggingweights: 0 MB (manifest)while the load cold-faults.:customids skip the heuristic (the user's weights already ridecheckpoint_pathinto the prewarm), but would use a manifest record if one existed under the id (no production path writes one today — smoke-test's custom leg records only verifications).Where
spawn_in_env— the choke point every spawn path (calculator, verify, serve, add) already goes through, and the same seam the node-local staging design (Node-local env staging: pack envs to single images, extract to local disk at spawn #180) will redirect later. A caller-suppliedprewarm_pathsis left alone.manifest.jsonraw, not viaload_manifest: this is the first time the calculator path reads the manifest, and an optimization hint must never print migration notes, take locks, or refuse a manifest written by a newer rootstock. Anything unparseable just falls back a tier; a failed lookup never fails the spawn.Observability
weights: 14.2 GB (manifest)/(heuristic)/weights: none recorded— field data for retiring the heuristic once record coverage is universal, and the line keeps its filesystem-health-probe role.Deployment
Ships client-side (the prewarm module is staged fresh at every spawn): deployed envs benefit from a client pin bump, no env rebuilds. Backfill on an existing install = one
rootstock smoke-testpass to write the records (#177).🤖 Generated with Claude Code