Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Please provide streaming for large content to avoid unnecessary RAM wasting or busting #1454

Open
Vampire opened this issue May 9, 2022 · 4 comments

Comments

@Vampire
Copy link

Vampire commented May 9, 2022

I have a Gradle build where I build a Docker image tar file which has a size of 166 MiB.
I use the GitHub publish Gradle plugin and tried to upload this Docker image as asset to the release.
This consistently resulted in an OutOfMemoryError.

The GitHub publish Gradle plugin under the hood uses this library.
And I also easily reproduced it stand-alone.
In version 1.1xx, you use an HttpsUrlConnection without setting it to fixed-length streaming or chunking mode and thus the JRE uses a byte-array output stream to colllect the whole body in RAM to determine the size up-front.
In version 1.3xx many things changed, but now you use your GitHubRequest.Builder#with(java.io.InputStream) method to load the whole file into RAM using IOUtils.toByteArray.

So at different places both load the whole file into RAM instead of properly streaming it.
Please support at least giving the size to methods like uploadAsset so that the size can be set as HTTP header but the content be streamed, or if GitHub supports it maybe even using chunked mode additionally if no explicit size was given.

The problem can pretty easily be reproduced using:

github
    .getRepository("my/repository")
    .listReleases()
    .iterator()
    .next()
    .uploadAsset("foo.tar.gz", new FileInputStream("file/that/is/too/large/for/RAM"), "application/octet-stream")
@bitwiseman
Copy link
Member

see #1405.

@Vampire
Copy link
Author

Vampire commented May 15, 2022

But that's a different issue, isn't it? There it is about receiving something and according to you it needs significant complexity to be fixed. Here it is about sending an artifact where you (at least in the 1.1xx version) just need to enable streaming mode and eventually let the user specify the file size or the file instead of just a stream. Should be pretty trivial to fix this one, shouldn't it?

@bitwiseman
Copy link
Member

@Vampire Sorry for not responding sooner. If you are able to create a PR enabling the behavior you want, I'm open to discussing.

@bitwiseman bitwiseman reopened this Aug 18, 2022
@bitwiseman
Copy link
Member

I can see how they are different path but the underlying framework currently treats them the same. I'll keep this open separately as I can see ways that the upload issue could be resolved separately from the download.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants