Stream EdgeZero asset responses instead of buffering them#825
Open
aram356 wants to merge 2 commits into
Open
Conversation
The EdgeZero entry point drained every asset/image-optimizer response into the Wasm heap before sending: `dispatch_asset_fallback` ran `buffer_asset_body` (a `BoundedWriter` capped by `publisher.max_buffered_body_bytes`) and re-attached the bytes as a buffered `Body::Once`. This defeated the streaming that `handle_asset_proxy_request` already produces (it requests a streaming origin response and hands back a `Body::Stream`), so large optimized images were fully materialized in memory. The legacy `main.rs` path never buffered — its `HandlerOutcome::AssetStreaming` arm commits headers via `stream_to_client()` and pipes the body chunk-by-chunk with `stream_asset_body`. Wire the same seam into the EdgeZero path: - `dispatch_asset_fallback` now attaches the origin `Body::Stream` to the response (dropping it only for HEAD/204/304, which keep the origin Content-Length and carry no body) instead of buffering. - `edgezero_main` sends through a new `send_core_response` helper that streams `Body::Stream` responses with `stream_to_client()` + `stream_asset_body` and sends buffered `Body::Once` responses in one shot. - Remove the now-dead `buffer_asset_body`/`BoundedWriter` usage and refresh the stale buffering docs. No edgezero dependency change is needed: `edgezero_core::body::Body` already carries a `Stream` variant. Publisher responses remain buffered (unchanged). Adds `dispatch_asset_fallback_streams_origin_body_without_buffering`, which injects a streaming HTTP client and asserts the dispatched response body is `Body::Stream`.
Collaborator
|
tested on staging, didn't get any 502 errors. |
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
2 participants
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.
Summary
On the EdgeZero entry path, asset-route responses (including Fastly Image Optimizer output) were fully read into the Wasm heap before being sent, instead of being streamed to the client. This streams them, matching the legacy path.
handle_asset_proxy_requestalready requests a streaming origin response (with_stream_response) and returns aBody::Stream. The EdgeZero dispatcher was discarding that:dispatch_asset_fallbackdrained the stream viabuffer_asset_body(aBoundedWritercapped bypublisher.max_buffered_body_bytes) and re-attached the bytes as a bufferedBody::Once, thenedgezero_mainsent it through the buffered-onlycompat::to_fastly_response(...).send_to_client(). This was a deliberate interim during the dual-path cutover.The legacy
main.rspath never buffered — itsHandlerOutcome::AssetStreamingarm commits headers withstream_to_client()and pipes the body chunk-by-chunk viastream_asset_body. This PR wires the same seam into the EdgeZero path.Changes
dispatch_asset_fallbackattaches the originBody::Streamto the response instead of buffering. HEAD and bodiless statuses (204, 304) drop the stream so they keep the originContent-Lengthand carry no body.edgezero_mainnow sends through a newsend_core_responsehelper:Body::Streamresponses are committed header-first viastream_to_client()and piped withstream_asset_body; bufferedBody::Onceresponses are sent in one shot. Both send sites use it.buffer_asset_body/BoundedWriterusage and refreshed the stale buffering docs.Publisher (HTML) responses remain buffered — unchanged.
Why no edgezero change
edgezero_core::body::Bodyalready carries aStreamvariant, and the buffering lived entirely intrusted-server-adapter-fastly. The dependency stays pinned toedgezero@branch=main; this is not part of edgezero PR #269 (the CLI-extensions epic), which is unrelated scope.Test plan
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo test --workspace(adapter 107 passed incl. the new test; workspace 1420+ passed, 0 failed)cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1dispatch_asset_fallback_streams_origin_body_without_bufferinginjects a streaming HTTP client and asserts the dispatched asset response body isBody::Stream.Note: the end-to-end
stream_to_client()send seam insend_core_responserequires the Fastly Compute runtime and is covered the same way the legacyAssetStreamingpath is — the chunk-piping primitivestream_asset_bodyhas its own unit test in core (stream_asset_body_reports_mid_stream_origin_errors).Closes #826.
Part of #480.