Commit Graph
5 Commits
Author SHA1 Message Date
Chris LuandClaude Opus 5.5 a1a208506e volume: serve a ranged GET without reading the whole needle (#12301)
* volume: serve a ranged GET without reading the whole needle

A ranged GET read every needle up to 1MB in full, checksum and all,
then copied out the requested bytes. Read the metadata and stream only
the range instead, as the Go server already does above 1MB and the Rust
server does at any size. A small range of a 790KB needle goes from
~340us to ~160us.

The metadata read stays non-terminal: the range read that follows
reports the io outcome, so a failing disk still builds its error streak.
HEAD, compressed needles and chunk manifests keep their current reads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK

* volume: read the whole needle for a range that may need an image transform

Without a URL extension, the stored name can still make a ranged GET
with width, height or crop parameters an image transform, which needs
the payload. Keep the full read there instead of streaming the range.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK

* volume: stream a range from the record its metadata came from

The streamed read looked the needle up again, so a replacement written
after the metadata read sent new bytes under the old length, checksum
and headers. Read the record the metadata read resolved, as the Rust
server does, and after a compaction refuse a record whose size changed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK

* rust volume: read the whole needle for a range that may need an image transform

A ranged GET with no URL extension but width, height or crop parameters
took the metadata-first path, so a stored image name never got its
transform. Read a small needle whole there, as the Go server does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK

* volume: keep later parts of a multi-range reply on a moved record

When a compaction moved the record before one part of a multi-range
reply, that part re-resolved it, but the next part went back to the
offset the metadata read saw. Remember the re-resolved offset too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK

* volume: check a compaction-moved record is the one the metadata came from

After a compaction, a range re-resolved the needle and accepted any record
of the same size, so a replacement written in between could send its
bytes under the old headers. Compare the moved record's checksum and
append time with the metadata already read, and fail otherwise.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK

* volume: accept a re-encoded checksum and count read errors on a moved record

Compaction rewrites a pre-3.09 checksum in the current encoding, so the
moved-record check now accepts either. A failed read of the moved
record's metadata now feeds the io-error streak and surfaces as that
error instead of not-found. The pinned record is a fresh value rather
than a write through the index's pointer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK

* volume: keep a moved record's wrapped offset when a range follows it

With 4-byte offsets, a record past the 32GB wrap is read at offset plus
MaxPossibleVolumeSize. The compaction refresh dropped that, so it checked
and streamed the wrong location. Retry the wrapped offset in the moved
record check, as the metadata read does, and apply it to the stream.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-11-01 12:00:22 -08:00
a235473105 [Volume] Preserve request semantics in replica proxy reads (#12075)
* fix 12074

* volume: shuffle a copy of cached replica locations for proxy reads

LookupVolumeId serves cached location slices shared between lookups; shuffling in place races with concurrent proxied reads of the same volume.

* seaweed-volume: forward inbound method and surface lookup failure in proxy reads

Mirror the Go handler: proxy_request used to always send GET, turning a client HEAD into a replica download, and a failed master lookup returned 404 like missing data. Forward the inbound method and answer 500 for lookup failures in proxy mode.

* volume: keep master lookup error detail out of the proxy error response

* volume: bound the proxied-request wait in the method-preservation test

* seaweed-volume: answer 404 for missing volumes in proxy-mode lookups

Mirror Go's LookupVolumeId: a per-volume error in the master's lookup
reply (e.g. a missing volume) means "no locations" and answers 404
downstream, not a lookup failure that proxy mode answers 500.

* volume: guard nil lookup result in proxy path

A master response that omits the requested vid leaves
LookupVolumeId returning a nil *LookupResult with a nil error;
dereferencing it panics the read handler.

* seaweed-volume: answered lookup errors are missing data, not failures

A master reply carrying a per-volume error with HTTP 404 (volume not
found) now yields empty locations like Go's LookupResult, so proxy mode
answers 404. Other failure statuses such as 503 during master warmup
remain lookup failures that proxy mode answers 500.

* seaweed-volume: drop hop-by-hop headers from replica proxy reads

Go's inbound request drops Host, Content-Length and Transfer-Encoding
into dedicated fields, so its forwarded header set never carries them.
hyper keeps them; forwarding a client's Content-Length with no body
stalls the replica's read, and forwarding its Host misnames the target.

* volume: bound the proxy master lookup to the request context

* volume: drop hop-by-hop and Connection-nominated headers from proxy reads

* seaweed-volume: drop Connection-nominated headers from proxy reads

* volume: skip the proxy lookup error when the inbound request is gone

* volume: drop proxy-specific headers from replica proxy reads

* seaweed-volume: drop proxy-specific headers from replica proxy reads

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* volume: drop hop-by-hop headers from replica proxy responses

* seaweed-volume: drop hop-by-hop headers from replica proxy responses

* seaweed-volume: a failed lookup status without an error body is still a failure

* volume: do not re-proxy an already-proxied read for a missing volume

A proxied request that reaches a server which still lacks the volume was
looked up and forwarded again, so a stale location answer could bounce
the read between two servers until the client gave up. The Rust volume
server's proxy_missing_volume already returns 404 for proxied=true;
mirror it here.

* seaweed-volume: replace stale proxied params in the forwarded query

Go builds the proxied URL with query.Set(reqIsProxied), which replaces
every proxied param the client sent. Appending after a client-supplied
proxied=false left the replica's first-value lookup reading the stale
marker, defeating the loop guard.

---------

Co-authored-by: chrislusf <chrislusf@users.noreply.github.com>
Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
2026-10-26 08:13:34 -07:00
8841eb6c70 volume: check If-None-Match before If-Modified-Since and match ETag lists (#12027)
* filer: match If-Match and If-None-Match against "*" and ETag lists

checkPreconditions compared the whole header as one ETag, so "*", a
list of ETags and a weak W/ tag in If-None-Match never matched
(RFC 9110 13.1.1, 13.1.2).

* volume: check If-None-Match before If-Modified-Since and match ETag lists

A volume GET answered 304 from If-Modified-Since even when If-None-Match
named a different ETag, and compared If-None-Match as one exact string,
so "*", lists and weak tags never matched (RFC 9110 13.1.2, 13.2.2).

* filer: split etag lists only outside quoted tags

A comma inside a quoted opaque tag is a legal etagc, not a list
separator; strings.Split cut such tags into fragments that
CanonicalizeETag could turn into a false match.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* filer: match repeated If-Match and If-None-Match header lines

Header.Get reads only the first field line, so a match on a later
repeated header was missed; join all field values like any other
comma-separated list.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* volume: match repeated If-None-Match header lines

Header.Get reads only the first field line; join all field values so a
match on a later repeated header is seen. Also covers a comma inside a
quoted opaque tag.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* volume: send the etag on a 304 like the filer does

RFC 9110 15.4.5 lists ETag among the fields a 304 must carry when a 200
would send it; SetEtag now runs before the notModified check.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* volume: mirror the conditional-GET fixes in the rust server

not_modified_response now checks If-None-Match before If-Modified-Since,
matches "*", weak tags, quoted commas and repeated field lines through
the same list rules, and sends the etag on a 304.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* volume: treat an empty If-None-Match like the Go server does

* filer: ignore empty elements in etag precondition lists

* volume: ignore empty elements in etag lists like the filer

* filer: let a present-but-empty precondition line suppress the date check

* volume: gate If-Modified-Since on a present If-None-Match line

* volume: gate If-Modified-Since on header presence like the Go server

---------

Co-authored-by: user.email <chrislusf@users.noreply.github.com>
Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: chrislusf <chris.lu@gmail.com>
2026-10-24 07:05:29 -07:00
df33e81915 volume: report the needle read error with a failed read (#11934)
* volume: report the needle read error with a failed read

A read that fails on the volume server, for example on a CRC mismatch,
returned a bare 500, so mount and filer logs only showed the status.
The volume server now writes the error into the response body, and the
read helpers add it to the error they return.

Co-authored-by: Bruce <82158894+half-life666@users.noreply.github.com>

* volume: keep the 500 bare on HEAD requests

writeJsonError's body write fails with ErrBodyNotAllowed under net/http
when the request method is HEAD, adding an error-writing log line on top
of the failed read. Send only the status, matching the no-error path.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* util/http: decode gzip-encoded error bodies in statusError

Full-chunk reads set Accept-Encoding: gzip themselves, so the
transport does not transparently decompress; a gzip-encoded error
body reached statusError as compressed bytes and produced an
unreadable message. statusError now takes the response and decodes
Content-Encoding before extracting the bounded message.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* volume: log the read error when the 500 stays bare

On HEAD requests the body write is skipped, which also skipped the
V(0) response log inside writeJson — a failed HEAD read left no
cause in normal volume-server logs. Log the error in the same
format before the bare 500.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* util/http: check the status before decoding gzip in GetAuthenticated

The gzip reader eagerly consumes the header when it is created, so
statusError saw a drained or corrupt body on a gzipped error response
and the server message was lost. Match the other read helpers and check
the status first.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* util/http: export StatusError

Other volume read paths outside this file need the same error body.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* volume: redact jwt query param when logging failed-read URLs

The new read-failure logs print the request URL, and volume reads accept
?jwt= as a credential, so a failed authenticated read would write the
bearer token to the volume server log.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* util/http: surface the error body on the remaining chunk-read paths

* volume: redact duplicate jwt params in failed-read logs

* operation: pass the lookup jwt on the chunk needle read

---------

Co-authored-by: Bruce <82158894+half-life666@users.noreply.github.com>
Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>
2026-10-21 20:24:48 -07:00
acb44a68a9 fix(volume): skip unnecessary needle data reads on delete (#11711)
* fix(volume): do not read a needle's data to delete it

A delete read the needle's whole payload from disk only to discard it.
The delete handler and BatchDelete need the needle's cookie and its
chunk-manifest flag, which live in the header and trailer, but fetched
them with a full needle read, the same call a GET uses.

readNeedle now reads the header and trailer only when MustMetaOnly is
set, whatever the needle's size, and the delete paths ask for that. The
Rust volume server honours must_meta_only the same way for its delete
paths. A compressed needle or a chunk manifest still gets the full read,
as before, because their callers need the bytes.

HEAD is unchanged. It used to set MustMetaOnly, which nothing read; it
now asks for AttemptMetaOnly alone, so it keeps reading a needle up to
PagedReadLimit whole. On a spinning disk that one read is cheaper than a
header read followed by a trailer read.

Version 1 needles are always read in full. They have no data size field
and no metadata after the data, which ReadNeedleMeta relies on, so a
metadata-only read of one failed with "invalid needle meta size". That
already happened for version 1 needles above PagedReadLimit read with
AttemptMetaOnly.

A delete therefore no longer verifies the data CRC of the needle it
removes. The cookie check is unchanged.

* fix(volume): read transformed images fully on HEAD too

A HEAD carrying resize or crop params reports the transformed length,
which a metadata read cannot produce. Go and Rust both answered such
HEADs from the source header, returning the stored size instead of the
cropped or resized one.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

---------

Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>
Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
2026-10-12 18:08:19 +08:00