From 7e2187180c1643396b3fc79f93cf38de3ab33d10 Mon Sep 17 00:00:00 2001 From: David Jardin Date: Thu, 22 May 2025 09:55:42 +0200 Subject: [PATCH 1/7] implement various tweaks to rate limits and queue --- app/Console/Commands/QueueHealthChecks.php | 2 +- app/Console/Commands/QueueUpdates.php | 2 +- app/Jobs/CheckSiteHealth.php | 15 +++++++-- app/Jobs/UpdateSite.php | 12 ++++++- app/Providers/AppServiceProvider.php | 20 ++++++++--- app/Providers/HttpclientServiceProvider.php | 35 ------------------- app/RemoteSite/Connection.php | 4 ++- config/horizon.php | 37 ++++++++++++++++----- 8 files changed, 74 insertions(+), 53 deletions(-) diff --git a/app/Console/Commands/QueueHealthChecks.php b/app/Console/Commands/QueueHealthChecks.php index 06cd153..09b8849 100644 --- a/app/Console/Commands/QueueHealthChecks.php +++ b/app/Console/Commands/QueueHealthChecks.php @@ -48,7 +48,7 @@ function (Collection $chunk) { $this->totalPushed += $chunk->count(); // Push each site check to queue - $chunk->each(fn ($site) => CheckSiteHealth::dispatch($site)); + $chunk->each(fn ($site) => CheckSiteHealth::dispatch($site)->onQueue('cron')); } ); diff --git a/app/Console/Commands/QueueUpdates.php b/app/Console/Commands/QueueUpdates.php index 57b433d..267bfeb 100644 --- a/app/Console/Commands/QueueUpdates.php +++ b/app/Console/Commands/QueueUpdates.php @@ -63,7 +63,7 @@ function (Collection $chunk) use ($targetVersion) { $this->totalPushed += $chunk->count(); // Push each site check to queue - $chunk->each(fn ($site) => UpdateSite::dispatch($site, $targetVersion)); + $chunk->each(fn ($site) => UpdateSite::dispatch($site, $targetVersion)->onQueue('updates')); } ); diff --git a/app/Jobs/CheckSiteHealth.php b/app/Jobs/CheckSiteHealth.php index 8fc188e..f3568ce 100644 --- a/app/Jobs/CheckSiteHealth.php +++ b/app/Jobs/CheckSiteHealth.php @@ -8,14 +8,17 @@ use App\RemoteSite\Connection; use App\TUF\TufFetcher; use Carbon\Carbon; +use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Queue\Queueable; use Illuminate\Support\Facades\App; -class CheckSiteHealth implements ShouldQueue +class CheckSiteHealth implements ShouldQueue, ShouldBeUnique { use Queueable; + public int $uniqueFor = 120; + /** * Create a new job instance. */ @@ -23,6 +26,14 @@ public function __construct(protected readonly Site $site) { } + /** + * Get the unique ID for the job. + */ + public function uniqueId(): string + { + return $this->site->id; + } + /** * Execute the job. */ @@ -66,6 +77,6 @@ public function handle(): void UpdateSite::dispatch( $this->site, $latestVersion - ); + )->onQueue('updates'); } } diff --git a/app/Jobs/UpdateSite.php b/app/Jobs/UpdateSite.php index 4a1970a..6d59a1f 100644 --- a/app/Jobs/UpdateSite.php +++ b/app/Jobs/UpdateSite.php @@ -9,15 +9,17 @@ use App\RemoteSite\Connection; use App\RemoteSite\Responses\PrepareUpdate; use GuzzleHttp\Exception\RequestException; +use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Queue\Queueable; use Illuminate\Support\Facades\App; use Illuminate\Support\Facades\Log; -class UpdateSite implements ShouldQueue +class UpdateSite implements ShouldQueue, ShouldBeUnique { use Queueable; protected ?int $preUpdateCode = null; + public int $uniqueFor = 3600; /** * Create a new job instance. @@ -26,6 +28,14 @@ public function __construct(protected readonly Site $site, protected string $tar { } + /** + * Get the unique ID for the job. + */ + public function uniqueId(): string + { + return $this->site->id; + } + /** * Execute the job. */ diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index d25fa7b..106d81e 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -23,10 +23,22 @@ public function register(): void public function boot(): void { RateLimiter::for('site', function (Request $request) { - return Limit::perMinute(10)->by( - // @phpstan-ignore-next-line - parse_url((string) $request->input('url'), PHP_URL_HOST) - ); + $siteHost = 'default'; + $siteIp = 'default'; + + if (is_string($request->input('url'))) { + $siteHost = parse_url($request->input('url'), PHP_URL_HOST); + } + + if ($siteHost !== 'default' && $dnsResult = dns_get_record($siteHost, DNS_A)) { + $siteIp = $dnsResult[0]['ip']; + } + + return [ + Limit::perMinute(5)->by("sitehost-" . $siteHost), + Limit::perMinute(10)->by("siteip-" . $siteIp), + Limit::perMinute(50)->by("ip-" . $request->ip()) + ]; }); } } diff --git a/app/Providers/HttpclientServiceProvider.php b/app/Providers/HttpclientServiceProvider.php index 0fa2547..a66e8c3 100644 --- a/app/Providers/HttpclientServiceProvider.php +++ b/app/Providers/HttpclientServiceProvider.php @@ -21,42 +21,7 @@ class HttpclientServiceProvider extends ServiceProvider public function register() { $this->app->singleton(Client::class, function ($app) { - $handlerStack = HandlerStack::create(new CurlHandler()); - $handlerStack->push( - Middleware::retry( - function ( - $retries, - Request $request, - Response $response = null, - \Throwable $exception = null - ) { - // Limit the number of retries to 3 - if ($retries >= 3) { - return false; - } - - // Retry connection exceptions - if ($exception instanceof ConnectException) { - return true; - } - - if ($response) { - // Retry on server errors - if ($response->getStatusCode() >= 500) { - return true; - } - } - - return false; - }, - function ($numberOfRetries) { - return 1000 * $numberOfRetries; - } - ) - ); - return new Client([ - 'handler' => $handlerStack, 'allow_redirects' => [ 'max' => 5, 'strict' => true, // "strict" redirects - that's key as a redirected POST stays a POST diff --git a/app/RemoteSite/Connection.php b/app/RemoteSite/Connection.php index 3f32adf..87f2e83 100644 --- a/app/RemoteSite/Connection.php +++ b/app/RemoteSite/Connection.php @@ -94,7 +94,9 @@ public function performWebserviceRequest( "headers" => [ "Content-Type" => "application/json", "Accept" => "application/vnd.api+json" - ] + ], + 'timeout' => 60.0, + 'connect_timeout' => 5.0, ] ); diff --git a/config/horizon.php b/config/horizon.php index dfac10b..4ed7a6c 100644 --- a/config/horizon.php +++ b/config/horizon.php @@ -180,9 +180,9 @@ */ 'defaults' => [ - 'supervisor-1' => [ + 'supervisor-cron' => [ 'connection' => 'redis', - 'queue' => ['default'], + 'queue' => ['default', 'cron'], 'balance' => 'auto', 'autoScalingStrategy' => 'time', 'maxProcesses' => 1, @@ -190,22 +190,43 @@ 'maxJobs' => 0, 'memory' => 128, 'tries' => 1, - 'timeout' => 60, + 'timeout' => 15, + 'nice' => 0, + ], + 'supervisor-updates' => [ + 'connection' => 'redis', + 'queue' => ['updates'], + 'balance' => 'auto', + 'autoScalingStrategy' => 'time', + 'maxProcesses' => 1, + 'maxTime' => 0, + 'maxJobs' => 0, + 'memory' => 128, + 'tries' => 1, + 'timeout' => 600, 'nice' => 0, ], ], 'environments' => [ 'production' => [ - 'supervisor-1' => [ - 'maxProcesses' => 10, - 'balanceMaxShift' => 1, - 'balanceCooldown' => 3, + 'supervisor-cron' => [ + 'maxProcesses' => 250, + 'balanceMaxShift' => 10, + 'balanceCooldown' => 25, + ], + 'supervisor-updates' => [ + 'maxProcesses' => 500, + 'balanceMaxShift' => 10, + 'balanceCooldown' => 25, ], ], 'local' => [ - 'supervisor-1' => [ + 'supervisor-cron' => [ + 'maxProcesses' => 3, + ], + 'supervisor-updates' => [ 'maxProcesses' => 3, ], ], From 891b71882d37aacee377b9d689fa4cc7bab9e730 Mon Sep 17 00:00:00 2001 From: David Jardin Date: Thu, 22 May 2025 09:59:55 +0200 Subject: [PATCH 2/7] cs fix --- app/Console/Commands/QueueHealthChecks.php | 2 +- app/Jobs/CheckSiteHealth.php | 2 +- app/Jobs/UpdateSite.php | 4 ++-- app/Providers/AppServiceProvider.php | 2 +- app/Providers/HttpclientServiceProvider.php | 6 ------ config/horizon.php | 4 ++-- 6 files changed, 7 insertions(+), 13 deletions(-) diff --git a/app/Console/Commands/QueueHealthChecks.php b/app/Console/Commands/QueueHealthChecks.php index 09b8849..06cd153 100644 --- a/app/Console/Commands/QueueHealthChecks.php +++ b/app/Console/Commands/QueueHealthChecks.php @@ -48,7 +48,7 @@ function (Collection $chunk) { $this->totalPushed += $chunk->count(); // Push each site check to queue - $chunk->each(fn ($site) => CheckSiteHealth::dispatch($site)->onQueue('cron')); + $chunk->each(fn ($site) => CheckSiteHealth::dispatch($site)); } ); diff --git a/app/Jobs/CheckSiteHealth.php b/app/Jobs/CheckSiteHealth.php index f3568ce..a522bb5 100644 --- a/app/Jobs/CheckSiteHealth.php +++ b/app/Jobs/CheckSiteHealth.php @@ -31,7 +31,7 @@ public function __construct(protected readonly Site $site) */ public function uniqueId(): string { - return $this->site->id; + return (string) $this->site->id; } /** diff --git a/app/Jobs/UpdateSite.php b/app/Jobs/UpdateSite.php index 6d59a1f..a83f025 100644 --- a/app/Jobs/UpdateSite.php +++ b/app/Jobs/UpdateSite.php @@ -33,7 +33,7 @@ public function __construct(protected readonly Site $site, protected string $tar */ public function uniqueId(): string { - return $this->site->id; + return (string) $this->site->id; } /** @@ -146,7 +146,7 @@ public function handle(): void $connection->notificationSuccess(["fromVersion" => $healthResult->cms_version]); // Trigger site health check to write the update version back to the db - CheckSiteHealth::dispatch($this->site); + CheckSiteHealth::dispatch($this->site) } protected function performExtraction(PrepareUpdate $prepareResult): void diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 106d81e..cbd97d4 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -30,7 +30,7 @@ public function boot(): void $siteHost = parse_url($request->input('url'), PHP_URL_HOST); } - if ($siteHost !== 'default' && $dnsResult = dns_get_record($siteHost, DNS_A)) { + if ($siteHost !== 'default' && $dnsResult = dns_get_record((string) $siteHost, DNS_A)) { $siteIp = $dnsResult[0]['ip']; } diff --git a/app/Providers/HttpclientServiceProvider.php b/app/Providers/HttpclientServiceProvider.php index a66e8c3..8450c08 100644 --- a/app/Providers/HttpclientServiceProvider.php +++ b/app/Providers/HttpclientServiceProvider.php @@ -3,12 +3,6 @@ namespace App\Providers; use GuzzleHttp\Client; -use GuzzleHttp\Exception\ConnectException; -use GuzzleHttp\Handler\CurlHandler; -use GuzzleHttp\HandlerStack; -use GuzzleHttp\Middleware; -use GuzzleHttp\Psr7\Request; -use GuzzleHttp\Psr7\Response; use Illuminate\Support\ServiceProvider; class HttpclientServiceProvider extends ServiceProvider diff --git a/config/horizon.php b/config/horizon.php index 4ed7a6c..9a5eee3 100644 --- a/config/horizon.php +++ b/config/horizon.php @@ -180,9 +180,9 @@ */ 'defaults' => [ - 'supervisor-cron' => [ + 'supervisor-default' => [ 'connection' => 'redis', - 'queue' => ['default', 'cron'], + 'queue' => ['default'], 'balance' => 'auto', 'autoScalingStrategy' => 'time', 'maxProcesses' => 1, From 32b5b75f9571e3851ecc75ea11140f42a8641eb8 Mon Sep 17 00:00:00 2001 From: David Jardin Date: Thu, 22 May 2025 10:03:11 +0200 Subject: [PATCH 3/7] syntax fix --- app/Jobs/UpdateSite.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Jobs/UpdateSite.php b/app/Jobs/UpdateSite.php index a83f025..c2bfb7e 100644 --- a/app/Jobs/UpdateSite.php +++ b/app/Jobs/UpdateSite.php @@ -146,7 +146,7 @@ public function handle(): void $connection->notificationSuccess(["fromVersion" => $healthResult->cms_version]); // Trigger site health check to write the update version back to the db - CheckSiteHealth::dispatch($this->site) + CheckSiteHealth::dispatch($this->site); } protected function performExtraction(PrepareUpdate $prepareResult): void From 02378384c45483a1c0aa9af97f373f853739ad6f Mon Sep 17 00:00:00 2001 From: David Jardin Date: Thu, 22 May 2025 10:05:23 +0200 Subject: [PATCH 4/7] fix label --- config/horizon.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/horizon.php b/config/horizon.php index 9a5eee3..26b2c2d 100644 --- a/config/horizon.php +++ b/config/horizon.php @@ -210,7 +210,7 @@ 'environments' => [ 'production' => [ - 'supervisor-cron' => [ + 'supervisor-default' => [ 'maxProcesses' => 250, 'balanceMaxShift' => 10, 'balanceCooldown' => 25, @@ -223,7 +223,7 @@ ], 'local' => [ - 'supervisor-cron' => [ + 'supervisor-default' => [ 'maxProcesses' => 3, ], 'supervisor-updates' => [ From f0001b41efe5818a7575334c9ed5e6f627bbd2f0 Mon Sep 17 00:00:00 2001 From: David Jardin Date: Thu, 22 May 2025 14:59:01 +0200 Subject: [PATCH 5/7] apply rate limit for all target ips --- app/Providers/AppServiceProvider.php | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index cbd97d4..4535460 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -2,6 +2,7 @@ namespace App\Providers; +use App\Network\DNSLookup; use Illuminate\Cache\RateLimiting\Limit; use Illuminate\Http\Request; use Illuminate\Support\Facades\RateLimiter; @@ -24,20 +25,30 @@ public function boot(): void { RateLimiter::for('site', function (Request $request) { $siteHost = 'default'; - $siteIp = 'default'; if (is_string($request->input('url'))) { $siteHost = parse_url($request->input('url'), PHP_URL_HOST); } - if ($siteHost !== 'default' && $dnsResult = dns_get_record((string) $siteHost, DNS_A)) { - $siteIp = $dnsResult[0]['ip']; + // Define a rate limit per target IP + $siteIpLimits = []; + + if ($siteHost !== 'default') { + $siteIps = (new DNSLookup)->getIPs($siteHost); + + foreach ($siteIps as $siteIp) { + $siteIpLimits[] = Limit::perMinute(5)->by("siteip-" . $siteIp); + } + } + + if (!count($siteIpLimits)) { + $siteIpLimits = [Limit::perMinute(5)->by("siteip-default")]; } return [ Limit::perMinute(5)->by("sitehost-" . $siteHost), - Limit::perMinute(10)->by("siteip-" . $siteIp), - Limit::perMinute(50)->by("ip-" . $request->ip()) + Limit::perMinute(50)->by("requestip-" . $request->ip()), + ...$siteIpLimits ]; }); } From 7199b2f212f0befcff74a5d82eb8623afbcf994c Mon Sep 17 00:00:00 2001 From: David Jardin Date: Thu, 22 May 2025 15:00:30 +0200 Subject: [PATCH 6/7] fix stan --- app/Providers/AppServiceProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 4535460..2ad4d6e 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -27,7 +27,7 @@ public function boot(): void $siteHost = 'default'; if (is_string($request->input('url'))) { - $siteHost = parse_url($request->input('url'), PHP_URL_HOST); + $siteHost = (string) parse_url($request->input('url'), PHP_URL_HOST); } // Define a rate limit per target IP From 81d0df1cba3ac2431a0ad28091c4553f53f4af64 Mon Sep 17 00:00:00 2001 From: David Jardin Date: Thu, 22 May 2025 15:00:49 +0200 Subject: [PATCH 7/7] cs fix --- app/Providers/AppServiceProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 2ad4d6e..7027757 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -34,7 +34,7 @@ public function boot(): void $siteIpLimits = []; if ($siteHost !== 'default') { - $siteIps = (new DNSLookup)->getIPs($siteHost); + $siteIps = (new DNSLookup())->getIPs($siteHost); foreach ($siteIps as $siteIp) { $siteIpLimits[] = Limit::perMinute(5)->by("siteip-" . $siteIp);