<html xmlns="http://www.w3.org/1999/xhtml"><head> <title></title> <meta name="viewport" content="width=device-width, initial-scale=1.0, user-scalable=no"> </head> <body style="font-family:Helvetica;color:#000000;font-size:13px;"><img id="B1754A1F341AC0E5A984833FC5630686" alt="" width="0px" src="https://receipts.canarymail.io/track/23BE42F21B750115E03660019DCF8935_B1754A1F341AC0E5A984833FC5630686.png" height="0px"><div id="CanaryBody"> <div> That’s good info. Thank you.</div><div><br></div><div>I have been doing some additional testing since my email last night and I have seen enough evidence to believe that file I/O in NJS is basically the source of the memory issues. I did some testing with very basic commands like readFileSync and Buffer + readSync and in all cases, the memory footprint when doing file handling in NJS is massive. </div><div><br></div><div>Just doing this:</div><div><br></div><div>let content = fs.readFileSync(path/to//file);</div><div>let parts = content.split(boundary);</div><div><br></div><div>Resulted in memory use that was close to a minimum of 4-8x the size of the file during my testing. We do have an upper bound on files that can be uploaded and that does contain this somwhat but it’s not hard for a larger request that is 99% file attachment to use exhorbitant amounts of memory. I actually tried doing a Buffer + readSync variation on the same thing and the memory footprint was actually FAR FAR worse when I did that.</div><div><br></div><div>The 4-8x minimum memory commit seems like a problem to me just generally. But the fact that readSync doesn’t seem to be any better on memory (much worse actually) basically means that NJS is only safe to use for processing smaller files (or POST bodies) right now. There’s just no good way to keep data that you don’t care about in a file from occupying excessive amounts of memory that can’t be reclaimed. If there is no way to improve the memory footprint when handling files (or big strings), no memory conservative way to stream a file through some sort of buffer, and no first-party utility for providing parsed POST bodies right now, then it might be worth the time to put some notes in the NJS docs that the fs module may not be appropriate for larger files (e.g. files over 1mb). </div><div><br></div><div>For what it’s worth, I’d also love to see some examples of how to properly use fs.readSync in the NJS examples docs. There really wasn’t much out there for that for NJS (or even in a lot of the Node docs) so I can’t say that my specific test implementation for that was ideal. But that’s just above and beyond the basic problems that I’m seeing with memory use with any form of file I/O at all (since the memory problems seem to be persistent whether doing reads or even log writes).</div> <div><br></div> </div> <div id="CanarySig"> <div> <div style="font-family:Helvetica;">—<br><div>Lance Dockins</div><br></div> <div><br></div> </div> </div> <div id="CanaryDropbox"> </div> <blockquote id="CanaryBlockquote"> <div> <div>On Thursday, Sep 21, 2023 at 5:01 PM, Dmitry Volyntsev <<a href="mailto:xeioex@nginx.com">xeioex@nginx.com</a>> wrote:<br></div> <div><br>On 9/21/23 6:50 AM, Lance Dockins wrote: <br> <br>Hi Lance, <br> <br>See my comments below. <br> <br><blockquote type="cite">Thanky you, Dmitry. <br> <br>One question before I describe what we are doing with NJS. I did read <br>about the VM handling process before switching from Lua to NJS and it <br>sounded very practical but my current understanding is that there <br>could be multiple VM’s instantiated for a single request. A js_set, <br>js_content, and js_header_filter directive that applies to a single <br>request, for example, would instantiate 3 VMs. And were you to need <br>to set multiple variables with js_set, then keep adding to that # of VMs. <br> <br> <br></blockquote>This is not correct. For js_set, js_content and js_header_filter there <br>is only a single VM. <br>The internalRedirect() is the exception, because a VM does not survive <br>it, but the previous VMs will not be freed until current request is <br>finished. BTW, a VM instance itself is pretty small in size (~2kb) so it <br>should not be a problem if you have a reasonable number of redirects. <br> <br> <br><blockquote type="cite"> <br>My original understanding of that was that those VMs would be <br>destroyed once they exited so even if you had multiple VMs <br>instantiated per request, the memory impact would not be cumulative in <br>a single request. Is that understanding correct? Or are you saying <br>that each VM accumulates more and more memory until the entire request <br>completes? <br> <br>As far as how we’re using NJS, we’re mostly using it for header <br>filters, internal redirection, and access control. So there really <br>shouldn’t be a threat to memory in most instances unless we’re not <br>just dealing with a single request memory leak inside of a VM but also <br>a memory leak that involves every VM that NJS instantiates just <br>accumulating memory until the request completes. <br> <br>Right now, my working theory about what is most likely to be creating <br>the memory spikes has to do with POST body analysis. Unfortunately, <br>some of the requests that I have to deal with are POSTs that have to <br>either be denied access or routed differently depending on the <br>contents of the POST body. Unfortunately, these same routes can vary <br>in the size of the POST body and I have no control over how any of <br>that works because the way it works is controlled by third parties. <br> One of those third parties has significant market share on the <br>internet so we can’t really avoid dealing with it. <br> <br>In any case, before we switched to NJS, we were using Lua to do the <br>same things and that gave us the advantage of doing both memory <br>cleanup if needed and also doing easy analysis of POST body args. I <br>was able to do this sort of thing with Lua before: <br>local post_args, post_err = ngx.req.get_post_args() <br>if post_args.arg_name = something then <br> <br>But in NJS, there’s no such POST body utility so I had to write my <br>own. The code that I use to parse out the POST body works for both <br>URL encoded POST bodies and multipart POST bodies, but it has to read <br>the entire POST into a variable before I can use it. For small POSTs, <br>that’s not a problem. For larger POSTs that contain a big attachment, <br>it would be. Ultimately, I only care about the string key/value pairs <br>for my purposes (not file attachments) so I was hoping to discard <br>attachment data while parsing the body. <br> <br> <br> <br></blockquote>Thank you for the feedback, I will add it as to a future feature list. <br> <br><blockquote type="cite"> I think that that is actually how Lua’s version of this works too. <br> So my next thought was that I could use a Buffer and rs.readSync to <br>read the POST body in buffer frames to keep memory minimal so that I <br>could could discard the any file attachments from the POST body and <br>just evaluate the key/value data that uses simple strings. But from <br>what you’re saying, it sounds like there’s basically no difference <br>between fs.readSync w/ a Buffer and rs.readFileSync in terms of actual <br>memory use. So either way, with a large POST body, you’d be <br>steamrolling the memory use in a single Nginx worker thread. When I <br>had to deal with stuff like this in Lua, I’d just run collectgarbage() <br>to clean up memory and it seemed to work fine. But then I also wasn’t <br>having to parse out the POST body myself in Lua either. <br> <br>It’s possible that something else is going on other than that. <br> qs.parse seems like it could get us into some trouble if the <br>query_string that was passed was unusuall long too from what you’re <br>saying about how memory is handled. <br> <br> <br></blockquote>for qs.parse() there is a limit for a number of arguments, which you can <br>specify. <br> <br><blockquote type="cite"> <br>None of the situations that I’m handling are for long running <br>requests. They’re all designed for very fast requests that come into <br>the servers that I manage on a constant basis. <br> <br>If you can shed some light on the way that VM’s and their memory are <br>handled per my question above and any insights into what to do about <br>this type of situation, that would help a lot. I don’t know if there <br>are any plans to offer a POST body parsing feature in NJS for those <br>that need to evalute POST body data like how Lua did it, but if there <br>was some way to be able to do that at the Nginx layer instead of at <br>the NJS layer, it seems like that could be a lot more sensitive to <br>memory use. Right now, if my understanding is correct, the only <br>option that I’d even have would be to just stop doing POST body <br>handling if the POST body is above a certain total size. I guess if <br>there was some way to forcibly free memory, that would help too. But <br>I don’t think that that is as common of a problem as having to deal <br>with very large query strings that some third party appends to a URL <br>(probably maliciously) and/or a very large file upload attached to a <br>multipart POST. So the only concern that I’d have about memory in a <br>situation where I don’t have to worry about memory when parsing a <br>larger file woudl be if multiple js_sets and such would just keep <br>spawning VMs and accumulating memory during a single request. <br> <br>Any thoughts? <br> <br>— <br>Lance Dockins <br> <br> <br>On Thursday, Sep 21, 2023 at 1:45 AM, Dmitry Volyntsev <br><xeioex@nginx.com> wrote: <br> <br>On 20.09.2023 20:37, Lance Dockins wrote: <br><blockquote type="cite">So I guess my question at the moment is whether endless memory use <br>growth being reported by njs.memoryStats.size after file writes is <br>some sort of false positive tied to quirks in how memory use is <br>being <br>reported or whether this is indicative of a memory leak? Any <br>insight <br>would be appreicated. <br></blockquote> <br>Hi Lance, <br>The reason njs.memoryStats.size keeps growing is because NJS uses <br>arena <br>memory allocator linked to a current request and a new object <br>representing memoryStats structure is returned every time <br>njs.memoryStats is accessed. Currently NJS does not free most of the <br>internal objects and structures until the current request is <br>destroyed <br>because it is not intended for a long running code. <br> <br>Regarding the sudden memory spikes, please share some details <br>about JS <br>code you are using. <br>One place to look is to analyze the amount of traffic that goes to <br>NJS <br>locations and what exactly those location do. <br> <br></blockquote></div> </div> </blockquote> </body></html>