Repository navigation
feat(settings): warn when PHP-FPM reaches pm.max_children - #46570
joshtrichards wants to merge 3 commits into
Conversation
joshtrichards
added
enhancement
2. developing
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
joshtrichards
added
the
pending documentation
b8594e2 to
a6626d6
Compare
|
@joshtrichards what is missing here? Can I help move this forward? :) |
Signed-off-by: Josh Richards <josh.t.richards@gmail.com>
a6626d6 to
16f8287
Compare
Report when PHP-FPM reaches its configured pm.max_children limit, including the peak active process count and pool restart time. Handle unavailable or incomplete FPM status with a warning, and link admins to the PHP-FPM documentation for troubleshooting guidance. Signed-off-by: Josh <josh.t.richards@gmail.com>
joshtrichards
changed the title
joshtrichards
added
3. to review
| ); | ||
| } | ||
|
|
||
| $status = fpm_get_status(); |
There was a problem hiding this comment.
Could we get a test for this? Wrapping fpm_get_status() in a small protected method would let a test cover the false / incomplete / reached cases.
| $message .= ' ' . $this->l10n->t( | ||
| 'When the pool is saturated, additional requests may be queued, time out, or fail. This can cause slow responses, client connection errors, or 502/504 gateway errors.' | ||
| ); | ||
| $message .= "\n\n" . $this->l10n->t( |
There was a problem hiding this comment.
I don't think the setup checks list keeps line breaks in the web UI (no pre-line on the description), so this ends up as one block there. Maybe drop the \n\n and keep it a single paragraph?
|
|
||
| #[\Override] | ||
| public function run(): SetupResult { | ||
| if (php_sapi_name() !== 'fpm-fcgi') { |
There was a problem hiding this comment.
From occ setupchecks this is always cli, so it shows as green without checking anything. Would an info result like "Not running under PHP-FPM" be clearer there?
| use OCP\SetupCheck\ISetupCheck; | ||
| use OCP\SetupCheck\SetupResult; | ||
|
|
||
| class PhpSAPI implements ISetupCheck { |
There was a problem hiding this comment.
Nitpick: PhpSAPI sounds broader than what this checks. What about PhpFpmMaxChildren?
| } | ||
|
|
||
| if ($maxChildrenReached > 0) { | ||
| $startTime = date('Y-m-d H:i:s T', $startTimeEpoch); |
There was a problem hiding this comment.
date() gives server time in a fixed format. IDateTimeFormatter::formatDateTime() would follow the admin's timezone and locale, wdyt?
Summary
Add a PHP setup check that warns when the PHP-FPM pool has reached its configured
pm.max_childrenlimit. Since this can cause various problems, let's give a hint to the admin.Include the number of occurrences, latest restart time, and peak active process count to help admins assess the impact.
The check is not applicable to non-FPM SAPIs, including
mod_php. PHP provides no direct equivalent for retrievingmod_phpstatus; using Apache’smod_statuswould require additional server configuration.If PHP-FPM status is unavailable or incomplete, report that the check could not be completed. Link to the PHP-FPM documentation and advise reviewing pool configuration and server capacity before increasing the limit.
TODO
Checklist