Debugging Nginx Memory Spikes on Production Servers

[Dmitry Volyntsev]

On 9/26/23 8:30 AM, Lance Dockins wrote:
> Up until now, I had assumed that string.match types of statements were 
> just transparently calling PCRE behind the scenes so that the 
> associated memory from the PCRE call was being freed after use.  Maybe 
> that’s not even an accurate read on how Nginx is using PCRE but that’s 
> how I envisioned it.

String.prototype.match() returns an array of matched elements. See 
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/match. 
It means that many calls to that method that have positive matches 
consume memory proportional to the size of the resulting chunks. But if 
the match fails it should not consume (much) memory. So if you have a 
bunch of if/then clauses until the first match it should not be a 
problem (if not let me know).


In general doing a routing in NJS (or any other scripting) is not 
recommended, because it complicates the native nginx location routing. 
Could you please explain what exactly you want to do with NJS RegExp 
what nginx cannot do with regexp locations?

>
> In any case, that lead me to a few questions.
>
>  1. How does keepalive affect these VMs?  I realize that keepalive is
>     for the connection rather than the request, but I still wanted to
>     confirm that VM’s are not sticking around occupying memory after
>     the request completes due to keepavlive settings.
>
>
All the current VMs are destroyed when a current HTTP request is finalized.


>  1.   We have a mix of fast requests and long running requests (e.g
>     5-10s) so if enough long running requests build up at the same
>     time that a flood of very fast requests come in, NJS could
>     certainly burn out a lot of memory under the current conditions as
>     I understand it.  If there were a lot of large file downloads,
>     those would also occupy memory for the entirety of the download if
>     I understand correctly.
>  2. Is it possible to adjust the NJS codebase to free memory for the
>     actual if condition and for string matches after they’ve run as
>     long as they didn’t include a variable assignment inside of the
>     actual if condition?  I’m not talking about whatever is in the
>     block content - just the actual condition.  Given how likely it is
>     for people to use basic if conditions and string matches in NJS,
>     that seems like it might be a stopgap measure that reduces the
>     memory footprint without having to build full garbage collection.
>
>
The problem with JS Regexps is that they produce a lot of intermediary 
objects. Because most of the RegExp related calls end up 
RegExp.prototype.exec() which return a large object representing the 
result of PCRE matching.


One way to mediate the problem could be using RegExp.prototype.test() 
which return just a boolean.
But I need to improve it first, because right now it uses 
RegExp.prototype.exec() internally. The reason RegExp.prototype.test() 
will be easier to improve is that I can be sure that the resulting 
object can be freed right away. In addition to that I plan to improve 
the memory consumption for RegExp.prototype.exec() result, thanks for 
reporting.


>
>  1. Is there any workaround for this type of memory problem other than
>     just to force the use of an internalRedirect to drop the existing
>     VM and create a new one?
>
>  1. Could it make sense to allow for a directive that would force the
>     destruction and regeneration of a new JS VM?  That wouldn’t solve
>     the memory leak that bulids to 1mb per request but it would
>     shorten its lifetime (which could ease memory pressure in
>     situations where there are some long running requests holding open
>     the requests)
>
I do not think so.