Skip to content

Commit b27c180

Browse files
authored
Exclude aborted jobs from pending count and stats (#505)
* 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. * Order union type hint as object|array|null to satisfy psr2r-sniffer
1 parent 4b4b934 commit b27c180

3 files changed

Lines changed: 97 additions & 14 deletions

File tree

src/Controller/Admin/QueueController.php

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,22 @@ public function index() {
113113
'failure_message IS NOT' => null,
114114
])
115115
->count();
116-
// Pending = total pending minus running and failed (to avoid double counting)
117-
$pendingJobs = max(0, $totalPending - $runningJobs - $failedJobs);
116+
// Aborted jobs (retries exhausted) carry a failure_message but are
117+
// terminal and already excluded from getPendingCount()/getPendingStats().
118+
// Subtract only the still-retriable failures here so aborted rows are not
119+
// removed twice (which would under-count, or zero out, real pending work).
120+
$retriableFailedJobs = $this->QueuedJobs->find()
121+
->where([
122+
'completed IS' => null,
123+
'failure_message IS NOT' => null,
124+
'OR' => [
125+
'status IS' => null,
126+
'status !=' => $this->QueuedJobs::STATUS_ABORTED,
127+
],
128+
])
129+
->count();
130+
// Pending = total pending minus running and still-retriable failed (to avoid double counting)
131+
$pendingJobs = max(0, $totalPending - $runningJobs - $retriableFailedJobs);
118132

119133
$configurations = (array)Configure::read('Queue');
120134

src/Model/Table/QueuedJobsTable.php

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ public function createConfig(): JobConfig {
210210
*
211211
* @return \Queue\Model\Entity\QueuedJob Saved job entity (or the existing pending entity if `unique` deduped).
212212
*/
213-
public function createJob(string $jobTask, array|object|null $data = null, array|JobConfig $config = []): QueuedJob {
213+
public function createJob(string $jobTask, object|array|null $data = null, JobConfig|array $config = []): QueuedJob {
214214
if (!$config instanceof JobConfig) {
215215
$config = $this->createConfig()->fromArray($config);
216216
}
@@ -844,6 +844,9 @@ public function reset(?int $id = null, bool $full = false): int {
844844
'failure_message' => null,
845845
'output' => null,
846846
'memory' => null,
847+
// Clear the terminal aborted status so a reset job counts as pending
848+
// again (getPendingCount()/isQueued() exclude status = aborted).
849+
'status' => null,
847850
];
848851
$conditions = [
849852
'completed IS' => null,
@@ -1103,16 +1106,35 @@ public function getPendingStats(): SelectQuery {
11031106
'failure_message',
11041107
'memory',
11051108
],
1106-
'conditions' => [
1107-
'completed IS' => null,
1109+
'conditions' => $this->pendingConditions(),
1110+
];
1111+
1112+
return $this->find('all', ...$findCond);
1113+
}
1114+
1115+
/**
1116+
* Shared WHERE conditions for "pending" jobs: not completed, due to run, and
1117+
* not terminally aborted. An aborted job keeps `completed IS NULL` but will
1118+
* never run again (retries exhausted), so it must not be reported as pending.
1119+
*
1120+
* @return array<int|string, mixed>
1121+
*/
1122+
protected function pendingConditions(): array {
1123+
return [
1124+
'completed IS' => null,
1125+
[
11081126
'OR' => [
11091127
'notbefore <=' => new DateTime(),
11101128
'notbefore IS' => null,
11111129
],
11121130
],
1131+
[
1132+
'OR' => [
1133+
'status IS' => null,
1134+
'status !=' => static::STATUS_ABORTED,
1135+
],
1136+
],
11131137
];
1114-
1115-
return $this->find('all', ...$findCond);
11161138
}
11171139

11181140
/**
@@ -1125,13 +1147,7 @@ public function getPendingStats(): SelectQuery {
11251147
*/
11261148
public function getPendingCount(): int {
11271149
return $this->find()
1128-
->where([
1129-
'completed IS' => null,
1130-
'OR' => [
1131-
'notbefore <=' => new DateTime(),
1132-
'notbefore IS' => null,
1133-
],
1134-
])
1150+
->where($this->pendingConditions())
11351151
->count();
11361152
}
11371153

tests/TestCase/Model/Table/QueuedJobsTableTest.php

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -874,6 +874,59 @@ public function testMarkJobAborted() {
874874
$this->assertNull($reloaded->completed);
875875
}
876876

877+
/**
878+
* Aborted jobs keep `completed IS NULL` but are terminal, so they must not
879+
* be counted by the pending stats/count used on the admin dashboard.
880+
*
881+
* @return void
882+
*/
883+
public function testGetPendingCountExcludesAborted() {
884+
$pending = $this->QueuedJobs->newEntity([
885+
'key' => 'key',
886+
'job_task' => 'FooBar',
887+
'reference' => 'pending-one',
888+
]);
889+
$this->QueuedJobs->saveOrFail($pending);
890+
891+
$aborted = $this->QueuedJobs->newEntity([
892+
'key' => 'key',
893+
'job_task' => 'FooBar',
894+
'reference' => 'aborted-one',
895+
]);
896+
$this->QueuedJobs->saveOrFail($aborted);
897+
$this->QueuedJobs->markJobAborted($aborted);
898+
899+
$this->assertSame(1, $this->QueuedJobs->getPendingCount());
900+
901+
$statsRefs = $this->QueuedJobs->getPendingStats()->all()->extract('reference')->toArray();
902+
$this->assertContains('pending-one', $statsRefs);
903+
$this->assertNotContains('aborted-one', $statsRefs);
904+
}
905+
906+
/**
907+
* Resetting an aborted job for rerun must clear its terminal status so it
908+
* counts as pending again and gets picked up.
909+
*
910+
* @return void
911+
*/
912+
public function testResetClearsAbortedStatus() {
913+
$job = $this->QueuedJobs->newEntity([
914+
'key' => 'key',
915+
'job_task' => 'FooBar',
916+
'reference' => 'reset-me',
917+
'attempts' => 3,
918+
]);
919+
$this->QueuedJobs->saveOrFail($job);
920+
$this->QueuedJobs->markJobAborted($job);
921+
$this->assertSame(0, $this->QueuedJobs->getPendingCount());
922+
923+
$this->QueuedJobs->reset($job->id, true);
924+
925+
$reloaded = $this->QueuedJobs->get($job->id);
926+
$this->assertNull($reloaded->status);
927+
$this->assertSame(1, $this->QueuedJobs->getPendingCount());
928+
}
929+
877930
/**
878931
* @return void
879932
*/

0 commit comments

Comments
 (0)