From c681dc0d0e95bd2d2093ca61305d410c84e0ec08 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sat, 3 Oct 2026 21:25:00 +0200 Subject: [PATCH] fix(queue): coalesce queued container status and helper cleanup jobs per server Queued GetContainersStatus runs are unique per server until a worker starts them, and CleanupHelperContainersJob locks per server instead of class-wide. Add UniqueUntilProcessingJobDecorator for laravel-actions. --- app/Actions/Docker/GetContainersStatus.php | 28 ++++++++- app/Jobs/CleanupHelperContainersJob.php | 13 ++++ .../UniqueUntilProcessingJobDecorator.php | 14 +++++ .../CleanupHelperContainersJobDedupTest.php | 26 ++++++++ .../Feature/GetContainersStatusDedupTest.php | 61 +++++++++++++++++++ 5 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 app/Support/Actions/UniqueUntilProcessingJobDecorator.php create mode 100644 tests/Feature/CleanupHelperContainersJobDedupTest.php create mode 100644 tests/Feature/GetContainersStatusDedupTest.php diff --git a/app/Actions/Docker/GetContainersStatus.php b/app/Actions/Docker/GetContainersStatus.php index aabecdbd2e..5ce2a90f6e 100644 --- a/app/Actions/Docker/GetContainersStatus.php +++ b/app/Actions/Docker/GetContainersStatus.php @@ -16,17 +16,33 @@ use App\Models\ServiceDatabase; use App\Notifications\Application\RestartLimitReached as ApplicationRestartLimitReached; use App\Services\ContainerStatusAggregator; use App\Services\RestartCountTracker; +use App\Support\Actions\UniqueUntilProcessingJobDecorator; +use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Support\Arr; use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; use Lorisleiva\Actions\Concerns\AsAction; +use Lorisleiva\Actions\Decorators\UniqueJobDecorator; -class GetContainersStatus +/** + * Queued runs are coalesced per server: while one run waits in the queue, further dispatches are + * dropped because the waiting run reads the same container snapshot. The lock is released when a + * worker starts the run, so a request made during a run still queues one follow-up. + * Synchronous run() calls are not affected. + */ +class GetContainersStatus implements ShouldBeUnique { use AsAction; public string $jobQueue = 'high'; + public int $jobTimeout = 120; + + /** + * Bounds how long a lost queued run (for example a killed worker) blocks new refreshes. + */ + public int $jobUniqueFor = 60; + public $applications; public ?Collection $containers; @@ -45,6 +61,16 @@ class GetContainersStatus protected ?Collection $serviceContainerRestartCounts; + public static function makeUniqueJob(mixed ...$arguments): UniqueJobDecorator + { + return new UniqueUntilProcessingJobDecorator(static::class, ...$arguments); + } + + public function getJobUniqueId(Server $server): string + { + return $server->uuid; + } + public function handle(Server $server, ?Collection $containers = null, ?Collection $containerReplicates = null) { $this->containers = $containers; diff --git a/app/Jobs/CleanupHelperContainersJob.php b/app/Jobs/CleanupHelperContainersJob.php index 52b4064feb..368bad5eaf 100644 --- a/app/Jobs/CleanupHelperContainersJob.php +++ b/app/Jobs/CleanupHelperContainersJob.php @@ -17,8 +17,21 @@ class CleanupHelperContainersJob implements ShouldBeEncrypted, ShouldBeUnique, S { use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; + /** + * Release the per-server lock after an hour if a worker dies before finishing. + */ + public int $uniqueFor = 3600; + public function __construct(public Server $server) {} + /** + * Keep the lock per server; a class-wide lock queued only one server per cleanup run. + */ + public function uniqueId(): string + { + return $this->server->uuid; + } + private static function helperContainersCommand(): string { return 'docker container ps --format \'{{json .}}\' | jq -s \'map(select(.Image|test("(^|/)coollabsio/coolify-helper(:|@)")))\''; diff --git a/app/Support/Actions/UniqueUntilProcessingJobDecorator.php b/app/Support/Actions/UniqueUntilProcessingJobDecorator.php new file mode 100644 index 0000000000..176d976987 --- /dev/null +++ b/app/Support/Actions/UniqueUntilProcessingJobDecorator.php @@ -0,0 +1,14 @@ +team = Team::factory()->create(); +}); + +it('queues one helper container cleanup per server', function () { + Queue::fake(); + $first = Server::factory()->create(['team_id' => $this->team->id]); + $second = Server::factory()->create(['team_id' => $this->team->id]); + + CleanupHelperContainersJob::dispatch($first); + CleanupHelperContainersJob::dispatch($second); + CleanupHelperContainersJob::dispatch($first); + + Queue::assertPushed(CleanupHelperContainersJob::class, 2); +}); diff --git a/tests/Feature/GetContainersStatusDedupTest.php b/tests/Feature/GetContainersStatusDedupTest.php new file mode 100644 index 0000000000..b1b1950214 --- /dev/null +++ b/tests/Feature/GetContainersStatusDedupTest.php @@ -0,0 +1,61 @@ +team = Team::factory()->create(); +}); + +afterEach(function () { + GetContainersStatus::clearFake(); +}); + +it('queues one status refresh per server while one is waiting', function () { + Queue::fake(); + $first = Server::factory()->create(['team_id' => $this->team->id]); + $second = Server::factory()->create(['team_id' => $this->team->id]); + + GetContainersStatus::dispatch($first); + GetContainersStatus::dispatch($first); + GetContainersStatus::dispatch($second); + + Queue::assertPushed(UniqueUntilProcessingJobDecorator::class, 2); + Queue::assertPushedOn('high', UniqueUntilProcessingJobDecorator::class); + Queue::assertPushed(UniqueUntilProcessingJobDecorator::class, fn ($job) => $job->timeout === 120); +}); + +it('accepts a new status refresh after the waiting one is lost', function () { + Queue::fake(); + $server = Server::factory()->create(['team_id' => $this->team->id]); + + GetContainersStatus::dispatch($server); + $this->travel(61)->seconds(); + GetContainersStatus::dispatch($server); + + Queue::assertPushed(UniqueUntilProcessingJobDecorator::class, 2); +}); + +it('queues a follow-up status refresh requested while one is running', function () { + $server = Server::factory()->create(['team_id' => $this->team->id]); + $runs = 0; + GetContainersStatus::partialMock() + ->shouldReceive('handle') + ->andReturnUsing(function (Server $server) use (&$runs) { + $runs++; + if ($runs === 1) { + GetContainersStatus::dispatch($server); + } + }); + + GetContainersStatus::dispatch($server); + + expect($runs)->toBe(2); +});