feat(remote): add single-flight Fetch coalescing (S13 Step 8, part 1/2)

David Fernandezrequested to merge
10io/s13-virtual-remote-foundation-step-8-part1of2 into main

Split into 2 stacked MRs to keep each part within a reviewable size (~600 LoC ideal; both parts land just under the 1000 hard cap): each part targets the previous one (part 1 targets main), and the stack is reviewed and merged bottom-up.

Part 1 of 2 of S13 virtual and remote foundation — Step 8: single-flight coalescing.

📦 What this part delivers

The in-process single-flight from the spec's Single-flight on Fetch section, wrapping the Step 7 Fetch cache-fill pipeline:

  • SingleFlight (internal/remote/singleflight.go) — per-repository coalescing keyed by path (the remote_repository_id half of the spec key is structural: one long-lived instance per remote repository, shared by every flow that fetches from it). The first caller per path leads the upstream GET and cache fill; concurrent callers follow.
  • flight + body wrappers (internal/remote/flight.go) — the flight state machine: raw-fetch followers joining before the leader's first byte tee live from the leader's read-driven fan-out (plain io.Pipe, constant memory); later joiners and every follower of a transformed fetch wait for the commit and read the committed blob back (force-streamed). Leader failure — including an abandoned fill — propagates to both follower models immediately.
  • max_concurrent_fills_per_repo caps leaders only, acquired after the collapse decision so N coalesced callers hold one slot; at the cap a new fill blocks, bounded by the caller's context and single_flight_wait_timeout, surfaced as the new remote.ErrSingleFlightTimeout sentinel.
  • Committed-read serving reuses the leader's Status/ETag/safe headers with a fresh body from BlobStore.OpenBlob(..., WithForceStream()); read-back failures are classified under remote.ErrCacheFill (the upstream response was usable — the failure is cache-side).

This part carries the two anchor tests: the step's acceptance criterion (N concurrent identical Fetches produce exactly one upstream GET and one cache write, and all N receive the streamed body) and distinct-path fill concurrency. The remaining follower-model and orchestration coverage is part 2.

🔍 Notes for reviewers

  • x/sync/singleflight.Group (the npm inline-build precedent) hands every waiter one shared value; followers here each need their own byte stream, so the registry is a hand-rolled keyed map over flight.
  • The flight is keyed by path, so the leader's FetchOptions win for the whole flight; a follower with a different IfNoneMatch can receive an outcome its own validator would not have earned. Documented on SingleFlight.Fetch.
  • The coalescing boundary is a usage contract: whichever step first wires SingleFlight into a composed flow (Step 11/12) must add the end-to-end test proving N concurrent requests through the real flow issue one upstream GET.
  • Beyond the plan's Files list for this step, the change touches errors.go (the new sentinel + an ErrCacheFill doc extension), splits the production code into singleflight.go + flight.go for readability, and extends the fetch_test.go fakeBlobStore with an additive OpenBlob (panic default preserved).
  • Cross-instance coordination for cold multi-GB pulls stays out of scope (work item #375); this layer is in-process by design.

📖 References

🗺️ E2E scenarios

No scenario catalog update: S13 is a format-agnostic library layer with no HTTP endpoints; client-visible e2e scenarios land with the format slices (S14–S16) that compose it.

Related to #329 (closed)

Stacked MRs (review/merge bottom-up)

Edited by David Fernandez

Merge request reports