From be7a9975410ba2dff0d267b028d461e0e0263e21 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 2 Nov 2026 14:31:26 +0100 Subject: [PATCH] fix(metrics): stop unreachable servers from blocking web workers Co-Authored-By: Claude Opus 5.5 --- app/Services/SentinelMetricsClient.php | 71 +++++++++++++++++++-- app/Traits/HasMetrics.php | 36 +++++++---- tests/Feature/ServerMetricsPrefetchTest.php | 14 ++-- 3 files changed, 93 insertions(+), 28 deletions(-) diff --git a/app/Services/SentinelMetricsClient.php b/app/Services/SentinelMetricsClient.php index fe3d785c63..ba1f556704 100644 --- a/app/Services/SentinelMetricsClient.php +++ b/app/Services/SentinelMetricsClient.php @@ -11,6 +11,64 @@ class SentinelMetricsClient public function __construct(protected Server $server) {} + /** + * Optional charts must not occupy web workers for the general SSH timeout. + * One in-flight read per server, a short shared response cache, and a cooldown + * keep repeated dashboard refreshes from reconnecting to an unavailable server. + */ + public static function fetchOverSsh(Server $server, array $commands, string $cacheKey): ?string + { + $prefix = 'sentinel:metrics-ssh:'.$server->id; + $responseKey = $prefix.':response:'.hash('sha256', $cacheKey); + if (Cache::has($prefix.':unavailable')) { + return null; + } + $cached = Cache::get($responseKey); + if (is_string($cached)) { + return $cached; + } + + $lock = Cache::lock($prefix.':inflight', 15); + if (! $lock->get()) { + return null; + } + + try { + // Another node may have completed a fetch while this lock was acquired. + if (Cache::has($prefix.':unavailable')) { + return null; + } + $cached = Cache::get($responseKey); + if (is_string($cached)) { + return $cached; + } + + try { + $response = instant_remote_process( + $commands, + $server, + timeout: 8, + // Mux setup and health checks have separate, longer timeouts. + disableMultiplexing: true, + maxAttempts: 1 + ); + if ($response === null || trim($response) === '') { + throw new \RuntimeException('Metrics returned an empty response.'); + } + } catch (\Throwable) { + Cache::put($prefix.':unavailable', true, 60); + + return null; + } + + Cache::put($responseKey, $response, 10); + + return $response; + } finally { + $lock->release(); + } + } + /** Rounded to the minute so the history URL (and its cache key) is stable within a minute. */ public static function rangeFrom(string $range): string { @@ -238,14 +296,13 @@ class SentinelMetricsClient { $token = $this->server->settings->ensureValidSentinelToken(); - return instant_remote_process( - ["docker exec coolify-sentinel sh -c 'curl -s --max-time 10 -H \"Authorization: Bearer {$token}\" \"{$url}\"'"], + $command = "curl -sS --connect-timeout 2 --max-time 3 -H @- \"{$url}\" <<'COOLIFY_SENTINEL_AUTH'\nAuthorization: Bearer {$token}\nCOOLIFY_SENTINEL_AUTH"; + + return self::fetchOverSsh( $this->server, - false, - timeout: 15, - // Metrics are read again on the next refresh. SSH retries would keep a worker past its timeout. - maxAttempts: 1 - ); + ["docker exec -i coolify-sentinel sh -s <<'COOLIFY_SENTINEL_SCRIPT'\n{$command}\nCOOLIFY_SENTINEL_SCRIPT"], + $url + ) ?? ''; } private function guard(string $response): string diff --git a/app/Traits/HasMetrics.php b/app/Traits/HasMetrics.php index 3a1648fc3f..1f74df8618 100644 --- a/app/Traits/HasMetrics.php +++ b/app/Traits/HasMetrics.php @@ -12,14 +12,14 @@ trait HasMetrics /** * Raw Sentinel responses from prefetchMetrics(), keyed by "type:mins"; each is read once. * - * @var array + * @var array */ protected array $prefetchedMetricResponses = []; /** * Fetch several metric types in one SSH round-trip instead of one per type. The - * get*Metrics() getters then read these responses. On a failure nothing is stored, - * so each getter fetches on its own as before. + * get*Metrics() getters then read these responses. Failed prefetches are remembered + * for this request so the getters do not retry the same unavailable server. * * @param array $types */ @@ -30,23 +30,26 @@ trait HasMetrics return; } + foreach ($types as $type) { + $this->prefetchedMetricResponses["{$type}:{$mins}"] = null; + } + $from = now()->subMinutes($mins)->toIso8601ZuluString(); $token = $this->metricsToken($server); // The token reaches curl on stdin (`-H @-`), so it is not a process argument. The token // only has letters, digits and `._-+/=` (ServerSetting::isValidSentinelToken), so it // cannot end the heredoc. Responses are separated by a 0x1E record separator. $script = implode("\nprintf '\\036'\n", array_map( - fn (string $type) => 'curl -s -H @- "'.$this->getMetricsEndpoint($type, $from, $mins)."\" <<'COOLIFY_SENTINEL_AUTH'\nAuthorization: Bearer {$token}\nCOOLIFY_SENTINEL_AUTH", + fn (string $type) => 'curl -sS --connect-timeout 2 --max-time 3 -H @- "'.$this->getMetricsEndpoint($type, $from, $mins)."\" <<'COOLIFY_SENTINEL_AUTH'\nAuthorization: Bearer {$token}\nCOOLIFY_SENTINEL_AUTH", $types )); - try { - $output = instant_remote_process( - ["docker exec -i coolify-sentinel sh -s <<'COOLIFY_SENTINEL_SCRIPT'\n{$script}\nCOOLIFY_SENTINEL_SCRIPT"], - $server, - maxAttempts: 1 - ); - } catch (\Throwable) { + $output = SentinelMetricsClient::fetchOverSsh( + $server, + ["docker exec -i coolify-sentinel sh -s <<'COOLIFY_SENTINEL_SCRIPT'\n{$script}\nCOOLIFY_SENTINEL_SCRIPT"], + static::class.':'.$this->uuid.':'.implode(',', $types).':'.$mins + ); + if ($output === null) { return; } @@ -152,13 +155,18 @@ trait HasMetrics $endpoint = $this->getMetricsEndpoint($type, $from, $mins); $token = $this->metricsToken($server); - $response = instant_remote_process( - ["docker exec coolify-sentinel sh -c 'curl -H \"Authorization: Bearer {$token}\" \"{$endpoint}\"'"], + $command = "curl -sS --connect-timeout 2 --max-time 3 -H @- \"{$endpoint}\" <<'COOLIFY_SENTINEL_AUTH'\nAuthorization: Bearer {$token}\nCOOLIFY_SENTINEL_AUTH"; + $response = SentinelMetricsClient::fetchOverSsh( $server, - false + ["docker exec -i coolify-sentinel sh -s <<'COOLIFY_SENTINEL_SCRIPT'\n{$command}\nCOOLIFY_SENTINEL_SCRIPT"], + static::class.':'.$this->uuid.':'.$type.':'.$mins ); } + if ($response === null) { + return null; + } + if (str($response)->contains('error')) { $error = json_decode($response, true); $error = data_get($error, 'error', 'Something is not okay, are you okay?'); diff --git a/tests/Feature/ServerMetricsPrefetchTest.php b/tests/Feature/ServerMetricsPrefetchTest.php index 12a5cb6078..6d84e68b35 100644 --- a/tests/Feature/ServerMetricsPrefetchTest.php +++ b/tests/Feature/ServerMetricsPrefetchTest.php @@ -52,20 +52,20 @@ it('keeps the Sentinel error of one prefetched metric', function () { ->and(fn () => $this->server->getMemoryMetrics(5))->toThrow(Exception::class, 'Unauthorized'); }); -it('falls back to one fetch per metric when the batch fails', function () { +it('does not retry an unavailable server after the batch fails', function () { $commands = []; Process::fake(function (PendingProcess $process) use (&$commands) { $commands[] = remoteProcessScript($process); - return count($commands) === 1 - ? Process::result('', 'ssh: connect to host failed', 1) - : Process::result('[{"time":2000,"percent":5}]'); + return Process::result('', 'ssh: connect to host failed', 1); }); $this->server->prefetchMetrics(['cpu', 'memory'], 5); - expect($this->server->getCpuMetrics(5))->toBe([[2000, 5.0]]) - ->and($commands)->toHaveCount(2); + expect($this->server->getCpuMetrics(5))->toBeNull() + ->and($this->server->getMemoryMetrics(5))->toBeNull() + ->and($this->server->getLoadMetrics(5))->toBeNull() + ->and($commands)->toHaveCount(1); }); it('quotes the bucketed history URL in a single fetch for a non-root user', function () { @@ -96,7 +96,7 @@ it('runs the batched metrics script with sudo for a non-root user', function () expect($commands)->toHaveCount(1) ->and($commands[0])->toContain('sudo docker exec -i coolify-sentinel sh -s') - ->and($commands[0])->toContain("\ncurl -s -H @- ") + ->and($commands[0])->toContain("\ncurl -sS --connect-timeout 2 --max-time 3 -H @- ") ->and($commands[0])->not->toContain('sudo curl') ->and($commands[0])->not->toContain('sudo printf'); });