From 798e3aafc9f8ab9d69fd28d04e74c26a1f146d9f Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 5 Oct 2026 21:19:44 +0200 Subject: [PATCH] fix(server): retry a manual Docker cleanup while another cleanup runs A manual cleanup that started while a stop-triggered cleanup held the server lock was dropped. The UI said it was queued, but it never ran. - Release a manual run and try again after 60 seconds - Stop the retries 780 seconds after dispatch (the lock expiry plus a margin) - Scheduled and stop-triggered runs are still dropped --- app/Jobs/DockerCleanupJob.php | 38 +++++++++-- tests/Feature/DockerCleanupJobTest.php | 87 ++++++++++++++++++++++++-- 2 files changed, 114 insertions(+), 11 deletions(-) diff --git a/app/Jobs/DockerCleanupJob.php b/app/Jobs/DockerCleanupJob.php index 03a9207827..6730e7a976 100644 --- a/app/Jobs/DockerCleanupJob.php +++ b/app/Jobs/DockerCleanupJob.php @@ -27,24 +27,52 @@ class DockerCleanupJob implements ShouldBeEncrypted, ShouldQueue public ?string $usageBefore = null; + /** + * A manual run waits for a running cleanup (released and tried again), so its exceptions must not + * count as reasons to try again. + */ + public $maxExceptions = 1; + + /** + * Seconds a manual run waits before it tries again while another cleanup holds the server lock. + */ + public const MANUAL_RELEASE_DELAY = 60; + public ?DockerCleanupExecution $execution_log = null; /** * Shares the per-server lock with queued CleanupDocker runs, so two cleanups never run on * one server at once. A scheduled run that finds the lock held marks its occurrence - * skipped (not missed); a manual run is dropped as before. The lock outlives the job + * skipped (not missed); a stop-triggered run is dropped. A manual run is released and tried + * again until the running cleanup finishes (see retryUntil()). The lock outlives the job * timeout, so a new run cannot start while a timed-out run is still finishing. */ public function middleware(): array { + $withoutOverlapping = ScheduledJobDeliveryService::withoutOverlapping(CleanupDocker::overlapLockKey($this->server), $this->occurrenceUuid) + ->shared() + ->expireAfter(CleanupDocker::overlapLockExpiresAfter()); + return [ - ScheduledJobDeliveryService::withoutOverlapping(CleanupDocker::overlapLockKey($this->server), $this->occurrenceUuid) - ->shared() - ->expireAfter(CleanupDocker::overlapLockExpiresAfter()) - ->dontRelease(), + $this->manualCleanup + ? $withoutOverlapping->releaseAfter(self::MANUAL_RELEASE_DELAY) + : $withoutOverlapping->dontRelease(), ]; } + /** + * A manual run may wait for one full cleanup that holds the lock. Fixed at dispatch, so a manual run of a + * killed worker, which comes back after retry_after, fails instead of running late. Other runs try once. + */ + public function retryUntil(): ?\DateTimeInterface + { + if (! $this->manualCleanup) { + return null; + } + + return now()->addSeconds(CleanupDocker::overlapLockExpiresAfter() + self::MANUAL_RELEASE_DELAY * 2); + } + public function __construct( public Server $server, public bool $manualCleanup = false, diff --git a/tests/Feature/DockerCleanupJobTest.php b/tests/Feature/DockerCleanupJobTest.php index 85607cabf4..37cafbd54d 100644 --- a/tests/Feature/DockerCleanupJobTest.php +++ b/tests/Feature/DockerCleanupJobTest.php @@ -1,6 +1,7 @@ team, DockerCleanupFailed::class); }); -it('drops a manual cleanup while another cleanup holds the server lock without touching occurrences', function () { +function dockerCleanupDatabaseQueue(): void +{ + Schema::create('jobs', function (Blueprint $table) { + $table->id(); + $table->string('queue')->index(); + $table->longText('payload'); + $table->unsignedTinyInteger('attempts'); + $table->unsignedInteger('reserved_at')->nullable(); + $table->unsignedInteger('available_at'); + $table->unsignedInteger('created_at'); + }); + config(['queue.default' => 'database']); +} + +function dockerCleanupWorkOnce(): void +{ + Artisan::call('queue:work', ['connection' => 'database', '--once' => true, '--queue' => maintenance_queue(), '--sleep' => 0]); +} + +it('runs a manual cleanup after another cleanup releases the server lock instead of dropping it', function () { + Carbon::setTestNow(Carbon::create(2026, 9, 17, 0, 0, 0, 'UTC')); + Notification::fake(); + Event::fake([DockerCleanupDone::class]); + dockerCleanupDatabaseQueue(); $server = dockerCleanupReachableServer(); - $job = new DockerCleanupJob($server, true); - $runningCleanupLock = Cache::lock($job->middleware()[0]->getLockKey($job), 3600); + $runningCleanupLock = Cache::lock('laravel-queue-overlap:docker-cleanup-'.$server->uuid, 3600); $runningCleanupLock->get(); $commands = dockerCleanupFakeProcesses(); - dispatch_sync($job); + DockerCleanupJob::dispatch($server, true); + dockerCleanupWorkOnce(); + + expect(DB::table('jobs')->count())->toBe(1) + ->and(DB::table('jobs')->value('available_at'))->toBe(now()->addSeconds(DockerCleanupJob::MANUAL_RELEASE_DELAY)->getTimestamp()) + ->and(DockerCleanupExecution::query()->exists())->toBeFalse() + ->and($commands)->toBeEmpty(); + + $runningCleanupLock->release(); + Carbon::setTestNow(now()->addSeconds(DockerCleanupJob::MANUAL_RELEASE_DELAY + 1)); + dockerCleanupWorkOnce(); + + expect(DB::table('jobs')->count())->toBe(0) + ->and(DB::table('failed_jobs')->count())->toBe(0) + ->and(DockerCleanupExecution::query()->sole()->status)->toBe('success') + ->and(ScheduledJobDelivery::query()->exists())->toBeFalse(); +}); + +it('fails a waiting manual cleanup instead of running it after its wait window', function () { + Carbon::setTestNow(Carbon::create(2026, 9, 17, 0, 0, 0, 'UTC')); + dockerCleanupDatabaseQueue(); + $server = dockerCleanupReachableServer(); + $runningCleanupLock = Cache::lock('laravel-queue-overlap:docker-cleanup-'.$server->uuid, 3600); + $runningCleanupLock->get(); + $commands = dockerCleanupFakeProcesses(); + + DockerCleanupJob::dispatch($server, true); + dockerCleanupWorkOnce(); + $runningCleanupLock->release(); + Carbon::setTestNow(now()->addDay()); + dockerCleanupWorkOnce(); + + expect(DB::table('jobs')->count())->toBe(0) + ->and(DB::table('failed_jobs')->count())->toBe(1) + ->and(DockerCleanupExecution::query()->exists())->toBeFalse() + ->and($commands)->toBeEmpty(); +}); + +it('drops a stop-triggered cleanup and a scheduled run while another cleanup holds the server lock', function () { + dockerCleanupDatabaseQueue(); + $server = dockerCleanupReachableServer(); + $runningCleanupLock = Cache::lock('laravel-queue-overlap:docker-cleanup-'.$server->uuid, 3600); + $runningCleanupLock->get(); + $commands = dockerCleanupFakeProcesses(); + + DockerCleanupJob::dispatch($server); + dockerCleanupWorkOnce(); $runningCleanupLock->release(); - expect(DockerCleanupExecution::query()->exists())->toBeFalse() - ->and(ScheduledJobDelivery::query()->exists())->toBeFalse() + expect(DB::table('jobs')->count())->toBe(0) + ->and(DB::table('failed_jobs')->count())->toBe(0) + ->and(DockerCleanupExecution::query()->exists())->toBeFalse() ->and($commands)->toBeEmpty(); });