feat(volumecache): adaptive size-bucketed transport#538
Open
deanq wants to merge 34 commits into
Open
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 Bot
added a commit
to runpod/docs
that referenced
this pull request
Jul 10, 2026
…docs Folds in runpod/runpod-python#538 (SLS-367): documents the additive max_workers constructor argument and the size-bucketed transport (small.tar + parallel big/ copy + versioned manifest). Also reconciles the page to the shipping VolumeCache API: the earlier env-var toggles (RUNPOD_VOLUME_CACHE/_MAX_GB/CACHE_DIRS), the warm() method, max_size_gb, and automatic worker-loop wiring were removed upstream in favor of the single explicit opt-in context manager.
|
Promptless prepared a documentation update related to this change. Triggered by PR #538 (adaptive size-bucketed transport) Docs handled via the existing VolumeCache draft (docs PR #693) rather than a new PR. Folded in this PR's user-facing delta — the additive Review: docs PR #693 |
…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)
- 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.
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).
…l 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
…e 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.
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%.
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.
deanq
force-pushed
the
deanq/sls-367-volumecache-adaptive-transport
branch
from
e3cf45c to
c242dd2
Compare
deanq
force-pushed
the
deanquinanola/sls-367-network-volume-warm-cache-for-serverless-volumecache
branch
from
fbb27d3 to
0883400
Compare
![capy-ai[bot]](https://avatars.githubusercontent.com/in/1915919?s=60&v=4){kind=small}
_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.
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).
There was a problem hiding this comment.
Pull request overview
Introduces an adaptive transport strategy for VolumeCache to improve warm-cache performance on network volumes by packing many small files into a single archive while copying larger files in parallel, keeping the public API (hydrate/sync/context manager) intact and adding max_workers.
Changes:
- Added
VolumeCacheimplementation with size-bucketed transport (small.tar+big/) and an atomicmanifest.jsoncommit marker. - Exported
VolumeCacheviarunpod.serverlessandrunpod.serverless.utils, and documented usage with an example handler. - Added a comprehensive test suite covering transport selection, atomicity, safety, and best-effort behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_serverless/test_utils/test_rp_volume_cache.py | Adds extensive unit coverage for the new adaptive-transport VolumeCache behavior. |
| tests/test_serverless/test_utils_init.py | Updates expected runpod.serverless.utils.__all__ to include VolumeCache. |
| tests/test_serverless/test_modules/test_job.py | Minor whitespace-only change. |
| tests/test_serverless/test_init.py | Updates expected runpod.serverless.__all__ to include VolumeCache. |
| runpod/serverless/utils/rp_volume_cache.py | Implements the adaptive, size-bucketed VolumeCache (archive + parallel copy + manifest). |
| runpod/serverless/utils/init.py | Re-exports VolumeCache from the utils package. |
| runpod/serverless/init.py | Re-exports VolumeCache from the serverless package. |
| README.md | Adds a short “Network-Volume Warm Cache” section pointing to docs. |
| examples/serverless/volume_cache_handler.py | Adds an example serverless handler demonstrating VolumeCache usage. |
| docs/serverless/volume_cache.md | Adds full documentation for configuration, design, and limitations. |
Comments suppressed due to low confidence (2)
runpod/serverless/utils/rp_volume_cache.py:93
max_workersis accepted without validation; passing 0 or a negative value will raise fromThreadPoolExecutorat runtime. Validate early and raise a clearValueErrorduring construction.
):
raise ValueError(
f"namespace must be a single safe path component, got {self._namespace!r}"
tests/test_serverless/test_utils/test_rp_volume_cache.py:346
- This assertion doesn't actually verify temp-file cleanup:
_copy_fileuses a PID+thread-suffixed temp name (dst.<pid>.<tid>.rpvc.tmp), so checking onlydst.bin.rpvc.tmpwill pass even if the real temp file is left behind. Use a glob over the expected pattern.
def test_do_sync_skips_missing_dirs(tmp_path):
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…o 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.
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.
Summary
Follow-up to #531. Replaces VolumeCache's serial per-file transport with a size-bucketed hybrid, chosen from an empirical benchmark on Runpod serverless (MooseFS network volume).
small.taron the volume (collapses per-file metadata round-trips).big/(incremental size/mtime diff; large-file subtree stays browsable).manifest.json, written last, is the atomic commit marker. Extraction always uses stdlibtarfilewith per-member path-traversal defense and per-member incremental skip.hydrate/sync/context manager); additivemax_workers.Why
Benchmark across file-shape quadrants, mirror direction (local -> volume):
Serial (today's VolumeCache) never wins and is catastrophic on many-small-file caches: 72x slower than tar and 10x slower than parallel at 40k files. Large files favor parallel (tar's single stream is dead weight). No single strategy wins everywhere, so transport adapts to mean file size.
Design
tarbinary with atarfilefallback; if both are unavailable, small files reclassify to the unpacked big bucket (no hard failure).os.replace; the manifest rename is the linearization point (write-once/read-many cache lifecycle).best_effort=False. An unknown/corrupt/future-version manifest is treated as absent so sync self-heals.Stacking
Stacked on #531 (base is its branch), so this diff shows only the adaptive-transport delta. Retarget to
mainonce #531 merges.Linear: SLS-367
Test plan
make quality-check: 586 passing,rp_volume_cache.pycoverage 97% (repo floor 90%).