mirror of
https://github.com/tiennm99/coolify.git
synced 2026-10-14 20:17:05 +00:00
refactor(scheduler): extract cron scheduling logic to shared helper
Extract the shouldRunNow() method from ScheduledJobManager and ServerManagerJob into a reusable shouldRunCronNow() helper function. This centralizes cron scheduling logic and enables consistent deduplication behavior across all scheduled job types. - Create shouldRunCronNow() helper in bootstrap/helpers/shared.php with timezone and dedup support - Refactor ScheduledJobManager and ServerManagerJob to use the shared helper - Add ScheduledJobDiagnostics command for inspecting cache state and scheduling decisions across all scheduled jobs - Simplify shouldRunNow tests to directly test the helper function - Add DockerCleanupJob test for error handling and execution tracking - Increase scheduled log retention from 1 to 7 days
This commit is contained in:
1 parent
89f2b83104
commit
f8f27fff13
8 files changed
+446
-317
No files matched your search
@@ -6,7 +6,6 @@ use App\Models\ScheduledDatabaseBackup;
|
||||
use App\Models\ScheduledTask;
|
||||
use App\Models\Server;
|
||||
use App\Models\Team;
|
||||
use Cron\CronExpression;
|
||||
use Illuminate\Bus\Queueable;
|
||||
use Illuminate\Contracts\Queue\ShouldQueue;
|
||||
use Illuminate\Foundation\Bus\Dispatchable;
|
||||
@@ -185,7 +184,7 @@ class ScheduledJobManager implements ShouldQueue
|
||||
$frequency = VALID_CRON_STRINGS[$frequency];
|
||||
}
|
||||
|
||||
if ($this->shouldRunNow($frequency, $serverTimezone, "scheduled-backup:{$backup->id}")) {
|
||||
if (shouldRunCronNow($frequency, $serverTimezone, "scheduled-backup:{$backup->id}", $this->executionTime)) {
|
||||
DatabaseBackupJob::dispatch($backup);
|
||||
$this->dispatchedCount++;
|
||||
Log::channel('scheduled')->info('Backup dispatched', [
|
||||
@@ -239,7 +238,7 @@ class ScheduledJobManager implements ShouldQueue
|
||||
$frequency = VALID_CRON_STRINGS[$frequency];
|
||||
}
|
||||
|
||||
if (! $this->shouldRunNow($frequency, $serverTimezone, "scheduled-task:{$task->id}")) {
|
||||
if (! shouldRunCronNow($frequency, $serverTimezone, "scheduled-task:{$task->id}", $this->executionTime)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -336,51 +335,6 @@ class ScheduledJobManager implements ShouldQueue
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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. Without dedupKey, falls back
|
||||
* to simple isDue() check.
|
||||
*/
|
||||
private function shouldRunNow(string $frequency, string $timezone, ?string $dedupKey = null): bool
|
||||
{
|
||||
$cron = new CronExpression($frequency);
|
||||
$baseTime = $this->executionTime ?? Carbon::now();
|
||||
$executionTime = $baseTime->copy()->setTimezone($timezone);
|
||||
|
||||
// No dedup key → simple isDue check
|
||||
if ($dedupKey === null) {
|
||||
return $cron->isDue($executionTime);
|
||||
}
|
||||
|
||||
// Get the most recent time this cron was due (including current minute)
|
||||
$previousDue = Carbon::instance($cron->getPreviousRunDate($executionTime, allowCurrentDate: true));
|
||||
|
||||
$lastDispatched = Cache::get($dedupKey);
|
||||
|
||||
if ($lastDispatched === null) {
|
||||
// First run after restart or cache loss: only fire if actually due right now.
|
||||
// Seed the cache so subsequent runs can use tolerance/catch-up logic.
|
||||
$isDue = $cron->isDue($executionTime);
|
||||
if ($isDue) {
|
||||
Cache::put($dedupKey, $executionTime->toIso8601String(), 86400);
|
||||
}
|
||||
|
||||
return $isDue;
|
||||
}
|
||||
|
||||
// Subsequent runs: fire if there's been a due time since last dispatch
|
||||
if ($previousDue->gt(Carbon::parse($lastDispatched))) {
|
||||
Cache::put($dedupKey, $executionTime->toIso8601String(), 86400);
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
private function processDockerCleanups(): void
|
||||
{
|
||||
// Get all servers that need cleanup checks
|
||||
@@ -411,7 +365,7 @@ class ScheduledJobManager implements ShouldQueue
|
||||
}
|
||||
|
||||
// Use the frozen execution time for consistent evaluation
|
||||
if ($this->shouldRunNow($frequency, $serverTimezone, "docker-cleanup:{$server->id}")) {
|
||||
if (shouldRunCronNow($frequency, $serverTimezone, "docker-cleanup:{$server->id}", $this->executionTime)) {
|
||||
DockerCleanupJob::dispatch(
|
||||
$server,
|
||||
false,
|
||||
|
||||
Reference in new issue
Block a user