On Fri, Jan 31, 2025 at 4:06 PM Arkadiy Kulev <e...@ethaniel.com> wrote:
> Hi! > > >> This wouldn't really work because FPM does not control the script during >>>> execution and would have to check it out after each allocation which is not >>>> really viable. >>>> >>> >>>> >>> Thanks for the feedback! I agree that monitoring memory usage after each >>> allocation would be infeasible. However, my suggestion was actually to >>> check memory usage *only once per request*, specifically at request >>> shutdown, when FPM regains control and before assigning another request to >>> that worker. >>> >> >>> >> I think that would require a different name because it would not reflect >> max memory usage in any way - it would be measured at the time when the >> memory usage is lowest. We could maybe set some options that would measure >> the maximum of increase of memory between requests - I mean difference >> between lowest memory usage (most likely after the first request) and then >> compare against current usage after the latest request and set limit on >> this difference. Not sure about the name for that. Maybe something like >> pm.max_memory_increase or something like that. >> > > I see where you’re coming from, but I believe measuring memory “delta” or > “increase” could be confusing for end users. In practice, admins and > developers often glance at tools like top or ps to see *current* memory > usage for each FPM worker, spot outliers, and then set a threshold > accordingly. > > If we start talking about “lowest usage” vs. “current usage” and a “max > increase,” that becomes much harder to translate to real-world > monitoring—and it’s non-intuitive compared to simply reading the value > right off top and setting a limit. So, from a usability standpoint, I > think a direct measurement of resident memory (as people see in common > system tools) would be the most straightforward and least confusing. > It's probably less confusing than setting pm.max_memory to some value and then see that the process allocates much more. We could potentially call it pm.max_idle_memory or something that clearly shows that it's not a total max memory. > Just to be clear, "memory_limit" helps kill runaway scripts mid-request. >>> By contrast, the newly proposed pm.max_memory is meant to catch processes >>> with a slow leak across multiple requests. We only need to check at the end >>> of each request, which is presumably when the worker returns control to FPM. >>> >> >> There is one thing to note that memory_limit actually measure only memory >> allocated through the per request php memory allocator so it's not actually >> limit on total usage including the standard allocator memory usage. So >> there would be still a use case for total limit using cgroups but I agree >> that the more important use is to catch slow leaks which the above should >> help with in a better way than pm.max_requests. >> > > You’re absolutely right that cgroups handle total memory usage—including > memory outside the PHP allocator—more accurately. But as I’ve mentioned > before, relying on cgroups to limit memory typically means an OOM kill that > can happen at *any* moment, often right in the middle of a request. > That’s precisely what I’m trying to avoid. > The whole idea behind pm.max_memory is to allow a *graceful* check *after* > each request completes, so we can recycle the worker before starting a new > request. That way, no request gets abruptly killed. cgroups don’t really > accommodate that scenario—they’re great for overall resource control but > not for per-request, child-level recycling within FPM. > Yeah it should really be the last resort. Agreed that for this particular case, the solution above would be better. Regards, Jakub