From aa57d0b962956e8e440e0a61231c641e23b2097c Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Thu, 28 May 2026 18:06:19 +0200 Subject: [PATCH 1/2] Exclude aborted jobs from pending count and stats getPendingCount() and getPendingStats() counted jobs that had exhausted their retries (status = aborted) even though they are terminal and will never run again. That inflated the admin pending total and any external caller. They now exclude status = aborted, matching isQueued() and the admin status filter. Two interactions handled: - The admin dashboard derives pending = totalPending - running - failed. Aborted rows carry a failure_message, so the failed operand still counted them; the derivation now subtracts only still-retriable failures to avoid double-removing aborted rows (which zeroed out real pending work). - reset() now clears status so a reset job loses its terminal aborted state and counts as pending and gets picked up again. --- src/Controller/Admin/QueueController.php | 18 ++++++- src/Model/Table/QueuedJobsTable.php | 38 +++++++++---- .../Model/Table/QueuedJobsTableTest.php | 53 +++++++++++++++++++ 3 files changed, 96 insertions(+), 13 deletions(-) diff --git a/src/Controller/Admin/QueueController.php b/src/Controller/Admin/QueueController.php index 71d5676a..758cf9a4 100644 --- a/src/Controller/Admin/QueueController.php +++ b/src/Controller/Admin/QueueController.php @@ -113,8 +113,22 @@ public function index() { 'failure_message IS NOT' => null, ]) ->count(); - // Pending = total pending minus running and failed (to avoid double counting) - $pendingJobs = max(0, $totalPending - $runningJobs - $failedJobs); + // Aborted jobs (retries exhausted) carry a failure_message but are + // terminal and already excluded from getPendingCount()/getPendingStats(). + // Subtract only the still-retriable failures here so aborted rows are not + // removed twice (which would under-count, or zero out, real pending work). + $retriableFailedJobs = $this->QueuedJobs->find() + ->where([ + 'completed IS' => null, + 'failure_message IS NOT' => null, + 'OR' => [ + 'status IS' => null, + 'status !=' => $this->QueuedJobs::STATUS_ABORTED, + ], + ]) + ->count(); + // Pending = total pending minus running and still-retriable failed (to avoid double counting) + $pendingJobs = max(0, $totalPending - $runningJobs - $retriableFailedJobs); $configurations = (array)Configure::read('Queue'); diff --git a/src/Model/Table/QueuedJobsTable.php b/src/Model/Table/QueuedJobsTable.php index 1a4feb04..c2e6f8b9 100644 --- a/src/Model/Table/QueuedJobsTable.php +++ b/src/Model/Table/QueuedJobsTable.php @@ -844,6 +844,9 @@ public function reset(?int $id = null, bool $full = false): int { 'failure_message' => null, 'output' => null, 'memory' => null, + // Clear the terminal aborted status so a reset job counts as pending + // again (getPendingCount()/isQueued() exclude status = aborted). + 'status' => null, ]; $conditions = [ 'completed IS' => null, @@ -1103,16 +1106,35 @@ public function getPendingStats(): SelectQuery { 'failure_message', 'memory', ], - 'conditions' => [ - 'completed IS' => null, + 'conditions' => $this->pendingConditions(), + ]; + + return $this->find('all', ...$findCond); + } + + /** + * Shared WHERE conditions for "pending" jobs: not completed, due to run, and + * not terminally aborted. An aborted job keeps `completed IS NULL` but will + * never run again (retries exhausted), so it must not be reported as pending. + * + * @return array + */ + protected function pendingConditions(): array { + return [ + 'completed IS' => null, + [ 'OR' => [ 'notbefore <=' => new DateTime(), 'notbefore IS' => null, ], ], + [ + 'OR' => [ + 'status IS' => null, + 'status !=' => static::STATUS_ABORTED, + ], + ], ]; - - return $this->find('all', ...$findCond); } /** @@ -1125,13 +1147,7 @@ public function getPendingStats(): SelectQuery { */ public function getPendingCount(): int { return $this->find() - ->where([ - 'completed IS' => null, - 'OR' => [ - 'notbefore <=' => new DateTime(), - 'notbefore IS' => null, - ], - ]) + ->where($this->pendingConditions()) ->count(); } diff --git a/tests/TestCase/Model/Table/QueuedJobsTableTest.php b/tests/TestCase/Model/Table/QueuedJobsTableTest.php index acbd2aa7..57412c85 100644 --- a/tests/TestCase/Model/Table/QueuedJobsTableTest.php +++ b/tests/TestCase/Model/Table/QueuedJobsTableTest.php @@ -874,6 +874,59 @@ public function testMarkJobAborted() { $this->assertNull($reloaded->completed); } + /** + * Aborted jobs keep `completed IS NULL` but are terminal, so they must not + * be counted by the pending stats/count used on the admin dashboard. + * + * @return void + */ + public function testGetPendingCountExcludesAborted() { + $pending = $this->QueuedJobs->newEntity([ + 'key' => 'key', + 'job_task' => 'FooBar', + 'reference' => 'pending-one', + ]); + $this->QueuedJobs->saveOrFail($pending); + + $aborted = $this->QueuedJobs->newEntity([ + 'key' => 'key', + 'job_task' => 'FooBar', + 'reference' => 'aborted-one', + ]); + $this->QueuedJobs->saveOrFail($aborted); + $this->QueuedJobs->markJobAborted($aborted); + + $this->assertSame(1, $this->QueuedJobs->getPendingCount()); + + $statsRefs = $this->QueuedJobs->getPendingStats()->all()->extract('reference')->toArray(); + $this->assertContains('pending-one', $statsRefs); + $this->assertNotContains('aborted-one', $statsRefs); + } + + /** + * Resetting an aborted job for rerun must clear its terminal status so it + * counts as pending again and gets picked up. + * + * @return void + */ + public function testResetClearsAbortedStatus() { + $job = $this->QueuedJobs->newEntity([ + 'key' => 'key', + 'job_task' => 'FooBar', + 'reference' => 'reset-me', + 'attempts' => 3, + ]); + $this->QueuedJobs->saveOrFail($job); + $this->QueuedJobs->markJobAborted($job); + $this->assertSame(0, $this->QueuedJobs->getPendingCount()); + + $this->QueuedJobs->reset($job->id, true); + + $reloaded = $this->QueuedJobs->get($job->id); + $this->assertNull($reloaded->status); + $this->assertSame(1, $this->QueuedJobs->getPendingCount()); + } + /** * @return void */ From 96d70b127f0ad12d75a16dcb0942d4eea009567d Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Thu, 28 May 2026 18:10:31 +0200 Subject: [PATCH 2/2] Order union type hint as object|array|null to satisfy psr2r-sniffer --- src/Model/Table/QueuedJobsTable.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Model/Table/QueuedJobsTable.php b/src/Model/Table/QueuedJobsTable.php index c2e6f8b9..51b29c4b 100644 --- a/src/Model/Table/QueuedJobsTable.php +++ b/src/Model/Table/QueuedJobsTable.php @@ -210,7 +210,7 @@ public function createConfig(): JobConfig { * * @return \Queue\Model\Entity\QueuedJob Saved job entity (or the existing pending entity if `unique` deduped). */ - public function createJob(string $jobTask, array|object|null $data = null, array|JobConfig $config = []): QueuedJob { + public function createJob(string $jobTask, object|array|null $data = null, JobConfig|array $config = []): QueuedJob { if (!$config instanceof JobConfig) { $config = $this->createConfig()->fromArray($config); }