mirror of
https://github.com/linuxserver/Heimdall.git
synced 2026-08-07 07:16:13 +00:00
Bound retry and unique-lock lifetimes on UpdateApps and ProcessApps
Both jobs implement ShouldBeUnique without a $uniqueFor value, which on
Redis and database drivers produces a lock that never expires. If the
worker is killed mid-fire (OOM, SIGKILL, server crash), the lock
persists and blocks all future dispatches of UpdateApps or ProcessApps
until the cache entry is manually cleared.
Neither job sets $tries, $backoff, or $timeout, so they inherit the
worker command's defaults (1 for queue:work, 0 for vapor:work), which
varies by platform and is brittle.
This change adds:
- $uniqueFor = 3600 lock expires after 1 hour
- $tries = 3 hard cap across worker restarts
- $backoff = [30, 60, 120] pace retries to reduce GitHub API load
- $timeout = 60 bound per-attempt wall-clock time
- failed(Throwable) log permanent failures (and preserve the
existing Cache::lock('updateApps')->forceRelease
on UpdateApps)
A new test (tests/Feature/QueueSafetyTest.php) asserts the retry
properties are present.
All existing tests still pass.
This commit is contained in:
@@ -14,11 +14,29 @@ use Illuminate\Queue\InteractsWithQueue;
|
||||
use Illuminate\Queue\SerializesModels;
|
||||
use Illuminate\Support\Facades\Log;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
use Throwable;
|
||||
|
||||
class ProcessApps implements ShouldQueue, ShouldBeUnique
|
||||
{
|
||||
use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
|
||||
|
||||
/**
|
||||
* Bound total attempts so a failed job stops after three retries across
|
||||
* worker restarts, independent of the broker's reserved-job recovery path.
|
||||
*/
|
||||
public int $tries = 3;
|
||||
|
||||
/** @var array<int, int> seconds between retries */
|
||||
public array $backoff = [30, 60, 120];
|
||||
|
||||
public int $timeout = 60;
|
||||
|
||||
/**
|
||||
* Expire the ShouldBeUnique lock after 1 hour so a crashed worker does
|
||||
* not permanently block future ProcessApps dispatches.
|
||||
*/
|
||||
public int $uniqueFor = 3600;
|
||||
|
||||
/**
|
||||
* Create a new job instance.
|
||||
*
|
||||
@@ -57,4 +75,11 @@ class ProcessApps implements ShouldQueue, ShouldBeUnique
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public function failed(Throwable $exception): void
|
||||
{
|
||||
Log::error(static::class . ' permanently failed', [
|
||||
'exception' => $exception->getMessage(),
|
||||
]);
|
||||
}
|
||||
}
|
||||
|
||||
+27
-1
@@ -12,11 +12,33 @@ use Illuminate\Queue\InteractsWithQueue;
|
||||
use Illuminate\Queue\SerializesModels;
|
||||
use Illuminate\Support\Facades\Cache;
|
||||
use Illuminate\Support\Facades\Log;
|
||||
use Throwable;
|
||||
|
||||
class UpdateApps implements ShouldQueue, ShouldBeUnique
|
||||
{
|
||||
use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
|
||||
|
||||
/**
|
||||
* Bound total attempts so a failed job stops after three retries across
|
||||
* worker restarts, independent of the broker's reserved-job recovery path.
|
||||
*/
|
||||
public int $tries = 3;
|
||||
|
||||
/** @var array<int, int> seconds between retries */
|
||||
public array $backoff = [30, 60, 120];
|
||||
|
||||
/**
|
||||
* Per-attempt wall-clock ceiling. Heavy users with many apps may need
|
||||
* a larger value; 60s covers the typical Heimdall deployment.
|
||||
*/
|
||||
public int $timeout = 60;
|
||||
|
||||
/**
|
||||
* Expire the ShouldBeUnique lock after 1 hour so a crashed worker does
|
||||
* not permanently block future UpdateApps dispatches.
|
||||
*/
|
||||
public int $uniqueFor = 3600;
|
||||
|
||||
/**
|
||||
* Create a new job instance.
|
||||
*
|
||||
@@ -49,8 +71,12 @@ class UpdateApps implements ShouldQueue, ShouldBeUnique
|
||||
Cache::lock('updateApps')->forceRelease();
|
||||
}
|
||||
|
||||
public function failed($exception): void
|
||||
public function failed(Throwable $exception): void
|
||||
{
|
||||
Cache::lock('updateApps')->forceRelease();
|
||||
|
||||
Log::error(static::class . ' permanently failed', [
|
||||
'exception' => $exception->getMessage(),
|
||||
]);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
<?php
|
||||
|
||||
namespace Tests\Feature;
|
||||
|
||||
use App\Jobs\ProcessApps;
|
||||
use App\Jobs\UpdateApps;
|
||||
use Tests\TestCase;
|
||||
|
||||
class QueueSafetyTest extends TestCase
|
||||
{
|
||||
/** @test */
|
||||
public function updateApps_has_bounded_retry_properties(): void
|
||||
{
|
||||
$job = new UpdateApps();
|
||||
|
||||
$this->assertSame(3, $job->tries);
|
||||
$this->assertSame([30, 60, 120], $job->backoff);
|
||||
$this->assertSame(60, $job->timeout);
|
||||
$this->assertSame(3600, $job->uniqueFor);
|
||||
}
|
||||
|
||||
/** @test */
|
||||
public function processApps_has_bounded_retry_properties(): void
|
||||
{
|
||||
$job = new ProcessApps();
|
||||
|
||||
$this->assertSame(3, $job->tries);
|
||||
$this->assertSame([30, 60, 120], $job->backoff);
|
||||
$this->assertSame(60, $job->timeout);
|
||||
$this->assertSame(3600, $job->uniqueFor);
|
||||
}
|
||||
|
||||
/** @test */
|
||||
public function both_jobs_expose_a_failed_method(): void
|
||||
{
|
||||
$this->assertTrue(method_exists(UpdateApps::class, 'failed'));
|
||||
$this->assertTrue(method_exists(ProcessApps::class, 'failed'));
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user