feat(client): stream parallel downloads to pipes#386
Conversation
ParallelGetStream fetches ranged chunks concurrently and reorders them into a sequential byte stream through an unlinked spill file, so huge git snapshots (5-10GB) can pipe straight into extraction with flat RAM. Consumed spill regions are hole-punched (Linux fallocate, macOS F_PUNCHHOLE) so steady-state disk usage tracks the gap between download and consumption; a cgo-based layout test guards the hand-defined darwin struct against SDK drift. git restore now streams downloads into extraction at every concurrency, overlapping the two and dropping the full-size temp file. Pipeline failures report the root cause (first error wins) rather than closed-pipe or cancellation echoes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
alecthomas
requested review from
inez and
worstell
and removed request for
a team and
inez
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sequential get, ParallelGet, and ParallelGetStream over a 4GiB object (real snapshots run to several GB) with the CLI's default 8MiB chunks, against the disk backend and a MinIO-backed S3 backend. s3clienttest now accepts testing.TB so benchmarks can use it. Chunked gets on the local disk backend are dominated by per-Open overhead (a synchronous setTTL metadata write per chunk) rather than I/O; on S3 parallel range gets beat the sequential read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
alecthomas
force-pushed
the
aat/parallel-get-stream
branch
from
81ae113 to
c9256e2
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: c9256e2b49
ℹ️ 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".
| // after Close. | ||
| func (s *spillBuffer) Close() error { | ||
| err := s.f.Close() | ||
| if rmErr := os.Remove(s.name); rmErr != nil && !errors.Is(rmErr, os.ErrNotExist) { |
There was a problem hiding this comment.
When the initial os.Remove(f.Name()) in newSpillBuffer succeeds (the normal Unix path), s.name is no longer tied to this open file; it is just a pathname that can be reused before Close runs. In a shared spill directory such as /tmp, another process can create an entry at that observed temp name during a long restore, and this unconditional remove would delete that unrelated file or directory. Track whether the initial unlink failed and only remove on close in that fallback case.
Useful? React with 👍 / 👎.
alecthomas
force-pushed
the
aat/parallel-get-stream
branch
2 times, most recently
from
f4dab42 to
c9256e2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9256e2b49
ℹ️ 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".
| // newSnapshotRangeServer serves body as a ranged snapshot endpoint, counting | ||
| // requests. ServeContent honours Range/If-Range against the ETag, so it | ||
| // returns 206 + Content-Range for the chunked requests ParallelGet makes. |
There was a problem hiding this comment.
This newly added helper comment is in a top-level test file, and /workspace/cachew/AGENTS.md:21 says, “NEVER add comments to top-level tests.” Leaving this and the other new top-level test comments in this commit violates the repo’s documented review rules, so please remove the comment rather than documenting test helpers here.
Useful? React with 👍 / 👎.
1 participant
ParallelGetStream fetches ranged chunks concurrently and reorders
them into a sequential byte stream through an unlinked spill file,
so huge git snapshots (5-10GB) can pipe straight into extraction
with flat RAM. Consumed spill regions are hole-punched (Linux
fallocate, macOS F_PUNCHHOLE) so steady-state disk usage tracks the
gap between download and consumption; a cgo-based layout test guards
the hand-defined darwin struct against SDK drift.
git restore now streams downloads into extraction at every
concurrency, overlapping the two and dropping the full-size temp
file. Pipeline failures report the root cause (first error wins)
rather than closed-pipe or cancellation echoes.
Co-Authored-By: Claude Fable 5 noreply@anthropic.com