feat(client): parallel git snapshot download with bounded in-memory streaming#360
feat(client): parallel git snapshot download with bounded in-memory streaming#360worstell wants to merge 1 commit into
Conversation
worstell
force-pushed
the
worstell/parallel-get-stream
branch
from
4d97fb3 to
83e3e16
Compare
worstell
changed the title
worstell
force-pushed
the
worstell/parallel-get-stream
branch
2 times, most recently
from
250ab5b to
5eaaa77
Compare
worstell
changed the title
worstell
force-pushed
the
worstell/parallel-get-stream
branch
from
5eaaa77 to
4f7efa2
Compare
|
@codex review |
![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: 4f7efa2a0d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
worstell
force-pushed
the
worstell/parallel-get-stream
branch
2 times, most recently
from
e5f6506 to
7b4044b
Compare
|
@codex review |
worstell
force-pushed
the
worstell/parallel-get-stream
branch
from
7b4044b to
882efc2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b4044bbd6
ℹ️ 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".
worstell
force-pushed
the
worstell/parallel-get-stream
branch
from
882efc2 to
011f22c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 011f22ce30
ℹ️ 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".
worstell
force-pushed
the
worstell/parallel-get-stream
branch
from
011f22c to
8097115
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8097115117
ℹ️ 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".
| // Place reads the chunk into its slot and queues it for in-order delivery to | ||
| // Read. It blocks until the chunk is within one window of the read cursor | ||
| // (backpressure from a slow consumer) and aborts if ctx is cancelled. A negative | ||
| // length switches to pass-through mode: the whole body is handed to Read | ||
| // directly, since a single-stream fallback has unknown size and must not be | ||
| // buffered. |
There was a problem hiding this comment.
Why does Amp always ignore the commenting instructions in the AGENTS.md in this repo? These comments are obscenely long.
worstell
force-pushed
the
worstell/parallel-get-stream
branch
2 times, most recently
from
c131050 to
084150b
Compare
worstell
changed the title
worstell
force-pushed
the
worstell/parallel-get-stream
branch
from
084150b to
7d35e51
Compare
…treaming Download git snapshots with up to N concurrent range requests, reassembled in order by a StreamSink and piped straight into zstd/tar extraction so transfer and extraction overlap. Peak buffering is capped at 2*concurrency*chunkSize regardless of snapshot size: fetchers block once they are a full window ahead of the consumer. Chunks are pinned to the discovery ETag so a mid-download rewrite is rejected rather than spliced; range-ignoring backends, missing ETags, and small objects fall back to a single streaming request. Chunk fetches retry transient per-replica failures, and extraction goes to a staging directory renamed into place only on success.
worstell
force-pushed
the
worstell/parallel-get-stream
branch
from
7d35e51 to
310ef7c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 310ef7c416
ℹ️ 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".
| return err | ||
| } | ||
|
|
||
| if err := os.Rename(staging, c.Directory); err != nil { |
There was a problem hiding this comment.
When c.Directory already exists and contains files (for example rerunning cachew git restore ... ./repo over a previous checkout or a partially prepared workspace), this final rename fails with directory not empty instead of completing the restore. The old path extracted with snapshot.Extract(..., c.Directory, ...), which allowed that workflow; if staging is required, the existing tree needs to be removed/replaced intentionally or this should only take the rename path when the destination is absent.
Useful? React with 👍 / 👎.
2 participants
Downloads git snapshots with up to N concurrent range requests (
--download-concurrency, default 8) reassembled in order by an in-memoryStreamSinkthat pipes straight into zstd→tar extraction, so transfer and extraction overlap.Memory is bounded regardless of snapshot size: the sink holds a fixed arena of 2×concurrency chunk buffers, and fetchers block once they are a full window ahead of the consumer, capping peak buffering at 2×concurrency×chunkSize — 256 MiB at the best-observed tuning of 16 MiB chunks × concurrency 8.
Chunks are pinned to the discovery response's ETag, so a mid-download rewrite fails the download rather than splicing bytes from different object versions. Servers without range support, objects without ETags, and small objects fall back transparently to a single streaming request. Extraction goes to a staging directory renamed into place only on success, so a failed download cannot leave a half-written checkout.
End-to-end benchmarking against a real multi-replica deployment showed this in-memory streaming approach consistently outperformed disk-backed reassembly (ring buffer and spill-file variants, which lack backpressure or pay a disk round-trip), restoring large repositories 1.4–2.8× faster than the sequential baseline.