feat(volumecache): adaptive size-bucketed transport#538
Open
deanq wants to merge 11 commits into
Open
Conversation
- 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.
|
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 |
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
1 participant
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%).