Skip to content

Conversation

@ChiragAgg5k
Copy link
Member

@ChiragAgg5k ChiragAgg5k commented Feb 2, 2026

Summary

  • Add TTL support to Connection interface set() and setArray() methods
  • Implement TTL in Redis connection using setex() when TTL > 0
  • Add optional jobTtl property to Queue class (default: 0 / no expiration)
  • Clean up old job records after retry re-enqueue to prevent leak

Problem

Job records stored at {namespace}.jobs.{queue}.{pid} were accumulating indefinitely because:

  1. Failed jobs added PID to failed queue but didn't clean up the job record after retry

Solution

  • Job records can now have an optional configurable TTL (default: no expiration to preserve existing behavior)
  • Successful jobs are deleted immediately (existing behavior)
  • Retried jobs have their old records deleted after re-enqueue
  • Users can opt-in to TTL by specifying jobTtl when creating a Queue

Usage

// No TTL (default - preserves existing behavior)
$queue = new Queue('my-queue');

// With 1 hour TTL for job records
$queue = new Queue('my-queue', 'utopia-queue', 3600);

Test plan

  • Enqueue a job without TTL and verify no expiration is set
  • Enqueue a job with TTL and verify the job record has TTL set
  • Run retry() and verify old job record is deleted after re-enqueue
  • Verify existing tests pass

Summary by CodeRabbit

  • New Features
    • Configurable job expiration (TTL) — jobs can be set to expire automatically when a TTL is configured (default: disabled).
  • Bug Fixes
    • Improved retry handling: successful retries remove old records; failed re-enqueues route jobs to the failed queue to prevent job loss and incorrect counters.

Job records stored at {namespace}.jobs.{queue}.{pid} were accumulating
indefinitely because no TTL was set. This adds:

- TTL parameter to Connection interface set() and setArray() methods
- TTL support in Redis connection using setex()
- Configurable jobTtl property on Queue class (default: 24 hours)
- Cleanup of old job records after retry re-enqueue
@coderabbitai
Copy link

coderabbitai bot commented Feb 2, 2026

Walkthrough

Adds TTL support for stored jobs and adjusts retry behavior. The Queue constructor gains a public int property jobTtl (default 86400). Utopia\Queue\Connection::set and ::setArray signatures now accept int $ttl = 0. Redis and RedisCluster connection implementations use SETEX when ttl > 0. The Redis broker (Broker/Redis.php) now passes $queue->jobTtl when moving jobs to the jobs store in consume. In retry, if enqueue succeeds the old job record is removed and the processed counter increments; if enqueue fails the PID is re-queued to the failed queue and processed is not incremented.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: add TTL to job records to prevent memory leak' clearly and concisely summarizes the main change: implementing TTL support for job records to address a memory leak issue. It is specific, related to the core changeset, and follows conventional commit format.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-redis-job-record-memory-leak

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@src/Queue/Broker/Redis.php`:
- Around line 152-158: The code currently calls $this->enqueue($queue,
$job->getPayload()) without checking its result and immediately deletes the job
record via $this->connection->remove(...), which can drop the job if enqueue
fails; update the logic to capture the result/exception from enqueue() and only
perform
$this->connection->remove("{$queue->namespace}.jobs.{$queue->name}.{$pid}") when
enqueue() succeeds, and if enqueue() fails, restore or re-add the PID to the
failed queue (or leave the job record intact) and log the failure so the job
isn't lost; reference the enqueue() call and the connection->remove(...)
invocation when making the change.

In `@src/Queue/Connection/Redis.php`:
- Around line 122-133: The RedisCluster class methods setArray and set must be
updated to match the Connection interface by adding the int $ttl = 0 parameter
to both signatures (setArray(string $key, array $value, int $ttl = 0) and
set(string $key, string $value, int $ttl = 0)), and implement TTL handling by
calling setex($key, $ttl, $value) when $ttl > 0 and getRedis()->set($key,
$value) otherwise; ensure setArray delegates to set($key, json_encode($value),
$ttl).
🧹 Nitpick comments (1)
src/Queue/Queue.php (1)

10-10: Consider adding validation for jobTtl.

Negative TTL values would be passed to Redis setex, which would cause an error. Consider validating that jobTtl is positive, similar to the name validation.

🛡️ Proposed validation
     public function __construct(
         public string $name,
         public string $namespace = 'utopia-queue',
         public int $jobTtl = 86400,
     ) {
         if (empty($this->name)) {
             throw new \InvalidArgumentException('Cannot create queue with empty name.');
         }
+        if ($this->jobTtl <= 0) {
+            throw new \InvalidArgumentException('Job TTL must be a positive integer.');
+        }
     }

Update setArray and set methods to match the Connection interface
with TTL parameter support.
@ChiragAgg5k ChiragAgg5k force-pushed the fix-redis-job-record-memory-leak branch from 898b297 to cd07b1b Compare February 2, 2026 09:42
Only delete job record after successful re-enqueue. If enqueue fails,
re-add PID to failed queue so the job can be retried later.
@ChiragAgg5k ChiragAgg5k force-pushed the fix-redis-job-record-memory-leak branch from cd07b1b to 659a51b Compare February 2, 2026 09:52
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@src/Queue/Broker/Redis.php`:
- Line 48: The current call to
$this->connection->setArray("{$queue->namespace}.jobs.{$queue->name}.{$message->getPid()}",
$nextMessage, $queue->jobTtl) starts the TTL at consume time and can let job
records expire during long processing; update the Broker\Redis logic so TTL is
not started at consume: either move TTL application to when a job is moved into
the failed list (apply TTL there), or refresh/set the TTL at that failure/retry
point, and update retry() to skip missing records instead of breaking if a job
key is absent; locate usages of setArray in the Redis class and modify the path
that sets jobTtl and the retry() implementation to handle missing keys
gracefully or defer TTL until failure.

In `@src/Queue/Queue.php`:
- Around line 7-11: The constructor for class Queue sets public int $jobTtl
without validating it, which can pass a negative TTL into Redis SETEX and cause
errors; update the __construct method to check the $jobTtl parameter (the public
property jobTtl) and either throw an InvalidArgumentException or normalize
negative values to 0 (choose one consistent behavior for the codebase) before
assigning it to the property so no negative TTL is stored or used.

@ChiragAgg5k ChiragAgg5k merged commit 5051c08 into main Feb 2, 2026
8 checks passed
@ChiragAgg5k ChiragAgg5k deleted the fix-redis-job-record-memory-leak branch February 2, 2026 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants