perf: stop reading every task_result payload to prune keys Redis already expires

[adhikjoshi]

The problem

pruneOldTaskResults() ran every PRUNE_CHECK_INTERVAL inside the startWorkers() loop. It SCANned the entire keyspace and GET the JSON of everytask_result:* key just to compare one timestamp.

It could never delete anything. task_result keys are always written with a TTL — setex(..., 3600, ...) in storeResult(), TASK_RESULT_RETENTION in the webhook paths — while the prune only deletes when now - finished_at > TASK_RESULT_RETENTION (86400). A 1-hour key is gone 23 hours before the prune would look at it; a 24-hour key expires at the exact moment the condition becomes true. Redis expiry always wins the race.

Measured on a production shard (redis-va8u7z49) over 26 days:

command	calls
SCAN	196,726,349
GET	228,765,062
DEL	2

That is ~2.2 hours of blocked event loop and 13.5 TB of network output to issue 2 deletes. Across the fleet this scan was roughly 85% of all Redis CPU time, and it is the direct cause of the LRANGE/SCAN entries dominating the slowlog and of intermittent ModelQ enqueue failed: read error on connection errors.

Sampling 1,200 live task_result keys across three shards found zero without a TTL.

The change

• Drop the call from the startWorkers() loop. Redis TTL is the mechanism; nothing needs to poll for it.
• Keep the method as a manual repair entry point (now public) for the one case Redis cannot handle by itself: a key whose TTL went missing (a write path that forgot the EX, a RENAME/RESTORE that dropped it), which would otherwise live forever.
• The rewritten method pipelines TTL — an 8-byte reply — and reads a value only for keys already known to be broken. A healthy keyspace is now walked without transferring a single payload.
• UNLINK instead of DEL, so multi-MB frees land off the main thread.
• SCAN COUNT 100 → 500.

Behaviour is otherwise unchanged: an old TTL-less key is still deleted along with its task: twin. A TTL-less key that is not yet old gets an expiry set, so it can't leak.

Why not MGET

Batching reads makes this worse, not better. task_result payloads reach 7 MB, so one MGET builds a single enormous client output buffer — and that is precisely what pushes RSS past the container memory limit and gets redis-server OOM-killed.

Testing

New tests/Integration/PruneTaskResultsTest.php uses a Redis subclass that counts value reads. Since phpredis multi(Redis::PIPELINE) returns the same object, this catches pipelined GETs too.

• testHealthyKeysAreNeverRead — the control case: 50 healthy keys cost 0 value reads
• testOnlyTheKeyThatLostItsTtlIsRead — 50 healthy + 1 leaked costs exactly 1 read, and the task: twin is deleted
• testRecentOrphanIsBoundedNotDeleted — a fresh TTL-less key is kept but given an expiry
• testWorkerLoopDoesNotScanTaskResults — the loop no longer calls the scan

Mutation-tested, each reverted independently and confirmed to turn the suite red:

1. reverting the TTL filter → getCalls 50 instead of 0
2. re-adding the call to the worker loop → loop test fails
3. dropping the task: twin UNLINK → twin assertion fails

Full suite: 134 tests, same 13 pre-existing Integration failures as main (task execution / streaming / enqueue — unrelated and failing before this branch). PHPStan level 6: identical 35 pre-existing errors, none introduced.

Note

The Python modelq library has the same dead loop at modelq/app/base.py. The equivalent fix is prepared there and needs its own release.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is enabled.}