perf(cache): parallel sub-range requests for large S3 ranged reads#387
perf(cache): parallel sub-range requests for large S3 ranged reads#387worstell wants to merge 1 commit into
Conversation
worstell
changed the title
worstell
force-pushed
the
worstell/s3-parallel-ranged-reads
branch
from
07ef9f7 to
3ade69d
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ade69dd3a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Read one byte then stall. The workers must park once the admission | ||
| // window (2 × concurrency fetched-but-unwritten chunks) fills, rather than | ||
| // racing ahead and buffering the remaining chunks in RAM. |
There was a problem hiding this comment.
This new inline explanation is inside a top-level test function, but /workspace/cachew/AGENTS.md explicitly says, "NEVER add comments to top-level tests," and only permits comments for non-obvious larger code blocks. Please remove this and the analogous explanatory comments in the new tests, or make the intent clear through subtest/helper names instead.
Useful? React with 👍 / 👎.
Full-object S3 reads already fan out across parallel range GETs, but a ranged read was served as one single-stream S3 GET capped at a fraction of available bandwidth. Clients issuing ranged reads against an object that must be served from S3 therefore paid a structural penalty per chunk. Large ranges now split into parallel sub-range requests, scaling the part size down (to a 4MiB floor) so ranges smaller than one configured part still fan out. Small ranges keep the single-stream path, and all sub-range requests remain etag-pinned. Parallel downloads also now bound run-ahead: a token window caps fetched-but-unwritten chunks at 2 x workers, so a consumer slower than the aggregate download rate applies backpressure instead of letting chunks pile up in RAM. Peak memory is 2 x DownloadConcurrency x part size regardless of object size.
worstell
force-pushed
the
worstell/s3-parallel-ranged-reads
branch
from
3ade69d to
0ba43f7
Compare
|
I actually have no idea what a "sub-range" request is, as opposed to just a range request, but instead of this, can we replace all the internal S3 parallelisation with our existing range reader machinery? That was what I was planning to do after getting all the other range reading working. ie. delete the internal S3 parallelisation and instead use ParallelGet in the Tiered backend for syncing the lower tiers |
2 participants
Full-object S3 reads already fan out across parallel range GETs, but a ranged read was served as a single-stream S3 GET capped at a fraction of available bandwidth. Chunked clients (e.g. the parallel git-snapshot restore) issuing ranged reads against an object that must be served from S3 therefore paid a structural penalty per chunk.
Large ranges now split into parallel sub-range requests, scaling the part size down (to a 4MiB floor) so ranges smaller than one configured download part still fan out across multiple streams. Small ranges (<8MiB) keep the single-stream path. All sub-range requests remain pinned to the same object revision via ETag.
Parallel downloads also now bound run-ahead: a token window caps fetched-but-unwritten chunks at 2×workers, so a consumer slower than the aggregate download rate applies backpressure to the workers instead of letting chunks pile up in RAM. Peak memory is 2 × download-concurrency × part size regardless of object size.
In end-to-end testing of cold-cache restores of a large repository, this cut restore time by ~38% and raised per-serve bandwidth ~2.7×.