feat(cache): add byte-range support to Cache.Open#343
Conversation
Change the Cache.Open signature to Open(ctx, key, start, end) with half-open [start, end) semantics; end == -1 reads to EOF, so a full read is Open(ctx, key, 0, -1). Out-of-range starts return ErrRangeNotSatisfiable and over-long ends are clamped, mirroring HTTP. Backends: memory slices the buffer, disk uses io.NewSectionReader (still returning the raw *os.File for full reads to keep the ServeContent sendfile path), and S3 issues a single ranged GET, replacing the parallel range-get reader (its download-* tuning options are now ignored and warn if set, pending a higher-level downloader). Plumb ranges over HTTP: the client sends a Range header, Remote.Open maps 206/416, and apiv1 serves 206 Partial Content via new httputil.ParseByteRange/ServeCachePartial, evaluating preconditions before the range per RFC 9110 §13.2.2. Rework tiered backfill: a generation-keyed backfillManager dedupes concurrent backfills per (namespace, key) and lets a Create cancel an in-flight one. Full reads keep the tee-backfill but now commit only on EOF (early close or mid-stream error discards the fragment); partial reads trigger a background full backfill so a fragment is never stored as the whole object. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
alecthomas
force-pushed
the
aat/open-range
branch
from
264a7f9 to
481ea97
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: 481ea97889
ℹ️ 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".
Address PR review feedback: - apiv1 now evaluates If-Range: a stale validator serves the full current object (200) instead of a 206 of the new object, so a resuming client cannot append fresh bytes onto a stale prefix (RFC 9110 §13.1.5). - resolveRange returns ErrRangeNotSatisfiable for any non-full range whose start is at or beyond the object size, including non-full ranges on empty objects, so local backends agree with the HTTP path (which returns 416 for bytes=0-0 on a zero-byte object). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc7fa1b216
ℹ️ 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".
| return | ||
| } | ||
|
|
||
| if err := httputil.ServeCachePartial(w, r, headers, cr, start, end, size); err != nil { |
There was a problem hiding this comment.
When a key is overwritten between Stat and Open, Open resolves/clamps the requested range against the new object but this response still advertises the start, end, and size computed from the old stat. For example, if an object shrinks from 100 to 50 bytes while handling Range: bytes=40-99, the body contains only bytes 40-49 while Content-Range says bytes 40-99/100, which breaks resumable clients that trust the advertised range. Retry/re-resolve when the opened representation differs from the stat result, or derive the advertised range from the opened object.
Useful? React with 👍 / 👎.
| // object. It is deduplicated and cancellable via the backfill manager. The | ||
| // background work uses a context detached from the request so it outlives it. | ||
| func (t Tiered) startBackgroundBackfill(ctx context.Context, key Key, src, dst Cache) { | ||
| bgCtx, cancel := context.WithCancel(context.WithoutCancel(ctx)) |
There was a problem hiding this comment.
For partial range hits this detaches the full-object backfill from the request, but Tiered.Close only closes the underlying caches and never cancels or waits for these goroutines. If the cache is closed right after a range response while the backfill is still copying, the goroutine can continue writing into a closed backend (for example Memory.Close nils its map, and disk closes its metadata DB), causing panics/errors during shutdown. Track these background backfills and cancel/wait for them before closing the tiers.
Useful? React with 👍 / 👎.
1 participant
Change the Cache.Open signature to Open(ctx, key, start, end) with
half-open [start, end) semantics; end == -1 reads to EOF, so a full
read is Open(ctx, key, 0, -1). Out-of-range starts return
ErrRangeNotSatisfiable and over-long ends are clamped, mirroring HTTP.
Backends: memory slices the buffer, disk uses io.NewSectionReader
(still returning the raw os.File for full reads to keep the
ServeContent sendfile path), and S3 issues a single ranged GET,
replacing the parallel range-get reader (its download- tuning options
are now ignored and warn if set, pending a higher-level downloader).
Plumb ranges over HTTP: the client sends a Range header, Remote.Open
maps 206/416, and apiv1 serves 206 Partial Content via new
httputil.ParseByteRange/ServeCachePartial, evaluating preconditions
before the range per RFC 9110 §13.2.2.
Rework tiered backfill: a generation-keyed backfillManager dedupes
concurrent backfills per (namespace, key) and lets a Create cancel an
in-flight one. Full reads keep the tee-backfill but now commit only on
EOF (early close or mid-stream error discards the fragment); partial
reads trigger a background full backfill so a fragment is never stored
as the whole object.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
🤖 Generated with Claude Code