fix: utils StreamResponseBody() memory use for large get requests #1089
Conversation
This file contains 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
benmcclelland
force-pushed
the
ben/streaming-get-response
branch
from
e2919d8 to
456414f
Compare
The StreamResponseBody() called ctx.Write() in a loop with a small buffer in an attempt to stream data back to client. But the ctx.Write() was just calling append buffer to the response instead of streaming the data back to the client. The correct way to stream the response back is to use (ctx *fasthttp.RequestCtx).SetBodyStream() to set the body stream reader, and the response will automatically get streamed back using the reader. This will also call Close() on our body since we are providing an io.ReadCloser. Testing this should be done with single large get requests such as aws s3api get-object --bucket bucket --key file /tmp/data for very large objects. The testing shows significantly reduced memory usage for large objects once the streaming is enabled. Fixes #1082
benmcclelland
force-pushed
the
ben/streaming-get-response
branch
from
456414f to
85ba390
Compare
niksis02
approved these changes
Feb 26, 2025
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.
The StreamResponseBody() called ctx.Write() in a loop with a small buffer in an attempt to stream data back to client. But the ctx.Write() was just calling append buffer to the response instead of streaming the data back to the client.
The correct way to stream the response back is to use (ctx *fasthttp.RequestCtx).SetBodyStream() to set the body stream reader, and the response will automatically get streamed back using the reader. This will also call Close() on our body since we are providing an io.ReadCloser.
Testing this should be done with single large get requests such as aws s3api get-object --bucket bucket --key file /tmp/data for very large objects. The testing shows significantly reduced memory usage for large objects once the streaming is enabled.
Fixes #1082