From c0a751072d3f8a047314ab2337b564e65846ef94 Mon Sep 17 00:00:00 2001 From: Eliah Rusin Date: Tue, 6 Oct 2026 04:54:55 +0300 Subject: [PATCH] volume server: apply Range to chunk manifests and forward raw headers when proxying (#11538) * volume server: read GET/HEAD needles off the store lock, and only once The GET/HEAD handler read the needle synchronously on the tokio worker while holding store.read(): first a stream-info read that loaded the whole record just to parse its meta, then, for every needle that was not streamed (small, compressed, chunk manifest, image ops), a second full read. For a tiered volume each read is an S3 GET under the store lock, and a writer queued behind it parks every other store reader. The regular-volume read now runs in spawn_blocking. Under the store guard it only resolves a NeedleReadPlan (index lookup, a freshly opened .dat handle or the remote backend, offset, size); the guard is dropped before any needle data I/O. No data-file lease is held across the read either, since a writer waits for one while holding the store write lock. The index size decides the read, as in Go's readNeedle: a HEAD, a ranged read or a needle above the stream threshold reads only its header and meta tail (ReadNeedleMeta) and hands off to StreamingBody or the range path; everything else is read in full once, with its checksum verified. A compressed or manifest needle found by the meta read is then read in full once. The range-from-source read also moves to spawn_blocking. Co-Authored-By: Claude Opus 5.5 (1M context) * volume server: stream needle chunks without the store lock StreamingBody::poll_frame took store.read() and find_volume for every chunk to compare the volume's compaction revision, dup'd the source handle, and allocated a fresh chunk buffer. With -hasSlowRead=false the stream also holds a data-file read lease for its whole life, while a writer waits for that lease under store.write(): the next chunk's store.read() then waits for the writer and the writer for the stream. The per-chunk re-lookup was also wrong. The stream reads a handle opened at plan time, which pins the .dat inode the offset was resolved against; a vacuum commit renames a new file over .dat and leaves that inode untouched. The re-looked-up offset belongs to the new file but was read from the old inode, so a stream whose needle a vacuum moved ended in a checksum error. The pinned offset stays valid, so the check, and with it every store access, is dropped, along with the now unused re_lookup_needle_data_offset and the revision fields of the read plan. The source is shared as an Arc instead of dup'd per chunk, and the chunk buffer is a BytesMut that the blocking read hands back with its result, so its allocation is reclaimed once the previous frame has been written. Co-Authored-By: Claude Opus 5.5 (1M context) * volume server: split get_or_head_handler_inner into phases get_or_head_handler_inner was a ~650-line function. Its middle resolved the needle and set five mutable flags (stream_info, can_stream, can_handle_head_from_meta, can_handle_range_from_source, bypass_cm) that three if-let reply paths then re-tested, each re-checking stream_info. It is now a 126-line orchestrator over named phases: reject_read_jwt, proxy_missing_volume, wait_for_download_slot, parse_read_request, read_ec_needle / read_volume_needle, etag_and_last_modified, not_modified_response, read_response_headers, and the reply phases stream_response, head_from_meta_response, range_from_source_response, buffered_payload and buffered_response. The read phases return a ReadPlan whose ReadStrategy enum (Stream, HeadFromMeta, RangeFromSource, Buffered) carries the NeedleStreamInfo only on the variants that use it, so the reply is one match instead of three flag checks. Pure refactor: every status code, header and header order, error text, metric increment, lock and data-file lease scope, spawn_blocking boundary and side-effect order is unchanged. Phases that can end the request return ControlFlow. A Range header that is not visible ASCII still falls through to the buffered path, as before. Co-Authored-By: Claude Opus 5.5 (1M context) * volume server: stop a needle stream once its volume becomes unavailable Taking the store lock out of StreamingBody also dropped its per-chunk unavailable_error() check. With -hasSlowRead a writer can take the data-file lease between chunks, fail its fsync and its truncate, and mark the volume unavailable; the stream then kept serving the rest of the needle from its pinned handle. The volume's io_unavailable reason is now an Arc-shared leaf mutex that the read plan hands to the stream. Each chunk checks it under its data-file lease, where the writer marks it, and fails with the same "volume is unavailable: " error the old check returned. Co-Authored-By: Claude Opus 5.5 (1M context) * volume server: read a non-ASCII or empty Range header as Go does A Range value with a byte >= 0x80 (obs-text, which hyper accepts) failed HeaderValue::to_str at both range gates. For a needle served as stored the handler had already chosen a meta-only read, so it fell through to the buffered path with no payload and answered 200 with an empty body; a compressed or EC needle answered 200 with the full body. Go's parseRange fails on a byte it can neither trim nor parse and answers 416 "invalid range", and trims Unicode whitespace such as NBSP into a normal 206. An empty Range value was also a 200 with an empty body, where Go sends the whole payload. Read Range once with from_utf8_lossy, dropping an empty value, and hand that one value to the read plan and to both range gates. A replaced byte never parses, so it is a 416; str::trim trims the same Unicode whitespace as strings.TrimSpace. A range read from the data file now always answers itself instead of falling through with an empty needle. The buffered path answers HEAD before it looks at Range, as Go's writeResponseContent does, so an EC HEAD with a Range is a 200 with the full length. Co-Authored-By: Claude Opus 5.5 (1M context) * volume server: apply Range to chunk manifests and forward raw headers when proxying A GET of a chunk manifest assembled the object and always answered 200 with the whole body, ignoring Range. Go serves the expanded manifest through writeResponseContent, which answers HEAD first and then hands Range to ProcessRangeRequest: 206 for one range, multipart/byteranges for several, 416 for an unsatisfiable or unparsable one. try_expand_chunk_manifest now returns the assembled body and headers, and the caller answers through buffered_response, the same helper the buffered needle path uses. A proxied read forwarded a request header only if HeaderValue::to_str succeeded, so a Range with an obs-text byte was dropped and the target answered 200 with the full body. Go copies every header value as is. Forward the raw HeaderValue for every header. Co-Authored-By: Claude Opus 5.5 (1M context) * volume server: reset Content-Type on range errors as net/http does Go's http.Error sets text/plain and nosniff unconditionally; keeping the needle's MIME type on a 416 mislabels the error body. Match it. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume server: count manifest downloads toward the limit Expanded manifests hard-coded track_download off, so chunked-file reads bypassed in-flight byte accounting while every other path honored the download limiter. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) Co-authored-by: Chris Lu Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- seaweed-volume/src/server/handlers.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/seaweed-volume/src/server/handlers.rs b/seaweed-volume/src/server/handlers.rs index ec01f60e2..b85332dae 100644 --- a/seaweed-volume/src/server/handlers.rs +++ b/seaweed-volume/src/server/handlers.rs @@ -1124,7 +1124,7 @@ async fn get_or_head_handler_inner( &method, data, response_headers, - false, + track_download, ), ControlFlow::Break(resp) => resp, }; @@ -1969,12 +1969,16 @@ fn range_content_range(r: HttpRange, total: i64) -> String { } fn range_error_response(mut headers: HeaderMap, msg: &str) -> Response { - if !headers.contains_key(header::CONTENT_TYPE) { - headers.insert( - header::CONTENT_TYPE, - "text/plain; charset=utf-8".parse().unwrap(), - ); - } + // net/http.Error resets Content-Type and sets nosniff even when a + // caller already set the object's MIME type. + headers.insert( + header::CONTENT_TYPE, + "text/plain; charset=utf-8".parse().unwrap(), + ); + headers.insert( + header::X_CONTENT_TYPE_OPTIONS, + "nosniff".parse().unwrap(), + ); let mut response = Response::new(Body::from(msg.to_string())); *response.status_mut() = StatusCode::RANGE_NOT_SATISFIABLE; *response.headers_mut() = headers;