mirror of
https://github.com/tiennm99/coolify.git
synced 2026-04-17 21:20:29 +00:00
Fix stale lock issue causing scheduled tasks to stop (#4539)
## Problem Scheduled tasks, backups, and auto-updates stopped working after 1-2 months with error: MaxAttemptsExceededException: App\Jobs\ScheduledJobManager has been attempted too many times. Root cause: ScheduledJobManager used WithoutOverlapping with only releaseAfter(60), causing locks without expiration (TTL=-1) that persisted indefinitely when jobs hung or processes crashed. ## Solution ### Part 1: Prevention (Future Locks) - Added expireAfter(60) to ScheduledJobManager middleware - Lock now auto-expires after 60 seconds (matches everyMinute schedule) - Changed from releaseAfter(60) to expireAfter(60)->dontRelease() - Follows Laravel best practices and matches other Coolify jobs ### Part 2: Recovery (Existing Locks) - Enhanced cleanup:redis command with --clear-locks flag - Scans Redis for stale locks (TTL=-1) and removes them - Called automatically during app:init on startup/upgrade - Provides immediate recovery for affected instances ## Changes - app/Jobs/ScheduledJobManager.php: Added expireAfter(60)->dontRelease() - app/Console/Commands/CleanupRedis.php: Added cleanupCacheLocks() method - app/Console/Commands/Init.php: Auto-clear locks on startup - tests/Unit/ScheduledJobManagerLockTest.php: Test to prevent regression - STALE_LOCK_FIX.md: Complete documentation ## Testing - Unit tests pass (2 tests, 8 assertions) - Code formatted with Pint - Matches pattern used by CleanupInstanceStuffsJob 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
60
tests/Unit/ScheduledJobManagerLockTest.php
Normal file
60
tests/Unit/ScheduledJobManagerLockTest.php
Normal file
@@ -0,0 +1,60 @@
|
||||
<?php
|
||||
|
||||
use App\Jobs\ScheduledJobManager;
|
||||
use Illuminate\Queue\Middleware\WithoutOverlapping;
|
||||
|
||||
it('uses WithoutOverlapping middleware with expireAfter to prevent stale locks', function () {
|
||||
$job = new ScheduledJobManager;
|
||||
$middleware = $job->middleware();
|
||||
|
||||
// Assert middleware exists
|
||||
expect($middleware)->toBeArray()
|
||||
->and($middleware)->toHaveCount(1);
|
||||
|
||||
$overlappingMiddleware = $middleware[0];
|
||||
|
||||
// Assert it's a WithoutOverlapping instance
|
||||
expect($overlappingMiddleware)->toBeInstanceOf(WithoutOverlapping::class);
|
||||
|
||||
// Use reflection to check private properties
|
||||
$reflection = new ReflectionClass($overlappingMiddleware);
|
||||
|
||||
// Check expireAfter is set (should be 60 seconds - matches job frequency)
|
||||
$expiresAfterProperty = $reflection->getProperty('expiresAfter');
|
||||
$expiresAfterProperty->setAccessible(true);
|
||||
$expiresAfter = $expiresAfterProperty->getValue($overlappingMiddleware);
|
||||
|
||||
expect($expiresAfter)->toBe(60)
|
||||
->and($expiresAfter)->toBeGreaterThan(0, 'expireAfter must be set to prevent stale locks');
|
||||
|
||||
// Check releaseAfter is NOT set (we use dontRelease)
|
||||
$releaseAfterProperty = $reflection->getProperty('releaseAfter');
|
||||
$releaseAfterProperty->setAccessible(true);
|
||||
$releaseAfter = $releaseAfterProperty->getValue($overlappingMiddleware);
|
||||
|
||||
expect($releaseAfter)->toBeNull('releaseAfter should be null when using dontRelease()');
|
||||
|
||||
// Check the lock key
|
||||
$keyProperty = $reflection->getProperty('key');
|
||||
$keyProperty->setAccessible(true);
|
||||
$key = $keyProperty->getValue($overlappingMiddleware);
|
||||
|
||||
expect($key)->toBe('scheduled-job-manager');
|
||||
});
|
||||
|
||||
it('prevents stale locks by ensuring expireAfter is always set', function () {
|
||||
$job = new ScheduledJobManager;
|
||||
$middleware = $job->middleware();
|
||||
|
||||
$overlappingMiddleware = $middleware[0];
|
||||
$reflection = new ReflectionClass($overlappingMiddleware);
|
||||
|
||||
$expiresAfterProperty = $reflection->getProperty('expiresAfter');
|
||||
$expiresAfterProperty->setAccessible(true);
|
||||
$expiresAfter = $expiresAfterProperty->getValue($overlappingMiddleware);
|
||||
|
||||
// Critical check: expireAfter MUST be set to prevent GitHub issue #4539
|
||||
expect($expiresAfter)->not->toBeNull(
|
||||
'expireAfter() is required to prevent stale locks (see GitHub #4539)'
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user