Implement large file streaming for proxy GET#30
Open
jdcasey wants to merge 1 commit intoCommonjava:blackboxfrom
Open
Implement large file streaming for proxy GET#30jdcasey wants to merge 1 commit intoCommonjava:blackboxfrom
jdcasey wants to merge 1 commit intoCommonjava:blackboxfrom
Conversation
NOTE: This is a backport of the attempt to implement file streaming on the archive-enabled version of this service. I've cut out digest handling and tracking record setup to make it work in the older codebase. It's based on commit: 989c6c1.
ligangty
reviewed
Nov 9, 2021
| int transferred = 0; | ||
| while ( transferred < total ) | ||
| { | ||
| int next = bufSize < total ? bufSize : total; |
Member
There was a problem hiding this comment.
Maybe this can be moved out of the while loop as bufSize and total will never change? And maybe Math.min(bufSize, total) is more readable?
| byte[] bytes = buffer.getBytes( transferred, next ); | ||
| out.write( bytes ); | ||
|
|
||
| transferred = next; |
Member
There was a problem hiding this comment.
I'm thinking the quit condition here for the while loop. There are two cases:
- For small files whose size < bufSize, so there will be only one loop and can quit successfully.
- For large files whose size > bufSize. In this case, the "total" will be the real file size(from the buffer.length), right? So that means the transferred will always be set to bufSize because of the "bufSize<total". Will this cause a dead loop as transferred will be less than total forever?
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.
This introduces another StreamingOutput implementation that wraps a Buffer response coming from Mutiny when we proxy a GET request for a file. This enables us to avoid buffering the whole file in memory when transferring back to the client. This is based on work in Commonjava/gateway.
This code has NOT been tested for performance, to ensure that the memory usage problems are fixed. We need to performance test this, probably with a profiler on localhost or similar at a minimum, to ensure heap memory doesn't spike when a really large file is proxied.
NOTE: This is a backport of the attempt to implement file streaming on the archive-enabled version of this service. I've cut out digest handling and tracking record setup to make it work in the older codebase. It's based on commit: 989c6c1.