From f9d10c22fbcfa5259530968e8dfbcd2517f6520e Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Fri, 2 Oct 2026 09:35:13 +0200 Subject: [PATCH] fix(backups): stop volume backup recovery retry storms The scheduler dispatched VolumeBackupRecoveryJob every minute for each pending execution, and each job retried 20 times with a 60 second backoff. A stuck execution kept about 20 failing jobs active at a time. - Run each recovery job once; the scheduler is the only retry path. - Make the job unique per execution so duplicates do not pile up. - Dispatch recovery for an execution at most once every 5 minutes. - Clear the pending flag and record the reason when recovery cannot succeed (server, resource, S3 storage, or filename no longer exists), so it does not fail forever and block new backups. Co-Authored-By: Claude Opus 5.5 --- app/Jobs/ScheduledJobManager.php | 9 +++- app/Jobs/VolumeBackupRecoveryJob.php | 49 ++++++++++++++--- tests/Feature/VolumeBackupTest.php | 79 ++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 7 deletions(-) diff --git a/app/Jobs/ScheduledJobManager.php b/app/Jobs/ScheduledJobManager.php index 0ec09c361f..c5d553d850 100644 --- a/app/Jobs/ScheduledJobManager.php +++ b/app/Jobs/ScheduledJobManager.php @@ -33,6 +33,11 @@ class ScheduledJobManager implements ShouldQueue private const CHUNK_SIZE = 100; + /** + * Pending volume backup recovery is retried at most once in this interval for each execution. + */ + private const VOLUME_BACKUP_RECOVERY_INTERVAL_MINUTES = 5; + /** * The schedule types that run as separate dispatchers, with the delivery job types they own. * Each type has its own overlap lock, so a slow type cannot make another type skip a run. @@ -481,7 +486,9 @@ class ScheduledJobManager implements ShouldQueue ->orWhere('s3_cleanup_pending', true)) ->chunkById(self::CHUNK_SIZE, function ($executions): void { foreach ($executions as $execution) { - VolumeBackupRecoveryJob::dispatch($execution); + if (Cache::add("volume-backup-recovery-dispatched:{$execution->id}", true, now()->addMinutes(self::VOLUME_BACKUP_RECOVERY_INTERVAL_MINUTES))) { + VolumeBackupRecoveryJob::dispatch($execution); + } } }); } diff --git a/app/Jobs/VolumeBackupRecoveryJob.php b/app/Jobs/VolumeBackupRecoveryJob.php index a3465b192e..c136c45820 100644 --- a/app/Jobs/VolumeBackupRecoveryJob.php +++ b/app/Jobs/VolumeBackupRecoveryJob.php @@ -5,27 +5,37 @@ namespace App\Jobs; use App\Models\ScheduledVolumeBackupExecution; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldBeEncrypted; +use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\Middleware\WithoutOverlapping; use Illuminate\Queue\SerializesModels; +use Illuminate\Support\Facades\Log; -class VolumeBackupRecoveryJob implements ShouldBeEncrypted, ShouldQueue +class VolumeBackupRecoveryJob implements ShouldBeEncrypted, ShouldBeUnique, ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; - public int $tries = 20; - - public int $backoff = 60; + /** + * ScheduledJobManager dispatches pending recovery again, so this job does not retry by itself. + */ + public int $tries = 1; public int $timeout = 120; + public int $uniqueFor = 300; + public function __construct(public ScheduledVolumeBackupExecution $execution) { $this->onQueue(crons_queue()); } + public function uniqueId(): string + { + return (string) $this->execution->id; + } + public function middleware(): array { return [ @@ -62,7 +72,13 @@ class VolumeBackupRecoveryJob implements ShouldBeEncrypted, ShouldQueue $server = $execution->scheduledVolumeBackup?->server(); if (! $server) { - throw new \RuntimeException('The server is unavailable for container recovery.'); + self::skipRecovery( + $execution, + ['stop_container_ids' => null, 'stop_recovery_pending' => false], + 'Container recovery was skipped because the server or resource no longer exists.', + ); + + return; } $stateFile = self::stateFile($execution); @@ -100,7 +116,13 @@ class VolumeBackupRecoveryJob implements ShouldBeEncrypted, ShouldQueue $s3 = $execution->s3; if (! $s3 || blank($execution->filename)) { - throw new \RuntimeException('The S3 storage or backup filename is unavailable for upload cleanup.'); + self::skipRecovery( + $execution, + ['s3_cleanup_pending' => false], + 'S3 upload cleanup was skipped because the S3 storage or backup filename no longer exists.', + ); + + return; } deleteBackupsS3($execution->filename, $s3); @@ -110,6 +132,21 @@ class VolumeBackupRecoveryJob implements ShouldBeEncrypted, ShouldQueue ]); } + /** + * Clears a pending recovery that cannot succeed, so it does not retry forever and block new backups. + * + * @param array $attributes + */ + private static function skipRecovery(ScheduledVolumeBackupExecution $execution, array $attributes, string $reason): void + { + Log::channel('scheduled-errors')->warning($reason, ['execution_id' => $execution->id]); + + $execution->update([ + ...$attributes, + 'message' => trim(($execution->message ?? '').' '.$reason), + ]); + } + public static function stateFile(ScheduledVolumeBackupExecution $execution): string { return '/tmp/coolify-volume-backup-'.$execution->uuid.'.stopped'; diff --git a/tests/Feature/VolumeBackupTest.php b/tests/Feature/VolumeBackupTest.php index 623e140dfb..1f733f72d4 100644 --- a/tests/Feature/VolumeBackupTest.php +++ b/tests/Feature/VolumeBackupTest.php @@ -2614,3 +2614,82 @@ it('dispatches pending recovery without starting another volume backup', functio ); Queue::assertNotPushed(VolumeBackupJob::class); }); + +it('dispatches pending recovery for an execution at most once every five minutes', function () { + config(['constants.coolify.self_hosted' => true]); + Carbon::setTestNow('2026-07-15 12:00:00'); + Queue::fake(); + $team = Team::factory()->create(); + [$application, $volume] = createVolumeBackupApplication($team); + $backup = $volume->scheduledBackups()->create([ + 'team_id' => $team->id, + 'frequency' => 'daily', + 'enabled' => true, + ]); + ScheduledVolumeBackupExecution::create([ + 'scheduled_volume_backup_id' => $backup->id, + 'status' => 'failed', + 'stop_recovery_pending' => true, + ]); + + (new ScheduledJobManager)->handle(); + Carbon::setTestNow('2026-07-15 12:04:00'); + (new ScheduledJobManager)->handle(); + + Queue::assertPushed(VolumeBackupRecoveryJob::class, 1); + + Carbon::setTestNow('2026-07-15 12:05:01'); + (new ScheduledJobManager)->handle(); + + Queue::assertPushed(VolumeBackupRecoveryJob::class, 2); +}); + +it('stops container recovery when the server no longer exists', function () { + Process::fake(); + $team = Team::factory()->create(); + [$application, $volume] = createVolumeBackupApplication($team); + $backup = $volume->scheduledBackups()->create([ + 'team_id' => $team->id, + 'frequency' => 'daily', + ]); + $execution = ScheduledVolumeBackupExecution::create([ + 'scheduled_volume_backup_id' => $backup->id, + 'status' => 'failed', + 'message' => 'Worker timed out', + 'stop_recovery_pending' => true, + ]); + $application->update([ + 'destination_id' => null, + 'destination_type' => null, + ]); + + (new VolumeBackupRecoveryJob($execution))->handle(); + + expect($execution->fresh()->stop_recovery_pending)->toBeFalse() + ->and($execution->fresh()->message)->toStartWith('Worker timed out') + ->and($execution->fresh()->message)->toContain('Container recovery was skipped'); + Process::assertNothingRan(); +}); + +it('stops S3 upload cleanup when the S3 storage no longer exists', function () { + $team = Team::factory()->create(); + [$application, $volume] = createVolumeBackupApplication($team); + $backup = $volume->scheduledBackups()->create([ + 'team_id' => $team->id, + 'frequency' => 'daily', + ]); + $execution = ScheduledVolumeBackupExecution::create([ + 'scheduled_volume_backup_id' => $backup->id, + 's3_storage_id' => null, + 'status' => 'failed', + 'filename' => '/data/coolify/backups/volumes/test/interrupted.tar.gz', + 's3_cleanup_pending' => true, + ]); + Storage::shouldReceive('build')->never(); + + (new VolumeBackupRecoveryJob($execution))->handle(); + + expect($execution->fresh()->s3_cleanup_pending)->toBeFalse() + ->and($execution->fresh()->s3_storage_deleted)->toBeFalsy() + ->and($execution->fresh()->message)->toContain('S3 upload cleanup was skipped'); +});