perf(cache): parallelize large S3 range reads#387
Conversation
worstell
changed the title
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".
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 |
0ba43f7 to
e6cb5f6
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6cb5f6ae0
ℹ️ 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".
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.
e6cb5f6 to
bb09887
Compare
worstell
changed the title
| @@ -0,0 +1,135 @@ | |||
| // Command lint-test-comments enforces the AGENTS.md rule that top-level tests | |||
There was a problem hiding this comment.
I don't know how useful this is in practice. I wonder if we should just rely on codex? ie. comments can still be completely useless, even if they're in the right location
2 participants
Full-object S3 reads already fan out across parallel range GETs, but a ranged read was served as a single S3 stream 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 ranged reads now fan out too: the requested range is split into parts fetched concurrently, scaling the part size down (to a 4MiB floor) so ranges smaller than one configured download part still use multiple streams. Small ranges (<8MiB) keep the single-stream path. All requests are pinned to the same object revision via ETag.
There is no S3-internal parallelisation engine: both the full-object and ranged paths drive the existing
client.ParallelGetmachinery through a smallRangeReaderadapter that maps window-relative ranges onto pinned S3 range GETs. This reuses ParallelGet's dispatch gating and bounded stream reassembly, 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 bounded at 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×.