Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:126254
Precedence: bulk
MIME-Version: 1.0
References: <CAGVaEcmDxxbFzNxXnF=jmdUDnQTk55zgorbsqQqyP8w2Z=-iaw@mail.gmail.com>
 <CAEKnhAFeiJESkqJ0o9PuU59VFK1hdfk2xeEDJn_BbsaF0sBy7Q@mail.gmail.com> <CAGVaEcnOWGYPPUTYn1OZkxCvLmXSnzrQm73YB=ucRMuOA6DCpA@mail.gmail.com>
In-Reply-To: <CAGVaEcnOWGYPPUTYn1OZkxCvLmXSnzrQm73YB=ucRMuOA6DCpA@mail.gmail.com>
Date: Fri, 31 Jan 2025 11:14:39 +0100
Message-ID: <CAEKnhAHO7z1_fW6Gkko4GDm3GGS+0sD9AEuMyhna+PQ_5Kx+2g@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] Introducing pm.max_memory for PHP-FPM
To: Arkadiy Kulev <eth@ethaniel.com>
Cc: internals@lists.php.net
Content-Type: multipart/alternative; boundary="000000000000221edc062cfdd071"
From: bukka@php.net (Jakub Zelenka)

--000000000000221edc062cfdd071
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

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 t=
o
> 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.


> We already have hooks for request startup and request shutdown in FPM.
> Could we simply insert a memory check at the request shutdown stage=E2=80=
=94where
> the worker is returning control to the FPM master process=E2=80=94before =
picking up
> a new request?
>
>
Yeah that would be possible.


> 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 e=
nd
> of each request, which is presumably when the worker returns control to F=
PM.
>

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.

Regards,

Jakub

--000000000000221edc062cfdd071
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div class=3D"gmail_quote gmail_quote_container"><div>Hi,<=
/div><div>=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0px=
 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><di=
v dir=3D"ltr"><div dir=3D"ltr"></div><div class=3D"gmail_quote"><blockquote=
 class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px so=
lid rgb(204,204,204);padding-left:1ex"><div dir=3D"ltr"><div class=3D"gmail=
_quote"><div>This wouldn&#39;t really work because FPM does not control the=
 script during execution and would have to check it out after each allocati=
on which is not really viable.</div></div></div></blockquote><div><br><div>=
Thanks for the feedback! I agree that monitoring memory usage after each al=
location would be infeasible. However, my suggestion was actually to check =
memory usage <b>only once per request</b>, specifically at request shutdown=
, when FPM regains control and before assigning another request to that wor=
ker.=C2=A0</div></div><div><br></div></div></div></blockquote><div><br></di=
v><div>I think that would require a different name because it would not ref=
lect max memory usage in any way - it would be measured at the time when th=
e memory usage is lowest. We could maybe set some options that would measur=
e the maximum of increase of memory between requests - I mean difference be=
tween lowest memory usage (most likely after the first request) and then co=
mpare 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_m=
emory_increase or something like that.</div><div>=C2=A0</div><blockquote cl=
ass=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid=
 rgb(204,204,204);padding-left:1ex"><div dir=3D"ltr"><div class=3D"gmail_qu=
ote"><div></div><div>We already have hooks for request startup and request =
shutdown in FPM. Could we simply insert a memory check at the request shutd=
own stage=E2=80=94where the worker is returning control to the FPM master p=
rocess=E2=80=94before picking up a new request?<br></div><div><br></div></d=
iv></div></blockquote><div><br></div><div>Yeah that would be possible.</div=
><div>=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px=
 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div di=
r=3D"ltr"><div class=3D"gmail_quote"><div></div><div>Just to be clear, &quo=
t;memory_limit&quot; helps kill runaway scripts mid-request. By contrast, t=
he newly proposed pm.max_memory is meant to catch processes with a slow lea=
k across multiple requests. We only need to check at the end of each reques=
t, which is presumably when the worker returns control to FPM.</div></div><=
/div></blockquote><div><br></div><div>There is one thing to note that memor=
y_limit actually measure only memory allocated through the per request php =
memory allocator so it&#39;s not actually limit on total usage including th=
e standard allocator memory usage. So there would be still a use case for t=
otal limit using cgroups but I agree that the more important use is to catc=
h slow leaks which the above should help with in a better way than pm.max_r=
equests.</div><div><br></div><div>Regards,</div><div><br></div><div>Jakub</=
div></div></div>

--000000000000221edc062cfdd071--