Skip to content

Commit fdae647

Browse files
committed
Persist terminal "aborted" state so dead jobs stop counting as queued
A job with `completed IS NULL` is treated as queued/in-progress by isQueued(), the admin "in_progress" filter, and the status badge. That conflates three different things: pending, actively running, and permanently failed. Once a job exhausts its retries it will never run again, yet it keeps `completed IS NULL` forever — so: * isQueued() keeps returning true, which wedges any non-concurrent caller (e.g. a QueueScheduler row) permanently behind the dead job; * the admin shows it as "Running" instead of failed. getFailedStatus() already distinguishes a retryable failure ("requeued") from a terminal one ("aborted"); this only persists that verdict so the rest of the system can see it. * Add `QueuedJobsTable::STATUS_ABORTED` and `markJobAborted()`. * Processor stamps it when getFailedStatus() === aborted (alongside the existing maxAttemptsExhausted event). * isQueued() and the "in_progress" search filter exclude aborted rows (null/other statuses are unaffected — fully backward compatible). * The status badge renders aborted as "Failed", checked before the `fetched` branch so a job whose worker died on its last attempt no longer shows a stuck "Running". Retryable failures are untouched: while attempts remain the job is genuinely in flight and still counts as queued, so there is no early re-dispatch or pile-up. flushFailedJobs() continues to prune old rows.
1 parent 25b2c08 commit fdae647

6 files changed

Lines changed: 109 additions & 5 deletions

File tree

src/Model/Filter/QueuedJobsCollection.php

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use Cake\Http\Exception\NotImplementedException;
77
use Cake\I18n\DateTime;
88
use Cake\ORM\Query\SelectQuery;
9+
use Queue\Model\Table\QueuedJobsTable;
910
use Search\Model\Filter\FilterCollection;
1011

1112
class QueuedJobsCollection extends FilterCollection {
@@ -32,9 +33,17 @@ public function initialize(): void {
3233
if ($status === 'in_progress') {
3334
$query->where([
3435
'completed IS' => null,
36+
// Exclude terminally-failed (aborted) jobs: they are
37+
// done, not in progress, even without a completed stamp.
3538
'OR' => [
36-
'notbefore <=' => new DateTime(),
37-
'notbefore IS' => null,
39+
'status IS' => null,
40+
'status !=' => QueuedJobsTable::STATUS_ABORTED,
41+
],
42+
'AND' => [
43+
'OR' => [
44+
'notbefore <=' => new DateTime(),
45+
'notbefore IS' => null,
46+
],
3847
],
3948
]);
4049

src/Model/Table/QueuedJobsTable.php

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,16 @@ class QueuedJobsTable extends Table {
7676
*/
7777
public const DAY = 86400;
7878

79+
/**
80+
* Terminal `status` value stamped on a job that has exhausted all of its
81+
* retries. Distinguishes a permanently-failed job (which will never run
82+
* again) from one that is pending, in progress, or still being retried —
83+
* all of which otherwise share `completed IS NULL`.
84+
*
85+
* @var string
86+
*/
87+
public const STATUS_ABORTED = 'aborted';
88+
7989
/**
8090
* @var array<string, string>
8191
*/
@@ -291,6 +301,15 @@ public function isQueued(string $reference, ?string $jobTask = null): bool {
291301
$conditions = [
292302
'reference' => $reference,
293303
'completed IS' => null,
304+
// A job that exhausted its retries (status = aborted) will never
305+
// run again, so it is not "queued" anymore even though it has no
306+
// completed timestamp. Excluding it stops callers that gate on
307+
// isQueued() — e.g. a non-concurrent scheduler row — from wedging
308+
// permanently behind a dead job. The OR keeps null/other statuses.
309+
'OR' => [
310+
'status IS' => null,
311+
'status !=' => static::STATUS_ABORTED,
312+
],
294313
];
295314
if ($jobTask) {
296315
$conditions['job_task'] = $jobTask;
@@ -769,6 +788,26 @@ public function markJobFailed(QueuedJob $job, ?string $failureMessage = null, ?s
769788
return (bool)$this->save($job);
770789
}
771790

791+
/**
792+
* Mark a job as terminally failed (aborted) after it has exhausted all
793+
* retries. Records the terminal `status` so the job is no longer reported
794+
* as queued/in-progress: `isQueued()` excludes it, the admin status filter
795+
* skips it, and the status badge renders it as failed rather than running.
796+
*
797+
* Call this only once the worker has determined no further attempts will
798+
* be made (see Processor, getFailedStatus() === self::STATUS_ABORTED).
799+
*
800+
* @param \Queue\Model\Entity\QueuedJob $job Job
801+
*
802+
* @return bool Success
803+
*/
804+
public function markJobAborted(QueuedJob $job): bool {
805+
return (bool)$this->updateAll(
806+
['status' => static::STATUS_ABORTED],
807+
['id' => $job->id],
808+
);
809+
}
810+
772811
/**
773812
* Removes all failed jobs.
774813
*
@@ -1202,7 +1241,7 @@ public function getFailedStatus(QueuedJob $queuedTask, array $taskConfiguration)
12021241
return $failureMessageRequeued;
12031242
}
12041243

1205-
return 'aborted';
1244+
return static::STATUS_ABORTED;
12061245
}
12071246

12081247
/**

src/Queue/Processor.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,13 @@ protected function runJob(QueuedJob $queuedJob, string $pid): void {
349349
EventManager::instance()->dispatch($event);
350350

351351
// Dispatch event when job has exhausted all retries
352-
if ($failedStatus === 'aborted') {
352+
if ($failedStatus === $this->QueuedJobs::STATUS_ABORTED) {
353+
// Persist the terminal state so the job stops counting as
354+
// queued/in-progress (it will never run again). Without this
355+
// it keeps `completed IS NULL` forever and wedges callers that
356+
// gate on isQueued(), e.g. a non-concurrent scheduler row.
357+
$this->QueuedJobs->markJobAborted($queuedJob);
358+
353359
$event = new Event('Queue.Job.maxAttemptsExhausted', $this, [
354360
'job' => $queuedJob,
355361
'failureMessage' => $failureMessage,

templates/element/Queue/status_badge.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,23 @@
11
<?php
2+
23
/**
34
* Job Status Badge Element
45
*
56
* @var \Cake\View\View $this
67
* @var \Queue\Model\Entity\QueuedJob $job The queued job entity
78
*/
9+
use Queue\Model\Table\QueuedJobsTable;
810

911
$status = 'pending';
1012
$icon = 'clock';
1113

1214
if ($job->completed) {
1315
$status = 'completed';
1416
$icon = 'check';
15-
} elseif ($job->failure_message) {
17+
} elseif ($job->status === QueuedJobsTable::STATUS_ABORTED || $job->failure_message) {
18+
// Terminal: retries exhausted (status = aborted) or a recorded failure.
19+
// Checked before `fetched` so a job whose worker died on its last attempt
20+
// shows as Failed, not stuck "Running".
1621
$status = 'failed';
1722
$icon = 'times';
1823
} elseif ($job->fetched) {

tests/TestCase/Model/Table/QueuedJobsTableTest.php

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -834,6 +834,46 @@ public function testIsQueued() {
834834
$this->assertFalse($result);
835835
}
836836

837+
/**
838+
* A job that has exhausted its retries (status = aborted) is terminal and
839+
* must no longer count as queued, even though it has no completed stamp.
840+
* Normal null-status rows still count.
841+
*
842+
* @return void
843+
*/
844+
public function testIsQueuedExcludesAborted() {
845+
$queuedJob = $this->QueuedJobs->newEntity([
846+
'key' => 'key',
847+
'job_task' => 'FooBar',
848+
'reference' => 'foo-bar',
849+
]);
850+
$this->QueuedJobs->saveOrFail($queuedJob);
851+
$this->assertTrue($this->QueuedJobs->isQueued('foo-bar'));
852+
853+
$this->QueuedJobs->markJobAborted($queuedJob);
854+
$this->assertFalse($this->QueuedJobs->isQueued('foo-bar'));
855+
}
856+
857+
/**
858+
* markJobAborted() stamps the terminal status without touching completed.
859+
*
860+
* @return void
861+
*/
862+
public function testMarkJobAborted() {
863+
$queuedJob = $this->QueuedJobs->newEntity([
864+
'key' => 'key',
865+
'job_task' => 'FooBar',
866+
'reference' => 'foo-bar',
867+
]);
868+
$this->QueuedJobs->saveOrFail($queuedJob);
869+
870+
$this->assertTrue($this->QueuedJobs->markJobAborted($queuedJob));
871+
872+
$reloaded = $this->QueuedJobs->get($queuedJob->id);
873+
$this->assertSame(QueuedJobsTable::STATUS_ABORTED, $reloaded->status);
874+
$this->assertNull($reloaded->completed);
875+
}
876+
837877
/**
838878
* @return void
839879
*/

tests/TestCase/Queue/ProcessorTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,11 @@ public function testMaxAttemptsExhaustedEvent() {
271271
// Verify event data
272272
// The event was fired successfully (assertEventFired passed)
273273
// We don't need to check the event data again since assertEventFired confirms it was fired
274+
275+
// The exhausted job must be stamped terminal so it no longer counts as
276+
// queued/in-progress (otherwise isQueued() wedges callers behind it).
277+
$reloaded = $QueuedJobs->get($job->id);
278+
$this->assertSame(QueuedJobsTable::STATUS_ABORTED, $reloaded->status);
274279
}
275280

276281
/**

0 commit comments

Comments
 (0)