Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/settings/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@
'OCA\\Settings\\SetupChecks\\PhpOpcacheSetup' => $baseDir . '/../lib/SetupChecks/PhpOpcacheSetup.php',
'OCA\\Settings\\SetupChecks\\PhpOutdated' => $baseDir . '/../lib/SetupChecks/PhpOutdated.php',
'OCA\\Settings\\SetupChecks\\PhpOutputBuffering' => $baseDir . '/../lib/SetupChecks/PhpOutputBuffering.php',
'OCA\\Settings\\SetupChecks\\PhpSAPI' => $baseDir . '/../lib/SetupChecks/PhpSAPI.php',
'OCA\\Settings\\SetupChecks\\PushService' => $baseDir . '/../lib/SetupChecks/PushService.php',
'OCA\\Settings\\SetupChecks\\RandomnessSecure' => $baseDir . '/../lib/SetupChecks/RandomnessSecure.php',
'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => $baseDir . '/../lib/SetupChecks/ReadOnlyConfig.php',
Expand Down
1 change: 1 addition & 0 deletions apps/settings/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ class ComposerStaticInitSettings
'OCA\\Settings\\SetupChecks\\PhpOpcacheSetup' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpOpcacheSetup.php',
'OCA\\Settings\\SetupChecks\\PhpOutdated' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpOutdated.php',
'OCA\\Settings\\SetupChecks\\PhpOutputBuffering' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpOutputBuffering.php',
'OCA\\Settings\\SetupChecks\\PhpSAPI' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpSAPI.php',
'OCA\\Settings\\SetupChecks\\PushService' => __DIR__ . '/..' . '/../lib/SetupChecks/PushService.php',
'OCA\\Settings\\SetupChecks\\RandomnessSecure' => __DIR__ . '/..' . '/../lib/SetupChecks/RandomnessSecure.php',
'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => __DIR__ . '/..' . '/../lib/SetupChecks/ReadOnlyConfig.php',
Expand Down
2 changes: 2 additions & 0 deletions apps/settings/lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
use OCA\Settings\SetupChecks\PhpOpcacheSetup;
use OCA\Settings\SetupChecks\PhpOutdated;
use OCA\Settings\SetupChecks\PhpOutputBuffering;
use OCA\Settings\SetupChecks\PhpSAPI;
use OCA\Settings\SetupChecks\PushService;
use OCA\Settings\SetupChecks\RandomnessSecure;
use OCA\Settings\SetupChecks\ReadOnlyConfig;
Expand Down Expand Up @@ -209,6 +210,7 @@ public function register(IRegistrationContext $context): void {
$context->registerSetupCheck(PhpOpcacheSetup::class);
$context->registerSetupCheck(PhpOutdated::class);
$context->registerSetupCheck(PhpOutputBuffering::class);
$context->registerSetupCheck(PhpSAPI::class);
$context->registerSetupCheck(RandomnessSecure::class);
$context->registerSetupCheck(ReadOnlyConfig::class);
$context->registerSetupCheck(RepairSanitizeSystemTagsAvailable::class);
Expand Down
89 changes: 89 additions & 0 deletions apps/settings/lib/SetupChecks/PhpSAPI.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Settings\SetupChecks;

use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\SetupCheck\ISetupCheck;
use OCP\SetupCheck\SetupResult;

class PhpSAPI implements ISetupCheck {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: PhpSAPI sounds broader than what this checks. What about PhpFpmMaxChildren?

public function __construct(
private IL10N $l10n,
private IURLGenerator $urlGenerator,
) {
}

#[\Override]
public function getName(): string {
return $this->l10n->t('PHP-FPM process limit');
}

#[\Override]
public function getCategory(): string {
return 'php';
}

#[\Override]
public function run(): SetupResult {
if (php_sapi_name() !== 'fpm-fcgi') {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

return SetupResult::success();
}

if (!function_exists('fpm_get_status')) {
return SetupResult::warning(
$this->l10n->t('PHP-FPM status is unavailable, so the max children check could not be completed.'),
$this->urlGenerator->linkToDocs('admin-php-fpm')
);
}

$status = fpm_get_status();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if ($status === false) {
return SetupResult::warning(
$this->l10n->t('PHP-FPM status could not be read, so the max children check could not be completed.'),
$this->urlGenerator->linkToDocs('admin-php-fpm')
);
}

$maxChildrenReached = $status['max-children-reached'] ?? null;
$maxActiveProcesses = $status['max-active-processes'] ?? null;
$startTimeEpoch = $status['start-time'] ?? null;

if (!is_int($maxChildrenReached) || !is_int($maxActiveProcesses) || !is_int($startTimeEpoch)) {
return SetupResult::warning(
$this->l10n->t('PHP-FPM status is incomplete, so the max children check could not be completed.'),
$this->urlGenerator->linkToDocs('admin-php-fpm')
);
}

if ($maxChildrenReached > 0) {
$startTime = date('Y-m-d H:i:s T', $startTimeEpoch);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

date() gives server time in a fixed format. IDateTimeFormatter::formatDateTime() would follow the admin's timezone and locale, wdyt?

$timesReached = $this->l10n->n('once', '%n times', $maxChildrenReached);

$message = $this->l10n->t(
'PHP-FPM reached its configured "pm.max_children" limit %s since its last restart at %s (peak active processes: %s).',
[$timesReached, $startTime, (string)$maxActiveProcesses],
);
$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(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

'The counter resets when PHP-FPM restarts. If saturation is ongoing or recurring, review pool configuration and server capacity and consider increasing "pm.max_children".'
);

return SetupResult::warning(
$message,
$this->urlGenerator->linkToDocs('admin-php-fpm'),
);
}

return SetupResult::success();
}
}
Loading