starred/coolify
1
0
Fork 0
mirror of https://github.com/coollabsio/coolify.git synced 2026-09-27 17:55:59 -04:00
Code Issues Packages Projects Releases Wiki Activity

feat(jobs): improve scheduled tasks with retry logic and queue cleanup

Browse Source 
- Add retry configuration to CoolifyTask (3 tries, 600s timeout)
- Add retry configuration to ScheduledTaskJob (3 tries, configurable timeout)
- Add retry configuration to DatabaseBackupJob (2 tries)
- Implement exponential backoff for all jobs (30s, 60s, 120s intervals)
- Add failed() handlers with comprehensive error logging to scheduled-errors channel
- Add execution tracking: started_at, retry_count, duration (decimal), error_details
- Add configurable timeout field to scheduled tasks (60-3600s, default 300s)
- Update UI to include timeout configuration in task creation/editing forms
- Increase ScheduledJobManager lock expiration from 60s to 90s for high-load environments
- Implement safe queue cleanup with restart vs runtime modes
  - Restart mode: aggressive cleanup (marks all processing jobs as failed)
  - Runtime mode: conservative cleanup (only marks jobs >12h as failed, skips deployments)
- Add cleanup:redis --restart flag for system startup
- Integrate cleanup into Dev.php init() for development environment
- Increase scheduled-errors log retention from 7 to 14 days
- Create comprehensive test suite (unit and feature tests)
- Add TESTING_GUIDE.md with manual testing instructions

Fixes issues with jobs failing after single attempt and "attempted too many times" errors
This commit is contained in:
Andras Bacsai
2025-11-10 11:11:18 +01:00
parent 775216e7a5
commit b22e79caec
22 changed files with 762 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/Console/Commands/CleanupRedis.php
+103 -1
View File
@@ -7,7 +7,7 @@ use Illuminate\Support\Facades\Redis;
class CleanupRedis extends Command
{
protected $signature = 'cleanup:redis {--dry-run : Show what would be deleted without actually deleting} {--skip-overlapping : Skip overlapping queue cleanup} {--clear-locks : Clear stale WithoutOverlapping locks}';
protected $signature = 'cleanup:redis {--dry-run : Show what would be deleted without actually deleting} {--skip-overlapping : Skip overlapping queue cleanup} {--clear-locks : Clear stale WithoutOverlapping locks} {--restart : Aggressive cleanup mode for system restart (marks all processing jobs as failed)}';
protected $description = 'Cleanup Redis (Horizon jobs, metrics, overlapping queues, cache locks, and related data)';
@@ -63,6 +63,14 @@ class CleanupRedis extends Command
$deletedCount += $locksCleaned;
}
// Clean up stuck jobs (restart mode = aggressive, runtime mode = conservative)
$isRestart = $this->option('restart');
if ($isRestart || $this->option('clear-locks')) {
$this->info($isRestart ? 'Cleaning up stuck jobs (RESTART MODE - aggressive)...' : 'Checking for stuck jobs (runtime mode - conservative)...');
$jobsCleaned = $this->cleanupStuckJobs($redis, $prefix, $dryRun, $isRestart);
$deletedCount += $jobsCleaned;
}
if ($dryRun) {
$this->info("DRY RUN: Would delete {$deletedCount} out of {$totalKeys} keys");
} else {
@@ -332,4 +340,98 @@ class CleanupRedis extends Command
return $cleanedCount;
}
/**
* Clean up stuck jobs based on mode (restart vs runtime).
*
* @param mixed $redis Redis connection
* @param string $prefix Horizon prefix
* @param bool $dryRun Dry run mode
* @param bool $isRestart Restart mode (aggressive) vs runtime mode (conservative)
* @return int Number of jobs cleaned
*/
private function cleanupStuckJobs($redis, string $prefix, bool $dryRun, bool $isRestart): int
{
$cleanedCount = 0;
$now = time();
// Get all keys with the horizon prefix
$keys = $redis->keys('*');
foreach ($keys as $key) {
$keyWithoutPrefix = str_replace($prefix, '', $key);
$type = $redis->command('type', [$keyWithoutPrefix]);
// Only process hash-type keys (individual jobs)
if ($type !== 5) {
continue;
}
$data = $redis->command('hgetall', [$keyWithoutPrefix]);
$status = data_get($data, 'status');
$payload = data_get($data, 'payload');
// Only process jobs in "processing" or "reserved" state
if (! in_array($status, ['processing', 'reserved'])) {
continue;
}
// Parse job payload to get job class and started time
$payloadData = json_decode($payload, true);
$jobClass = data_get($payloadData, 'displayName', 'Unknown');
$pushedAt = (int) data_get($data, 'pushed_at', 0);
// Calculate how long the job has been processing
$processingTime = $now - $pushedAt;
$shouldFail = false;
$reason = '';
if ($isRestart) {
// RESTART MODE: Mark ALL processing/reserved jobs as failed
// Safe because all workers are dead on restart
$shouldFail = true;
$reason = 'System restart - all workers terminated';
} else {
// RUNTIME MODE: Only mark truly stuck jobs as failed
// Be conservative to avoid killing legitimate long-running jobs
// Skip ApplicationDeploymentJob entirely (has dynamic_timeout, can run 2+ hours)
if (str_contains($jobClass, 'ApplicationDeploymentJob')) {
continue;
}
// Skip DatabaseBackupJob (large backups can take hours)
if (str_contains($jobClass, 'DatabaseBackupJob')) {
continue;
}
// For other jobs, only fail if processing > 12 hours
if ($processingTime > 43200) { // 12 hours
$shouldFail = true;
$reason = 'Processing for more than 12 hours';
}
}
if ($shouldFail) {
if ($dryRun) {
$this->warn(" Would mark as FAILED: {$jobClass} (processing for ".round($processingTime / 60, 1)." min) - {$reason}");
} else {
// Mark job as failed
$redis->command('hset', [$keyWithoutPrefix, 'status', 'failed']);
$redis->command('hset', [$keyWithoutPrefix, 'failed_at', $now]);
$redis->command('hset', [$keyWithoutPrefix, 'exception', "Job cleaned up by cleanup:redis - {$reason}"]);
$this->info(" ✓ Marked as FAILED: {$jobClass} (processing for ".round($processingTime / 60, 1).' min) - '.$reason);
}
$cleanedCount++;
}
}
if ($cleanedCount === 0) {
$this->info($isRestart ? ' No jobs to clean up' : ' No stuck jobs found (all jobs running normally)');
}
return $cleanedCount;
}
}
app/Console/Commands/Dev.php
+10
View File
@@ -45,6 +45,16 @@ class Dev extends Command
} else {
echo "Instance already initialized.\n";
}
// Clean up stuck jobs and stale locks on development startup
try {
echo "Cleaning up Redis (stuck jobs and stale locks)...\n";
Artisan::call('cleanup:redis', ['--restart' => true, '--clear-locks' => true]);
echo "Redis cleanup completed.\n";
} catch (\Throwable $e) {
echo "Error in cleanup:redis: {$e->getMessage()}\n";
}
CheckHelperImageJob::dispatch();
}
}
app/Console/Commands/Init.php
+1 -1
View File
@@ -73,7 +73,7 @@ class Init extends Command
$this->cleanupUnusedNetworkFromCoolifyProxy();
try {
$this->call('cleanup:redis', ['--clear-locks' => true]);
$this->call('cleanup:redis', ['--restart' => true, '--clear-locks' => true]);
} catch (\Throwable $e) {
echo "Error in cleanup:redis command: {$e->getMessage()}\n";
}
app/Jobs/CoolifyTask.php
+49
View File
@@ -3,18 +3,35 @@
namespace App\Jobs;
use App\Actions\CoolifyTask\RunRemoteProcess;
use App\Enums\ProcessStatus;
use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldBeEncrypted;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\SerializesModels;
use Illuminate\Support\Facades\Log;
use Spatie\Activitylog\Models\Activity;
class CoolifyTask implements ShouldBeEncrypted, ShouldQueue
{
use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
/**
* The number of times the job may be attempted.
*/
public $tries = 3;
/**
* The maximum number of unhandled exceptions to allow before failing.
*/
public $maxExceptions = 1;
/**
* The number of seconds the job can run before timing out.
*/
public $timeout = 600;
/**
* Create a new job instance.
*/
@@ -42,4 +59,36 @@ class CoolifyTask implements ShouldBeEncrypted, ShouldQueue
$remote_process();
}
/**
* Calculate the number of seconds to wait before retrying the job.
*/
public function backoff(): array
{
return [30, 90, 180]; // 30s, 90s, 180s between retries
}
/**
* Handle a job failure.
*/
public function failed(?\Throwable $exception): void
{
Log::channel('scheduled-errors')->error('CoolifyTask permanently failed', [
'job' => 'CoolifyTask',
'activity_id' => $this->activity->id,
'server_uuid' => $this->activity->getExtraProperty('server_uuid'),
'command_preview' => substr($this->activity->getExtraProperty('command') ?? '', 0, 200),
'error' => $exception?->getMessage(),
'total_attempts' => $this->attempts(),
'trace' => $exception?->getTraceAsString(),
]);
// Update activity status to reflect permanent failure
$this->activity->properties = $this->activity->properties->merge([
'status' => ProcessStatus::ERROR->value,
'error' => $exception?->getMessage() ?? 'Job permanently failed',
'failed_at' => now()->toIso8601String(),
]);
$this->activity->save();
}
}
app/Jobs/DatabaseBackupJob.php
+37 -1
View File
@@ -23,6 +23,7 @@ use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\SerializesModels;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Str;
use Throwable;
use Visus\Cuid2\Cuid2;
@@ -31,6 +32,16 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
{
use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
/**
* The number of times the job may be attempted.
*/
public $tries = 2;
/**
* The maximum number of unhandled exceptions to allow before failing.
*/
public $maxExceptions = 1;
public ?Team $team = null;
public Server $server;
@@ -659,17 +670,42 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
return "{$helperImage}:{$latestVersion}";
}
/**
* Calculate the number of seconds to wait before retrying the job.
*/
public function backoff(): array
{
return [60, 300]; // 1min, 5min between retries
}
public function failed(?Throwable $exception): void
{
Log::channel('scheduled-errors')->error('DatabaseBackup permanently failed', [
'job' => 'DatabaseBackupJob',
'backup_id' => $this->backup->uuid,
'database' => $this->database?->name ?? 'unknown',
'database_type' => get_class($this->database ?? new \stdClass),
'server' => $this->server?->name ?? 'unknown',
'total_attempts' => $this->attempts(),
'error' => $exception?->getMessage(),
'trace' => $exception?->getTraceAsString(),
]);
$log = ScheduledDatabaseBackupExecution::where('uuid', $this->backup_log_uuid)->first();
if ($log) {
$log->update([
'status' => 'failed',
'message' => 'Job failed: '.($exception?->getMessage() ?? 'Unknown error'),
'message' => 'Job permanently failed after '.$this->attempts().' attempts: '.($exception?->getMessage() ?? 'Unknown error'),
'size' => 0,
'filename' => null,
'finished_at' => Carbon::now(),
]);
}
// Notify team about permanent failure
if ($this->team) {
$this->team->notify(new BackupFailed($this->backup, $this->database, $this->backup_output));
}
}
}
app/Jobs/ScheduledJobManager.php
+1 -1
View File
@@ -52,7 +52,7 @@ class ScheduledJobManager implements ShouldQueue
{
return [
(new WithoutOverlapping('scheduled-job-manager'))
->expireAfter(60) // Lock expires after 1 minute to prevent stale locks
->expireAfter(90) // Lock expires after 90s to handle high-load environments with many tasks
->dontRelease(), // Don't re-queue on lock conflict
];
}
app/Jobs/ScheduledTaskJob.php
+80 -2
View File
@@ -18,11 +18,27 @@ use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\SerializesModels;
use Illuminate\Support\Facades\Log;
class ScheduledTaskJob implements ShouldQueue
{
use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
/**
* The number of times the job may be attempted.
*/
public $tries = 3;
/**
* The maximum number of unhandled exceptions to allow before failing.
*/
public $maxExceptions = 1;
/**
* The number of seconds the job can run before timing out.
*/
public $timeout = 300;
public Team $team;
public Server $server;
@@ -55,6 +71,9 @@ class ScheduledTaskJob implements ShouldQueue
}
$this->team = Team::findOrFail($task->team_id);
$this->server_timezone = $this->getServerTimezone();
// Set timeout from task configuration
$this->timeout = $this->task->timeout ?? 300;
}
private function getServerTimezone(): string
@@ -70,9 +89,13 @@ class ScheduledTaskJob implements ShouldQueue
public function handle(): void
{
$startTime = Carbon::now();
try {
$this->task_log = ScheduledTaskExecution::create([
'scheduled_task_id' => $this->task->id,
'started_at' => $startTime,
'retry_count' => $this->attempts() - 1,
]);
$this->server = $this->resource->destination->server;
@@ -129,15 +152,70 @@ class ScheduledTaskJob implements ShouldQueue
'message' => $this->task_output ?? $e->getMessage(),
]);
}
$this->team?->notify(new TaskFailed($this->task, $e->getMessage()));
// Log the error to the scheduled-errors channel
Log::channel('scheduled-errors')->error('ScheduledTask execution failed', [
'job' => 'ScheduledTaskJob',
'task_id' => $this->task->uuid,
'task_name' => $this->task->name,
'server' => $this->server->name ?? 'unknown',
'attempt' => $this->attempts(),
'error' => $e->getMessage(),
]);
// Only notify and throw on final failure
if ($this->attempts() >= $this->tries) {
$this->team?->notify(new TaskFailed($this->task, $e->getMessage()));
}
// Re-throw to trigger Laravel's retry mechanism with backoff
throw $e;
} finally {
ScheduledTaskDone::dispatch($this->team->id);
if ($this->task_log) {
$finishedAt = Carbon::now();
$duration = round($startTime->floatDiffInSeconds($finishedAt), 2);
$this->task_log->update([
'finished_at' => Carbon::now()->toImmutable(),
'finished_at' => $finishedAt->toImmutable(),
'duration' => $duration,
]);
}
}
}
/**
* Calculate the number of seconds to wait before retrying the job.
*/
public function backoff(): array
{
return [30, 60, 120]; // 30s, 60s, 120s between retries
}
/**
* Handle a job failure.
*/
public function failed(?\Throwable $exception): void
{
Log::channel('scheduled-errors')->error('ScheduledTask permanently failed', [
'job' => 'ScheduledTaskJob',
'task_id' => $this->task->uuid,
'task_name' => $this->task->name,
'server' => $this->server->name ?? 'unknown',
'total_attempts' => $this->attempts(),
'error' => $exception?->getMessage(),
'trace' => $exception?->getTraceAsString(),
]);
if ($this->task_log) {
$this->task_log->update([
'status' => 'failed',
'message' => 'Job permanently failed after '.$this->attempts().' attempts: '.($exception?->getMessage() ?? 'Unknown error'),
'finished_at' => Carbon::now()->toImmutable(),
]);
}
// Notify team about permanent failure
$this->team?->notify(new TaskFailed($this->task, $exception?->getMessage() ?? 'Unknown error'));
}
}
app/Livewire/Project/Shared/ScheduledTask/Add.php
+6
View File
@@ -34,11 +34,14 @@ class Add extends Component
public ?string $container = '';
public int $timeout = 300;
protected $rules = [
'name' => 'required|string',
'command' => 'required|string',
'frequency' => 'required|string',
'container' => 'nullable|string',
'timeout' => 'required|integer|min:60|max:3600',
];
protected $validationAttributes = [
@@ -46,6 +49,7 @@ class Add extends Component
'command' => 'command',
'frequency' => 'frequency',
'container' => 'container',
'timeout' => 'timeout',
];
public function mount()
@@ -103,6 +107,7 @@ class Add extends Component
$task->command = $this->command;
$task->frequency = $this->frequency;
$task->container = $this->container;
$task->timeout = $this->timeout;
$task->team_id = currentTeam()->id;
switch ($this->type) {
@@ -130,5 +135,6 @@ class Add extends Component
$this->command = '';
$this->frequency = '';
$this->container = '';
$this->timeout = 300;
}
}
app/Livewire/Project/Shared/ScheduledTask/Show.php
+5
View File
@@ -40,6 +40,9 @@ class Show extends Component
#[Validate(['string', 'nullable'])]
public ?string $container = null;
#[Validate(['integer', 'required', 'min:60', 'max:3600'])]
public int $timeout = 300;
#[Locked]
public ?string $application_uuid;
@@ -99,6 +102,7 @@ class Show extends Component
$this->task->command = str($this->command)->trim()->value();
$this->task->frequency = str($this->frequency)->trim()->value();
$this->task->container = str($this->container)->trim()->value();
$this->task->timeout = $this->timeout;
$this->task->save();
} else {
$this->isEnabled = $this->task->enabled;
@@ -106,6 +110,7 @@ class Show extends Component
$this->command = $this->task->command;
$this->frequency = $this->task->frequency;
$this->container = $this->task->container;
$this->timeout = $this->task->timeout ?? 300;
}
}
app/Models/ScheduledTask.php
+8
View File
@@ -12,6 +12,14 @@ class ScheduledTask extends BaseModel
protected $guarded = [];
protected function casts(): array
{
return [
'enabled' => 'boolean',
'timeout' => 'integer',
];
}
public function service()
{
return $this->belongsTo(Service::class);
app/Models/ScheduledTaskExecution.php
+10
View File
@@ -8,6 +8,16 @@ class ScheduledTaskExecution extends BaseModel
{
protected $guarded = [];
protected function casts(): array
{
return [
'started_at' => 'datetime',
'finished_at' => 'datetime',
'retry_count' => 'integer',
'duration' => 'decimal:2',
];
}
public function scheduledTask(): BelongsTo
{
return $this->belongsTo(ScheduledTask::class);