Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Concurrent source replacement can corrupt restored files, and scheduling memory and argument validation need correction.
3 open findings
What changed in this PR
Builds on #620 to improve VolumeCache hydration speed and reliability on Global Store volumes.
Changes:
- Replaces whole-file hydration with configurable parallel range reads and transient-error retries.
- Assembles files in temporary storage before atomic publication.
- Adds regression tests and documents hydration tuning.
| File | Description |
|---|---|
| tests/test_serverless/test_utils/test_rp_volume_cache.py | Tests range copying, retries, cleanup, and configuration. |
| runpod/serverless/utils/rp_volume_cache.py | Implements range-parallel hydration and shared helpers. |
| docs/serverless/volume_cache.md | Documents hydration behavior and tuning options. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Hydrate now splits every big file into 8 MiB ranges and copies the ranges of all files from one pool of 64 threads, each range on its own fds. Object-store mounts (Global Store's geesefs) serve one sequential stream at ~140 MiB/s, and whole-file parallelism doesn't add up. Worse, parallel whole-file copies there fail with ENOMEM, and hydrate dropped those files without retry. Measured on a 4090 pod in EU-RO-1, hydrating a 15.2 GB 7B model from a global volume: before: 9 of 11 files (2 of 4 weight shards lost to ENOMEM), 24-53s after: 11 of 11 files, byte-identical, 17-22s (670-844 MiB/s) ENOMEM/EIO/EAGAIN are retried per range with backoff. Files are assembled in a temp file and renamed only when every range copied, so a failure leaves the local file untouched. A mirror shorter than its manifest entry fails instead of restoring zero padding. Tunable via hydrate_workers and hydrate_chunk_size. Sync is unchanged.
Address review feedback on #621: - Pin each mirror file's size+mtime before its first range and reject the file if a range opens a replaced source, or if the mirror size disagrees with the manifest (a sync that landed after the manifest was read), instead of publishing mixed or truncated bytes. - Generate ranges lazily and pull them from hydrate_workers threads so scheduling stays O(workers) instead of one future per range. - Stop remaining ranges and remove pre-sized temp files when an unexpected (non-OSError) error aborts the hydrate. - Reject non-integer and bool tuning values at construction.
deanq
force-pushed
the
feature/volume-cache-parallel-hydrate
branch
from
October 15, 2026 07:14
0037705 to
51ffc0a
Compare
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Hoist the last attempt out of the retry loop so _with_range_retry has no implicit fall-through return (CodeQL py/mixed-returns). Adds a test pinning the attempt count when transient errors persist.
runpod-flash 1.21.0 emits a FutureWarning on first public-API access, which pytest turns into a conftest ImportError. Set RUNPOD_FLASH_SILENCE_DEPRECATION=1 for the e2e step.
_with_range_retry only wraps _copy_range, which returns None, so stop forwarding its result (CodeQL py/procedure-return-value-used).
This branch has not been deployed
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.
Bug
Even with #620,
VolumeCache.hydrate()on a Global Store volume silently restores only part of a model. Hydrate copies whole big files in parallel withshutil.copyfile, which usessendfileon Linux. Under that load the geesefs mount fails reads withENOMEM._copy_filelogs a warning and drops the file, nothing retries it, and the worker falls back to a cold download.It's also slow when it works. The mount serves one sequential stream at ~140 MiB/s, and reading several whole files at once doesn't add up.
Measured on a 4090 pod in EU-RO-1. A 15.2 GB Qwen2.5-7B was synced to a global volume, then hydrated into an empty dir, alternating implementations, with the page cache dropped between runs:
ENOMEM), both runsdiff -r)Fix
Hydrate splits every big file into
hydrate_chunk_sizeranges (default 8 MiB). One pool ofhydrate_workersthreads (default 64) copies the ranges of all files withpread/pwrite, each range on its own descriptors. Independent range reads scale to the host NIC (~1.1 GiB/s measured), where whole-file copies don't.ENOMEM/EIO/EAGAINwith exponential backoff (8 attempts, ~6s total). Any other error, or exhausted retries, fails the file, and its remaining ranges are skipped.os.replaced into place only after every range has copied, stamped with the manifest's source mtime as in fix(serverless): VolumeCache copies files on volumes that reject utime/chmod #620. A failed file leaves any existing local copy untouched and is not counted as restored.hydrate_workersandhydrate_chunk_sizeare validated likemax_workers.workers × chunk) is the knob that matters. The default is 512 MiB, and that's also the peak hydrate memory. In earlier testbed runs, much past ~1 GiB in flight triggeredENOMEMstorms on busy hosts.sync()is unchanged. Writes to the mount scale with the number of parallel files (~400 MiB/s at 8), and range writes into one geesefs object haven't been validated.mtime_of/mtimeplumbing from_copy_parallel/_copy_file, extracted a_tmp_pathhelper (4 call sites), and added_positive_or_defaultfor constructor validation.docs/serverless/volume_cache.mddocuments the new options and the hydrate path.Tests
14 new tests in
test_rp_volume_cache.py, written first and confirmed failing:ENOMEMis retried and the file restored.make quality-checkpasses: 1492 tests, 91.7% total coverage, andrp_volume_cache.pyis at 97%. The remaining uncovered lines predate this change.