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

Fix CarrierWave reads large files into memory #2314

Conversation

dosuken123
Copy link
Contributor

Hi from GitLab.

We're currently having a problem because Carrierwave eagerly loads large files into memory. This was originally fixed at #468, but reproduced at #1517.

I fixed by adding a logic to reopen closed streams when ::Fog#read is kicked. This shouldn't affect Carrierwave performance, nor breaking #1517 changes.

Just in case, I added a test to check that CW works even if the file is StringIO, i.e. new_file.read path.

/cc

@dosuken123
Copy link
Contributor Author

/cc @stanhu @ayufan

@dosuken123
Copy link
Contributor Author

@mshibuya Hi. I assume you're the one of the mainteners. Could you review this PR, please? Thanks!

@dosuken123
Copy link
Contributor Author

/cc @amatsuda

@dosuken123 dosuken123 force-pushed the fix-large-file-memeory-consumption branch from 7348ccc to 800e0db Compare May 28, 2018 05:36
@mshibuya mshibuya merged commit 1f688bb into carrierwaveuploader:master May 28, 2018
@mshibuya
Copy link
Member

Great, thanks.

@ayufan
Copy link

ayufan commented May 28, 2018

@mshibuya Would you mind pushing new CarrierWave release with that?

@ayufan
Copy link

ayufan commented May 29, 2018

@mshibuya do you have moment to do it? :)

@ayufan
Copy link

ayufan commented May 30, 2018

@mshibuya :)

@mshibuya
Copy link
Member

Sure, but give me some time for it 🙏

filipalacerda pushed a commit to gitlabhq/gitlabhq that referenced this pull request Jun 1, 2018
filipalacerda pushed a commit to gitlabhq/gitlabhq that referenced this pull request Jun 1, 2018
…ve' into 'master'

Add a monkey patch of carrierwaveuploader/carrierwave#2314

See merge request gitlab-org/gitlab-ce!19102
@mshibuya
Copy link
Member

@ayufan Sorry for delay, just released 1.2.3 🚢

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

Successfully merging this pull request may close these issues.

None yet

6 participants