mirror of
https://github.com/coollabsio/coolify.git
synced 2026-08-24 10:05:47 -05:00
feat(jobs): add cache-based deduplication for delayed cron execution
Implements getPreviousRunDate() + cache-based tracking in shouldRunNow() to prevent duplicate dispatch of scheduled jobs when queue delays push execution past the cron minute. This resilience ensures jobs catch missed windows without double-dispatching within the same cron window. Updated scheduled job dispatches to include dedupKey parameter: - Docker cleanup operations - Server connection checks - Sentinel restart checks - Server storage checks - Server patch checks DockerCleanupJob now dispatches on the 'high' queue for faster processing. Includes comprehensive test coverage for dedup behavior across different cron schedules and delay scenarios.
This commit is contained in:
@@ -39,7 +39,9 @@ class DockerCleanupJob implements ShouldBeEncrypted, ShouldQueue
|
||||
public bool $manualCleanup = false,
|
||||
public bool $deleteUnusedVolumes = false,
|
||||
public bool $deleteUnusedNetworks = false
|
||||
) {}
|
||||
) {
|
||||
$this->onQueue('high');
|
||||
}
|
||||
|
||||
public function handle(): void
|
||||
{
|
||||
|
||||
@@ -350,7 +350,7 @@ class ScheduledJobManager implements ShouldQueue
|
||||
$baseTime = $this->executionTime ?? Carbon::now();
|
||||
$executionTime = $baseTime->copy()->setTimezone($timezone);
|
||||
|
||||
// No dedup key → simple isDue check (used by docker cleanups)
|
||||
// No dedup key → simple isDue check
|
||||
if ($dedupKey === null) {
|
||||
return $cron->isDue($executionTime);
|
||||
}
|
||||
@@ -411,7 +411,7 @@ class ScheduledJobManager implements ShouldQueue
|
||||
}
|
||||
|
||||
// Use the frozen execution time for consistent evaluation
|
||||
if ($this->shouldRunNow($frequency, $serverTimezone)) {
|
||||
if ($this->shouldRunNow($frequency, $serverTimezone, "docker-cleanup:{$server->id}")) {
|
||||
DockerCleanupJob::dispatch(
|
||||
$server,
|
||||
false,
|
||||
|
||||
@@ -14,6 +14,7 @@ use Illuminate\Queue\InteractsWithQueue;
|
||||
use Illuminate\Queue\SerializesModels;
|
||||
use Illuminate\Support\Carbon;
|
||||
use Illuminate\Support\Collection;
|
||||
use Illuminate\Support\Facades\Cache;
|
||||
use Illuminate\Support\Facades\Log;
|
||||
|
||||
class ServerManagerJob implements ShouldBeEncrypted, ShouldQueue
|
||||
@@ -80,7 +81,7 @@ class ServerManagerJob implements ShouldBeEncrypted, ShouldQueue
|
||||
private function dispatchConnectionChecks(Collection $servers): void
|
||||
{
|
||||
|
||||
if ($this->shouldRunNow($this->checkFrequency)) {
|
||||
if ($this->shouldRunNow($this->checkFrequency, dedupKey: 'server-connection-checks')) {
|
||||
$servers->each(function (Server $server) {
|
||||
try {
|
||||
// Skip SSH connection check if Sentinel is healthy — its heartbeat already proves connectivity
|
||||
@@ -129,13 +130,13 @@ class ServerManagerJob implements ShouldBeEncrypted, ShouldQueue
|
||||
|
||||
if ($sentinelOutOfSync) {
|
||||
// Dispatch ServerCheckJob if Sentinel is out of sync
|
||||
if ($this->shouldRunNow($this->checkFrequency, $serverTimezone)) {
|
||||
if ($this->shouldRunNow($this->checkFrequency, $serverTimezone, "server-check:{$server->id}")) {
|
||||
ServerCheckJob::dispatch($server);
|
||||
}
|
||||
}
|
||||
|
||||
$isSentinelEnabled = $server->isSentinelEnabled();
|
||||
$shouldRestartSentinel = $isSentinelEnabled && $this->shouldRunNow('0 0 * * *', $serverTimezone);
|
||||
$shouldRestartSentinel = $isSentinelEnabled && $this->shouldRunNow('0 0 * * *', $serverTimezone, "sentinel-restart:{$server->id}");
|
||||
// Dispatch Sentinel restart if due (daily for Sentinel-enabled servers)
|
||||
|
||||
if ($shouldRestartSentinel) {
|
||||
@@ -149,7 +150,7 @@ class ServerManagerJob implements ShouldBeEncrypted, ShouldQueue
|
||||
if (isset(VALID_CRON_STRINGS[$serverDiskUsageCheckFrequency])) {
|
||||
$serverDiskUsageCheckFrequency = VALID_CRON_STRINGS[$serverDiskUsageCheckFrequency];
|
||||
}
|
||||
$shouldRunStorageCheck = $this->shouldRunNow($serverDiskUsageCheckFrequency, $serverTimezone);
|
||||
$shouldRunStorageCheck = $this->shouldRunNow($serverDiskUsageCheckFrequency, $serverTimezone, "server-storage-check:{$server->id}");
|
||||
|
||||
if ($shouldRunStorageCheck) {
|
||||
ServerStorageCheckJob::dispatch($server);
|
||||
@@ -157,7 +158,7 @@ class ServerManagerJob implements ShouldBeEncrypted, ShouldQueue
|
||||
}
|
||||
|
||||
// Dispatch ServerPatchCheckJob if due (weekly)
|
||||
$shouldRunPatchCheck = $this->shouldRunNow('0 0 * * 0', $serverTimezone);
|
||||
$shouldRunPatchCheck = $this->shouldRunNow('0 0 * * 0', $serverTimezone, "server-patch-check:{$server->id}");
|
||||
|
||||
if ($shouldRunPatchCheck) { // Weekly on Sunday at midnight
|
||||
ServerPatchCheckJob::dispatch($server);
|
||||
@@ -167,7 +168,14 @@ class ServerManagerJob implements ShouldBeEncrypted, ShouldQueue
|
||||
// Crash recovery is handled by sentinelOutOfSync → ServerCheckJob → CheckAndStartSentinelJob.
|
||||
}
|
||||
|
||||
private function shouldRunNow(string $frequency, ?string $timezone = null): bool
|
||||
/**
|
||||
* Determine if a cron schedule should run now.
|
||||
*
|
||||
* When a dedupKey is provided, uses getPreviousRunDate() + last-dispatch tracking
|
||||
* instead of isDue(). This is resilient to queue delays — even if the job is delayed
|
||||
* by minutes, it still catches the missed cron window.
|
||||
*/
|
||||
private function shouldRunNow(string $frequency, ?string $timezone = null, ?string $dedupKey = null): bool
|
||||
{
|
||||
$cron = new CronExpression($frequency);
|
||||
|
||||
@@ -175,6 +183,29 @@ class ServerManagerJob implements ShouldBeEncrypted, ShouldQueue
|
||||
$baseTime = $this->executionTime ?? Carbon::now();
|
||||
$executionTime = $baseTime->copy()->setTimezone($timezone ?? config('app.timezone'));
|
||||
|
||||
return $cron->isDue($executionTime);
|
||||
if ($dedupKey === null) {
|
||||
return $cron->isDue($executionTime);
|
||||
}
|
||||
|
||||
$previousDue = Carbon::instance($cron->getPreviousRunDate($executionTime, allowCurrentDate: true));
|
||||
|
||||
$lastDispatched = Cache::get($dedupKey);
|
||||
|
||||
if ($lastDispatched === null) {
|
||||
$isDue = $cron->isDue($executionTime);
|
||||
if ($isDue) {
|
||||
Cache::put($dedupKey, $executionTime->toIso8601String(), 86400);
|
||||
}
|
||||
|
||||
return $isDue;
|
||||
}
|
||||
|
||||
if ($previousDue->gt(Carbon::parse($lastDispatched))) {
|
||||
Cache::put($dedupKey, $executionTime->toIso8601String(), 86400);
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -194,6 +194,55 @@ it('falls back to isDue when no dedup key is provided', function () {
|
||||
expect($result2)->toBeFalse();
|
||||
});
|
||||
|
||||
it('catches delayed docker cleanup when job runs past the cron minute', function () {
|
||||
// Simulate a previous dispatch at :10
|
||||
Cache::put('docker-cleanup:42', Carbon::create(2026, 2, 28, 10, 10, 0, 'UTC')->toIso8601String(), 86400);
|
||||
|
||||
// Freeze time at :22 — job was delayed 2 minutes past the :20 cron window
|
||||
Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 22, 0, 'UTC'));
|
||||
|
||||
$job = new ScheduledJobManager;
|
||||
$reflection = new ReflectionClass($job);
|
||||
|
||||
$executionTimeProp = $reflection->getProperty('executionTime');
|
||||
$executionTimeProp->setAccessible(true);
|
||||
$executionTimeProp->setValue($job, Carbon::now());
|
||||
|
||||
$method = $reflection->getMethod('shouldRunNow');
|
||||
$method->setAccessible(true);
|
||||
|
||||
// isDue() would return false at :22, but getPreviousRunDate() = :20
|
||||
// lastDispatched = :10 → :20 > :10 → fires
|
||||
$result = $method->invoke($job, '*/10 * * * *', 'UTC', 'docker-cleanup:42');
|
||||
|
||||
expect($result)->toBeTrue();
|
||||
});
|
||||
|
||||
it('does not double-dispatch docker cleanup within same cron window', function () {
|
||||
// First dispatch at :10
|
||||
Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 10, 0, 'UTC'));
|
||||
|
||||
$job = new ScheduledJobManager;
|
||||
$reflection = new ReflectionClass($job);
|
||||
|
||||
$executionTimeProp = $reflection->getProperty('executionTime');
|
||||
$executionTimeProp->setAccessible(true);
|
||||
$executionTimeProp->setValue($job, Carbon::now());
|
||||
|
||||
$method = $reflection->getMethod('shouldRunNow');
|
||||
$method->setAccessible(true);
|
||||
|
||||
$first = $method->invoke($job, '*/10 * * * *', 'UTC', 'docker-cleanup:99');
|
||||
expect($first)->toBeTrue();
|
||||
|
||||
// Second run at :11 — should NOT dispatch (previousDue=:10, lastDispatched=:10)
|
||||
Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 11, 0, 'UTC'));
|
||||
$executionTimeProp->setValue($job, Carbon::now());
|
||||
|
||||
$second = $method->invoke($job, '*/10 * * * *', 'UTC', 'docker-cleanup:99');
|
||||
expect($second)->toBeFalse();
|
||||
});
|
||||
|
||||
it('respects server timezone for cron evaluation', function () {
|
||||
// UTC time is 22:00 Feb 28, which is 06:00 Mar 1 in Asia/Singapore (+8)
|
||||
Carbon::setTestNow(Carbon::create(2026, 2, 28, 22, 0, 0, 'UTC'));
|
||||
|
||||
@@ -0,0 +1,102 @@
|
||||
<?php
|
||||
|
||||
use App\Jobs\ServerManagerJob;
|
||||
use Illuminate\Support\Carbon;
|
||||
use Illuminate\Support\Facades\Cache;
|
||||
|
||||
beforeEach(function () {
|
||||
Cache::flush();
|
||||
});
|
||||
|
||||
it('catches delayed sentinel restart when job runs past midnight', function () {
|
||||
// Simulate previous dispatch yesterday at midnight
|
||||
Cache::put('sentinel-restart:1', Carbon::create(2026, 2, 27, 0, 0, 0, 'UTC')->toIso8601String(), 86400);
|
||||
|
||||
// Job runs 3 minutes late at 00:03
|
||||
Carbon::setTestNow(Carbon::create(2026, 2, 28, 0, 3, 0, 'UTC'));
|
||||
|
||||
$job = new ServerManagerJob;
|
||||
$reflection = new ReflectionClass($job);
|
||||
|
||||
$executionTimeProp = $reflection->getProperty('executionTime');
|
||||
$executionTimeProp->setAccessible(true);
|
||||
$executionTimeProp->setValue($job, Carbon::now());
|
||||
|
||||
$method = $reflection->getMethod('shouldRunNow');
|
||||
$method->setAccessible(true);
|
||||
|
||||
// isDue() would return false at 00:03, but getPreviousRunDate() = 00:00 today
|
||||
// lastDispatched = yesterday 00:00 → today 00:00 > yesterday → fires
|
||||
$result = $method->invoke($job, '0 0 * * *', 'UTC', 'sentinel-restart:1');
|
||||
|
||||
expect($result)->toBeTrue();
|
||||
});
|
||||
|
||||
it('catches delayed weekly patch check when job runs past the cron minute', function () {
|
||||
// Simulate previous dispatch last Sunday at midnight
|
||||
Cache::put('server-patch-check:1', Carbon::create(2026, 2, 22, 0, 0, 0, 'UTC')->toIso8601String(), 86400);
|
||||
|
||||
// This Sunday at 00:02 — job was delayed 2 minutes
|
||||
// 2026-03-01 is a Sunday
|
||||
Carbon::setTestNow(Carbon::create(2026, 3, 1, 0, 2, 0, 'UTC'));
|
||||
|
||||
$job = new ServerManagerJob;
|
||||
$reflection = new ReflectionClass($job);
|
||||
|
||||
$executionTimeProp = $reflection->getProperty('executionTime');
|
||||
$executionTimeProp->setAccessible(true);
|
||||
$executionTimeProp->setValue($job, Carbon::now());
|
||||
|
||||
$method = $reflection->getMethod('shouldRunNow');
|
||||
$method->setAccessible(true);
|
||||
|
||||
$result = $method->invoke($job, '0 0 * * 0', 'UTC', 'server-patch-check:1');
|
||||
|
||||
expect($result)->toBeTrue();
|
||||
});
|
||||
|
||||
it('catches delayed storage check when job runs past the cron minute', function () {
|
||||
// Simulate previous dispatch yesterday at 23:00
|
||||
Cache::put('server-storage-check:5', Carbon::create(2026, 2, 27, 23, 0, 0, 'UTC')->toIso8601String(), 86400);
|
||||
|
||||
// Today at 23:04 — job was delayed 4 minutes
|
||||
Carbon::setTestNow(Carbon::create(2026, 2, 28, 23, 4, 0, 'UTC'));
|
||||
|
||||
$job = new ServerManagerJob;
|
||||
$reflection = new ReflectionClass($job);
|
||||
|
||||
$executionTimeProp = $reflection->getProperty('executionTime');
|
||||
$executionTimeProp->setAccessible(true);
|
||||
$executionTimeProp->setValue($job, Carbon::now());
|
||||
|
||||
$method = $reflection->getMethod('shouldRunNow');
|
||||
$method->setAccessible(true);
|
||||
|
||||
$result = $method->invoke($job, '0 23 * * *', 'UTC', 'server-storage-check:5');
|
||||
|
||||
expect($result)->toBeTrue();
|
||||
});
|
||||
|
||||
it('does not double-dispatch within same cron window', function () {
|
||||
Carbon::setTestNow(Carbon::create(2026, 2, 28, 0, 0, 0, 'UTC'));
|
||||
|
||||
$job = new ServerManagerJob;
|
||||
$reflection = new ReflectionClass($job);
|
||||
|
||||
$executionTimeProp = $reflection->getProperty('executionTime');
|
||||
$executionTimeProp->setAccessible(true);
|
||||
$executionTimeProp->setValue($job, Carbon::now());
|
||||
|
||||
$method = $reflection->getMethod('shouldRunNow');
|
||||
$method->setAccessible(true);
|
||||
|
||||
$first = $method->invoke($job, '0 0 * * *', 'UTC', 'sentinel-restart:10');
|
||||
expect($first)->toBeTrue();
|
||||
|
||||
// Next minute — should NOT dispatch again
|
||||
Carbon::setTestNow(Carbon::create(2026, 2, 28, 0, 1, 0, 'UTC'));
|
||||
$executionTimeProp->setValue($job, Carbon::now());
|
||||
|
||||
$second = $method->invoke($job, '0 0 * * *', 'UTC', 'sentinel-restart:10');
|
||||
expect($second)->toBeFalse();
|
||||
});
|
||||
Reference in New Issue
Block a user