-
Notifications
You must be signed in to change notification settings - Fork 68
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
Excessive memory usage #67
Comments
Let me add another piece of data: I was able to do a 40,000 file, 130GB zip on a server with 4GB memory and 4GB swap. So whatever is going on here was not scaling the way I expected. (And even that one still used almost all of the resources of the server, which still seems a lot.) |
I've been working on the project that I have that uses this and was reminded of the issue. I wonder if there is something about the particular file list (either just in raw size or with it having some sort of peculiarity) that is is causing the Ragel parser to blow up, which seems possible based on a bit of searching. I have an example file list that blows it up, but I want to see how far I need to cut it down to get it to work to produce a more minimal example (that will blow up my 4GB mem/4GB swap machine). It is about 170000 files (essentially the same one I mentioned initially). |
The file that blows up my machine has the following properties:
So it seems like parsing this 7.2MB file is taking over 8GB. Taking it down to 12250 lines it can do it, just barely. If I remember I'll attach the file once the AWS links in it have expired in a week, but it's just a lot of lines like:
Does the parsing try to do the whole file at once rather than splitting into individual lines on '\r\n' and then parsing each individual line? |
You can see the parser logic here: https://github.com/evanmiller/mod_zip/blob/master/ngx_http_zip_parsers.rl It's possible that the grammar is ambiguous, and Ragel blowing up with internal state (i.e. for backtracking). You might try recompiling the |
I think that it's possible that what is going on here is that the URI field allows \r and \n and that is forcing a lot of backtracking information to be kept. I'm going to try regenerating things with that possibility removed and then try it on one of my test cases to see if there is an improvement. |
No, that was not enough. I'm not really familiar with Ragel, so I'll look at it some more to see if it provides any facilities that I think might help. |
hi @evanmiller , do you see another possible source of this problem besides the Ragel parser? |
I've figured out the bug; it's not the parser. Every time mod_zip reads a chunk of input (about 8KB), it allocates a new buffer, copies old all contents, but does not free the old buffer: https://github.com/evanmiller/mod_zip/blob/master/ngx_http_zip_module.c#L153-L161 This results in an O(N²) memory usage, so even 10MB of input will take up GBs of memory. Freeing the old buffer fixes it - though it's still inefficient in terms of CPU. Better yet, just use Here's a fix: quiltdata@1b6813a. If that looks reasonable, I'll send out a proper PR. |
@dimaryaz Excellent find! Yes feel free to open a PR. An array seems like an unusual data structure to hold a string, but it seems like it will work. |
Currently, every time mod_zip reads 8KB, it allocates a new string, copies all existing data, and doesn't free the old string. This results in GBs of memory use for a 10MB input. Just use an nginx array which automatically doubles in size as needed. (Still leaks memory, but not as much.) Fixes evanmiller#67
This is with nginx 1.8.1 and mod_zip 1.1.6.
I'm trying to download a zip package for around 170,000 files and 100GB.
I use the workaround I described in issue #42 because of the large number of files we sometimes deal with, but this doesn't seem related.
When I watch under top, nginx just uses all the memory of the machine. I tried this with a machine with as much as 160GB of memory, and nginx used it all in less than a minute. This seems to be happening while it is dealing with the manifest file; the second nginx server in the above workaround never gets hit before memory is exhausted.
I don't see anything in the configuration that I think would be causing this problem. I will attach a minorly scrubbed version:
nginx.conf.txt
Doing some other trials the problem seems to be with the number of files to go into the zip, not the overall size in bytes.
When I was developing the code that uses this (beginning of 2016) I had experimented with some even larger (in terms of number of files) downloads than this, and they were fine, but I don't really have good records to suggest what might be different.
Honestly I'm at a loss to think how nginx even could consume that much memory doing this, but I watched it happen.
The text was updated successfully, but these errors were encountered: