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.

Sign up for GitHub

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

Jump to bottom

Add data streaming where beneficial to reduce memory usage #3365

Merged

ssddanbrown merged 5 commits into development from data_streaming

Apr 24, 2022

Member

ssddanbrown commented Apr 2, 2022 •

edited

Areas to address

- Attachment Downloads
- Attachment Uploads
- API Attachment Downloads
- API Attachment Uploads
- ~~Image Uploads~~ Upload path already looks to be at reasonable minimum. Resizing is loaded into memory either way. 18MB image on 32MB memory uploads okay.
- Image Downloads? Was already streaming.

Todo

Test when using s3-like storage for each area
- Tested on minio, Works very nicely.

Related

Related to #2886


          Updated attachment download responses to stream from filesystem

82e8b15

This allows download of attachments that are larger than current memory
limits, since we're not loading the entire file into memory any more.

For inline file responses, we take a 1kb portion of the file to sniff
before to check mime before we proceed.

ssddanbrown added 🛠️ Enhancement 🏭 Back-End labels

ssddanbrown added this to the Next Feature Release milestone

ssddanbrown self-assigned this

ssddanbrown changed the title ~~Add data streaming where required to reduce memory usage~~ Add data streaming where beneficial to reduce memory usage

ssddanbrown added 4 commits

April 2, 2022 18:42


          Fixed streamed outputs in more extreme scenarios

6749faa

Fixes hitting memory limits where downloaded file sizes are much greater
than memory limit. Stopping and flushing output buffer seemed to stop
limits causing issues when fpassthru is used.
Tested with 24M memory limit and 734M file


          Added streamed uploads for attachments

cb770c5


          Added streaming support to API attachment read responses

08a8c00

Required some special handling due to the content being base64-encoded
within a JSON response.


          Fixed tests from streaming changes

59d1fb2

- Added testing check to buffer stop/clear on streaming output due to
  interference during tests.
- Made content-disposition header a little safer in download responses.
- Also aligned how we check for testing environment.

Member Author

ssddanbrown commented Apr 3, 2022

Ready for merge when appropriate.

ssddanbrown merged commit 829f808 into development

ssddanbrown deleted the data_streaming branch

April 24, 2022 12:59

ssddanbrown mentioned this pull request

Error uploading files more then 150mb #2886

Closed

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