perf(blobs): stream downloads, single-scan, and in-flight memory budget - #174
Merged
Conversation
Large artifact blobs (>100MB) were fully materialized in RAM at every hop and, with a 1GiB per-request ceiling and no concurrency cap, N concurrent requests could OOM the worker. This bounds and smooths that memory use: - Stream blob downloads as 256KiB chunked frames (refcounted Bytes slices, no per-frame copy) on both worker fetch_rollout_blob and master download_experiment_blob, instead of one Body::from(Vec<u8>) frame. Removes the extra full-blob copy in the send path and lets slow clients backpressure. - Add RolloutStore::get_record_with_blob: one base-first scan returning (record, payload), replacing the master download's get_by_id + get_blob double scan. - Add a process-wide in-flight blob-byte budget (ROLLOUT_MAX_INFLIGHT_BLOB_BYTES, 0=disabled). Uploads reserve Content-Length before buffering; downloads hold the reservation through the streamed send. Over-budget requests get 503 OVERLOADED instead of allocating. Bounds concurrency, not max blob size. - specs/rollout-blob-streaming.md documents the memory model and proposes the larger next step (blob-v2 BlobFile range reads) not implemented here. Tests: BlobBudget admission/release + lone-oversized, chunk-boundary reassembly with reservation release, content-length parsing, get_record_with_blob across WAL/base/miss. fmt + clippy --all-targets clean. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Rollout artifact blobs (
binary_payload) are stored as an inlineLargeBinarycolumn (deliberately — the MemWAL LSM scanner can't materialize blob-v2 columns),
so every blob is fully materialized in RAM at each hop:
part) then copied into an Arrow buffer via
LargeBinaryBuilder::finish().get_blob→take_rowsmaterializes the Arrow buffer, then.to_vec()copies to an ownedVec<u8>(≈2× blob size), thenBody::fromhands the whole thing to the HTTP layer as one frame.
With a 1 GiB per-request ceiling and no concurrency cap, N concurrent large
requests needed ≈
2 × size × Nbytes and could OOM the worker. For >100 MB blobsthis is the real risk under concurrency.
This PR does the three lower-risk mitigations and documents the larger step.
Changes
1. Streaming downloads. Worker
fetch_rollout_bloband masterdownload_experiment_blobnow send the payload as 256 KiB chunked frames(
blob_stream_body) instead of oneBody::from(Vec<u8>). Frames are refcountedBytesslices of one allocation (no per-frame copy); removes the extra full-blobcopy in the send queue and lets a slow client apply backpressure.
Content-Lengthstill set.
2. Single-scan record+blob. New
RolloutStore::get_record_with_blobreturns(record, payload)from one base-first scan, replacing the master download'sget_by_id+get_blobdouble scan over the same shard. Keeps base-first + theNotFound-tolerant WAL fallback.
3. In-flight blob-byte budget. Process-wide
BlobBudget(
ROLLOUT_MAX_INFLIGHT_BLOB_BYTES,0=disabled). Uploads reserveContent-Lengthbefore buffering; downloads hold the reservation through the streamed send.
Over-budget requests get
503 OVERLOADED(+rollout_blob_budget_rejections_totalmetric) instead of allocating. Bounds concurrency, not max blob size — a lone
oversized request is admitted when idle.
4. Proposal only (not implemented):
specs/rollout-blob-streaming.mddocumentsthe memory model and the next step — lance blob-v2 offloaded columns +
BlobFilerange reads for true read-from-storage-in-frames — with the LSM-scanner blocker
and a two-sub-step rollout. Corresponds to a separate future PR.
Tests
BlobBudgetadmission/release + lone-oversized-request behavior.blob_stream_bodychunk-boundary reassembly (byte-for-byte) + reservationreleased when the stream drains.
content_lengthheader parsing/defaulting.get_record_with_blobacross un-merged WAL, merged base, and miss.cargo fmt --all -- --checkandcargo clippy --all-targets -D warningsclean on all three crates.Notes
Vec<u8>fromget_blob— that needs item 4 (range reads). This PR removes the extra copyand caps concurrency.
0); no behavior change unless configured.