feat(serverless): network-volume warm cache (VolumeCache) - #531
Conversation
|
Capy auto-review is paused for this organization because the usage-cycle auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
|
Promptless prepared a documentation update related to this change. Triggered by runpod-python PR #531 Documents the new Serverless network-volume warm cache: a new page covering the Review: Document Serverless network-volume warm cache (VolumeCache) |
There was a problem hiding this comment.
Pull request overview
This PR introduces a reusable serverless SDK primitive (VolumeCache) for warming local model/cache directories from a mounted network volume and syncing back deltas as per-worker tar “shards”, then wires it into the serverless worker lifecycle to persist model weights across worker recycling (SLS-367).
Changes:
- Added
runpod.serverless.utils.rp_volume_cache.VolumeCachewithhydrate(),sync(), andwarm()plus built-in helpers (build_default_cache,sync_after_job). - Auto-integrated the cache into the worker loop (
run_workerhydrates/registers) and job execution (run_jobdispatches a one-time async sync after a successful handler run). - Expanded public exports (
runpod.serverless.VolumeCache,runpod.serverless.utils.VolumeCache) and added a comprehensive unit test suite.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_serverless/test_utils/test_rp_volume_cache.py | New unit test suite covering VolumeCache behavior, safety, retention, and worker/job wiring. |
| tests/test_serverless/test_utils_init.py | Updates runpod.serverless.utils.__all__ expectations to include VolumeCache. |
| tests/test_serverless/test_init.py | Updates runpod.serverless.__all__ expectations to include VolumeCache. |
| runpod/serverless/worker.py | Hydrates and registers the default cache before starting job scaling. |
| runpod/serverless/utils/rp_volume_cache.py | New implementation of VolumeCache plus built-in env-driven wiring helpers. |
| runpod/serverless/utils/init.py | Re-exports VolumeCache from runpod.serverless.utils. |
| runpod/serverless/modules/rp_job.py | Dispatches a one-time background cache sync after non-streaming jobs. |
| runpod/serverless/init.py | Re-exports VolumeCache from runpod.serverless. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Addressed review feedback in ede2d5c: Copilot (behavioral):
CodeQL / code-quality:
Tests added for streaming sync, retention resilience, and namespace validation. Full suite green (566 tests, 94% coverage). Still open for a maintainer decision before merge: default-on vs opt-in for the serverless built-in when a volume is mounted (currently on, disabled via |
fbb27d3 to
0883400
Compare
Passing dirs as a bare string/Path (e.g. dirs="/root/.cache") iterated it character by character, putting entries like "/" into _dirs and making sync/hydrate walk far broader paths than intended. Normalize scalar str/bytes/PathLike input to a one-element list. Addresses capy-ai review on PR #531.
The #531 base branch was rebased, orphaning this branch's fork point. Merge #531's current tip so #538 is mergeable again. Resolved the three overlapping files by keeping the adaptive-transport versions and folding in #531's only newer change -- the scalar dirs normalization in __init__ (plus its test).
|
I'm curious how this is more efficient or easier than just simply mounting the volume at that path to begin with? |
…test state Wrap build_default_cache() parse+construct in try/except to prevent malformed RUNPOD_VOLUME_CACHE_MAX_GB from crashing worker startup; degrades to cache-disabled instead. Add sync() no-op to fake cache in test_run_worker_hydrates_registered_cache and wrap test body with reset_builtin_state_for_test() to prevent module-state leakage to subsequent tests (sync_after_job could fire and hit AttributeError).
…e, namespace validation, lint
… open() in assert, unused global)
Passing dirs as a bare string/Path (e.g. dirs="/root/.cache") iterated it character by character, putting entries like "/" into _dirs and making sync/hydrate walk far broader paths than intended. Normalize scalar str/bytes/PathLike input to a one-element list. Addresses capy-ai review on PR #531.
d261f73 to
ce6adfe
Compare
A bytes dirs entry (e.g. dirs=b"/root/.cache") passed os.fspath unchanged, so os.walk yielded bytes names and _is_safe_dest compared bytes against str prefixes, raising TypeError that best-effort mode silently swallowed. Decode each entry with os.fsdecode so all _dirs are text. Add a regression test. Addresses review feedback on #531 (carried here as the superseding PR).
|
Superseded by #538. That PR carries the complete SLS-367 deliverable: the full VolumeCache primitive from this branch plus the adaptive size-bucketed transport, on the current |
…ve transport (#538) * feat(serverless): add VolumeCache skeleton with availability gating * feat(serverless): VolumeCache.sync writes collision-free delta shards * fix(serverless): tolerate coarse network-fs mtime granularity in VolumeCache delta * feat(serverless): VolumeCache.hydrate extracts shards with idempotent marker * fix(serverless): drop 3.12-only tarfile filter kwarg to honor py3.10 floor * fix(serverless): apply tar data filter when available, honoring py3.10 floor * feat(serverless): harden VolumeCache extract against traversal and symlinks * fix(serverless): realpath-normalize cache dirs so symlinked mounts match * feat(serverless): add size-capped retention to VolumeCache * feat(serverless): add best-effort guard and warm() context manager * feat(serverless): export VolumeCache from runpod.serverless * feat(serverless): auto-hydrate model cache from network volume in worker loop * fix(serverless): guard build_default_cache and isolate worker-wiring test state Wrap build_default_cache() parse+construct in try/except to prevent malformed RUNPOD_VOLUME_CACHE_MAX_GB from crashing worker startup; degrades to cache-disabled instead. Add sync() no-op to fake cache in test_run_worker_hydrates_registered_cache and wrap test body with reset_builtin_state_for_test() to prevent module-state leakage to subsequent tests (sync_after_job could fire and hit AttributeError). * fix(serverless): guard cache sync dispatch, sweep temp shards, cover all outputs * fix(serverless): address review - streaming sync, retention resilience, namespace validation, lint * feat(serverless): make network-volume warm cache opt-in; add usage docs * refactor(serverless): VolumeCache mirror+reconcile closure; drop env-var built-in * fix(serverless): bound atexit sync join, unique temp names, exclude temp files * fix(serverless): clear CodeQL alerts (empty-except, unreachable test, open() in assert, unused global) * docs(serverless): add VolumeCache warm-cache handler example * feat(volumecache): add size-partition, max_workers, and packed-format paths * feat(volumecache): add versioned manifest read/write and change detection * feat(volumecache): pack small-file bucket via tar with tarfile fallback * feat(volumecache): extract small bucket via tarfile with per-member safety * fix(volumecache): harden _extract_small against corrupt archives - Remove the unplanned "._" AppleDouble basename skip from the production extractor; it would silently drop legitimate files on a real worker. The macOS bsdtar artifact it worked around is now handled in the affected tests by monkeypatching _tar_binary to force deterministic pure-Python tarfile packing. - Guard tf.getmembers() so a truncated/corrupt small.tar can no longer raise out of _extract_small; return the count extracted so far instead. Add a regression test that truncates a valid archive mid-body and asserts no exception propagates. - Drop the redundant os.path.realpath(dst)/os.makedirs additions in _extract_small; TarFile.extract already creates parent directories and _is_safe_dest already resolves the path internally, matching the brief's structure. * test(volume-cache): target inner getmembers guard in _extract_small Replace archive-truncation test with a mocked tarfile.open whose getmembers() raises tarfile.TarError, deterministically exercising the inner corrupt-member-listing guard instead of the outer open()-time guard (truncation offset unreliably hit the outer path instead). * feat(volumecache): add thread-pool parallel copy helper * feat(volumecache): mirror and hydrate via size-bucketed tar + parallel copy - rewrite _do_sync: pack small files into small.tar, parallel-copy big files into big/, write manifest.json last as the atomic commit marker; fall back to unpacked big copy when tar is unavailable - rewrite _do_hydrate: read manifest, extract small.tar, parallel-copy big entries whose destinations pass _is_safe_dest - suppress BSD tar AppleDouble sidecars via COPYFILE_DISABLE (no-op on GNU tar) - migrate format-coupled sync/hydrate/context-manager tests to manifest-based round-trip assertions * fix(volumecache): gate small-archive extract on manifest + prune stale archive _do_hydrate extracted small.tar unconditionally, so a stale archive left behind by a sync that reclassified small files to big (pack failure) would briefly overwrite fresh data before the big-copy pass caught up, inflating the restored count. Gate the extract on manifest["small"] being non-empty, and remove the stale archive in _do_sync before the manifest is written whenever no small files remain. * docs(volumecache): document size-bucketed adaptive transport Update the module and class docstrings and the volume-cache doc to describe the small.tar + big/ + manifest.json layout (replacing the stale per-file flat-mirror description) and document the max_workers constructor argument. Add a targeted test for the isfile() skip branch in _extract_small to keep coverage at 97%. * fix(volume-cache): harden manifest reads and pack_small cleanup Treat non-dict or unknown-version manifests as absent so sync self-heals instead of raising downstream on unexpected shapes. Move the small.tar listing write inside the temp-cleanup try block so a write failure (e.g. ENOSPC) can't leak the .list temp file. Add coverage for the subprocess-tar failure path and correct the docs' idempotency/limitations claims around manifest.json rewrites and orphaned big/ files. * feat(serverless): add VolumeCache skeleton with availability gating * feat(serverless): VolumeCache.sync writes collision-free delta shards * fix(serverless): tolerate coarse network-fs mtime granularity in VolumeCache delta * feat(serverless): VolumeCache.hydrate extracts shards with idempotent marker * fix(serverless): drop 3.12-only tarfile filter kwarg to honor py3.10 floor * fix(serverless): apply tar data filter when available, honoring py3.10 floor * feat(serverless): harden VolumeCache extract against traversal and symlinks * fix(serverless): realpath-normalize cache dirs so symlinked mounts match * feat(serverless): add size-capped retention to VolumeCache * feat(serverless): add best-effort guard and warm() context manager * feat(serverless): export VolumeCache from runpod.serverless * feat(serverless): auto-hydrate model cache from network volume in worker loop * fix(serverless): guard build_default_cache and isolate worker-wiring test state Wrap build_default_cache() parse+construct in try/except to prevent malformed RUNPOD_VOLUME_CACHE_MAX_GB from crashing worker startup; degrades to cache-disabled instead. Add sync() no-op to fake cache in test_run_worker_hydrates_registered_cache and wrap test body with reset_builtin_state_for_test() to prevent module-state leakage to subsequent tests (sync_after_job could fire and hit AttributeError). * fix(serverless): guard cache sync dispatch, sweep temp shards, cover all outputs * fix(serverless): address review - streaming sync, retention resilience, namespace validation, lint * feat(serverless): make network-volume warm cache opt-in; add usage docs * refactor(serverless): VolumeCache mirror+reconcile closure; drop env-var built-in * fix(serverless): bound atexit sync join, unique temp names, exclude temp files * fix(serverless): clear CodeQL alerts (empty-except, unreachable test, open() in assert, unused global) * docs(serverless): add VolumeCache warm-cache handler example * fix(volumecache): repack small archive when a small file is deleted _changed_vs_manifest reports only additions/modifications, so a small file deleted locally while others remained left a stale small.tar that _extract_small (member-by-member) would resurrect on hydrate. Detect deletions against the prior manifest and repack the archive to the current small set. Addresses capy-ai review on PR #538. * fix(volumecache): treat a scalar dirs path as a single directory Passing dirs as a bare string/Path (e.g. dirs="/root/.cache") iterated it character by character, putting entries like "/" into _dirs and making sync/hydrate walk far broader paths than intended. Normalize scalar str/bytes/PathLike input to a one-element list. Addresses capy-ai review on PR #531. * refactor(volumecache): stream copy count and confine hydrate source to big/ Addresses Copilot review on PR #538: - _copy_parallel streams the success count from pool.map instead of materializing a per-file results list for large trees. - _do_hydrate confines each big-bucket source path inside big/ (new _within guard) so a crafted manifest entry can't read outside the mirror; the destination was already guarded by _is_safe_dest. * fix(volumecache): normalize bytes dirs to text paths A bytes dirs entry (e.g. dirs=b"/root/.cache") passed os.fspath unchanged, so os.walk yielded bytes names and _is_safe_dest compared bytes against str prefixes, raising TypeError that best-effort mode silently swallowed. Decode each entry with os.fsdecode so all _dirs are text. Add a regression test. Addresses review feedback on #531 (carried here as the superseding PR). * fix(serverless): omit failed big-file copies from VolumeCache manifest (SLS-367) _copy_parallel now returns (copied, current) source-path sets so callers can tell which files actually reached the volume. _do_sync records big_meta only for the copied|current union, so a big file whose copy failed is no longer written to the manifest as present -- preventing a later cold hydrate from serving stale volume bytes for it. * fix(serverless): validate VolumeCache volume_path and max_workers - normalize volume_path with os.fsdecode like dirs, so a bytes path can't make _mirror_root mix bytes/str and raise a TypeError that best-effort mode swallows (silently disabling the cache) - reject non-positive max_workers at construction instead of surfacing later as a swallowed ThreadPoolExecutor ValueError - tests: bytes volume_path normalization; max_workers in {0, -1, -8}
Summary
Adds
VolumeCache, a small explicit primitive that warms local directories (e.g. a model cache) across serverless workers using a mounted network volume. It turns a repeated multi-GB model download on every cold start into a one-time cost per endpoint. Stdlib-only, best-effort: any failure degrades to a cold worker and never raises into your handler.Interface — a closure
__enter__→hydrate(): reconcile volume → container (copy files missing/newer in the container).__exit__→sync(background=True): reconcile container → volume on a daemon thread so your code returns immediately; anatexitjoin (bounded by a timeout) completes it before the process ends.How it works
{volume}/.cache/{namespace}/…(namespacedefaults toRUNPOD_ENDPOINT_ID). No opaque archives, no accumulation.copy2to a unique temp +os.replace). Concurrent workers are last-writer-wins with no torn files.namespacemust be a single safe path component.Notes
>=3.10floor.withblock where it fits. There is no automatic worker-loop wiring or environment-variable toggle — an earlier draft had a built-in; it was removed in favor of the single explicit primitive. Auto-wiring can be layered on later if desired.Test plan
tests/test_serverless/test_utils/test_rp_volume_cache.py: availability gating, namespace validation, sync→hydrate round-trip, reconcile idempotence (unchanged files not recopied), newer-file protection, path exclusions, symlink skip, hydrate destination-containment (escape attempt blocked), context-manager enter/exit (+ exception propagation), background completion via the atexit join (bounded), temp-file exclusion, and best-effort swallow vs re-raise.tests/test_serverlesssuite green; repomake quality-checkpasses (557 tests, ~94% overall, ~98% on the new module).Docs
docs/serverless/volume_cache.mdand a short README section under Serverless.