Add reties delay logic

[YaroslavStryhun]

Hello!

• Type: bug fix | new feature | code quality | documentation
• Link to issue:

In raising this pull request, I confirm the following (please check boxes):

• I have read and understood the Contributing Guidelines?
• I have checked that another pull request for this purpose does not exist.
• I wrote some tests for this PR.

Small description of change:

Thanks

Summary by CodeRabbit

• New Features

  • Optional per-job exponential backoff for retries; per-job backoff is persisted with each job.
  • New global defaults for backoff initial delay and factor.

• API Changes

  • Enqueue/put now accepts an optional backoff parameter when creating jobs.

• Behavior Changes

  • Jobs with backoff are rescheduled using initial delay and factor based on retry count; jobs without backoff retain existing behavior.

[coderabbitai]

Walkthrough

Adds per-job exponential backoff: new BackoffStrategyDTO, extends Queue::put to accept and serialize backoff into the enqueue payload, and updates qless.lua with config defaults, per-job backoff persistence, and backoff-aware retry scheduling on lock invalidation.

Changes

Cohort / File(s)	Summary
DTO src/Queues/DTO/BackoffStrategyDTO.php	New immutable DTO with initialDelay and factor; constructor, getInitialDelay(): int, getFactor(): int, and toArray(): array returning {'factor' => int, 'initial_delay' => int}.
Queue API src/Queues/Queue.php	Queue::put signature extended to accept ?BackoffStrategyDTO $backoffStrategyDTO = null; serializes backoff via toArray() and includes it in the payload sent to the client; adds use Qless\Queues\DTO\BackoffStrategyDTO.
Core backoff behavior src/qless-core/qless.lua	Adds backoff-initial-delay and backoff-factor defaults; QlessQueue:put persists per-job backoff JSON when provided; invalidate_locks reads per-job backoff (or config defaults) and, when initial_delay > 0, computes delay = initial_delay * (factor ^ retry_count) to schedule retries instead of immediate requeue; retains existing behavior when no backoff or initial_delay == 0.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor App
  participant Queue as PHP Queue
  participant Client as Qless Client
  participant Lua as qless.lua

  App->>Queue: put(class, data, ..., backoffDTO?)
  Queue->>Queue: backoffDTO?.toArray()
  Queue->>Client: enqueue(payload + backoff)
  Client->>Lua: QlessQueue:put(options with backoff)
  Lua->>Lua: hmset job fields (+ backoff JSON if provided)
  Lua-->>Client: jid
  Client-->>Queue: jid
  Queue-->>App: jid

Loading

sequenceDiagram
  autonumber
  participant Worker as Worker
  participant Lua as qless.lua
  note over Worker,Lua: Lock expires / invalidate_locks runs
  Lua->>Lua: Read job.backoff or config defaults
  alt initial_delay > 0 and backoff provided
    Lua->>Lua: retry_count from history
    Lua->>Lua: delay = initial_delay * (factor ^ retry_count)
    Lua->>Lua: move job to scheduled with computed delay (state='scheduled')
  else No backoff or initial_delay == 0
    Lua->>Lua: requeue job immediately (existing path)
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Pre-merge checks (1 passed, 2 warnings)

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The description is a generic pull request template with no information about the actual implementation, missing any mention of the added backoff strategy or retry delay logic changes.	Please update the description to include a concise summary of the implemented changes, such as the addition of BackoffStrategyDTO, updates to Queue::put parameters, Lua backoff configuration, and any related tests.
Docstring Coverage	⚠️ Warning	Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title succinctly indicates the addition of retry delay logic, which aligns with the primary changeset of introducing backoff strategy and scheduling exponential delays.

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch add-retry-delay

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 5

🧹 Nitpick comments (2)

src/Queues/DTO/BackoffStrategyDTO.php (1)

10-15: Use typed properties for stricter contracts.

Keeps types consistent with constructor signatures.

-    private $initialDelay;
+    private int $initialDelay;
@@
-    private $factor;
+    private int $factor;

src/Queues/Queue.php (1)

125-128: Backoff payload wiring LGTM, but consider JSON error handling.

Optional: use JSON_THROW_ON_ERROR for tags/depends/backoff to fail fast on invalid payloads (if PHP min version allows).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 5e47114 and 7e45708.

📒 Files selected for processing (3)

• src/Queues/DTO/BackoffStrategyDTO.php (1 hunks)
• src/Queues/Queue.php (3 hunks)
• src/qless-core/qless.lua (3 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/Queues/Queue.php (2)

src/Queues/DTO/BackoffStrategyDTO.php (2)

• BackoffStrategyDTO (5-46)
• toArray (39-45)

src/Jobs/JobData.php (1)

• toArray (32-35)

🪛 GitHub Actions: PR checks and tests

src/Queues/Queue.php

[error] 111-111: Lua script invocation failed: 'Lua redis() command arguments must be strings or integers' (script f_15c3572b35acf0713ccad4d65166bf6c84b5cdfa).

🔇 Additional comments (3)

src/Queues/Queue.php (1)

89-91: Document optional BackoffStrategyDTO addition. Backward-compatible; update CHANGELOG and README.

src/qless-core/qless.lua (2)

389-392: Defaults LGTM.

backoff-initial-delay=0 keeps legacy behavior; factor=3 is a sane default.

---

1870-1999: Re-run CI: hmset/unpack and key-prefix issues appear resolved; no remaining occurrences detected.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/qless-core/qless.lua (2)

388-392: Backoff config defaults added — looks good; consider max-cap (and optional jitter).

Defaults are sensible. As a small enhancement, add a configurable cap (e.g., backoff-max-delay) to prevent unbounded delays, and optionally a jitter knob to avoid thundering herds. You can wire these in later where delay is computed.

Apply this minimal diff to extend defaults:

   ['jobs-history']       = 604800,
   ['jobs-failed-history'] = 604800,
   -- retries logic
   ['backoff-initial-delay'] = 0,  -- Default delay in seconds. 0 means disabled.
-  ['backoff-factor']        = 3   -- Exponential factor.
+  ['backoff-factor']        = 3,  -- Exponential factor.
+  ['backoff-max-delay']     = 0,  -- 0 = no cap; otherwise cap delay in seconds.
+  ['backoff-jitter']        = 0   -- 0..1 fraction of delay to jitter (optional).

---

1916-1993: Exponential backoff scheduling is correct overall; two small fixes.

• Possible off-by-one in attempt index: after you decrement remaining, the first retry yields attempt_index = 1, making the initial delay larger than intended. If you want the first retry to use initial_delay, adjust as below. If the current behavior is intentional, please disregard.
• Extra arg in hdel: passing 0 to hdel tries to delete field "0". Drop it.

Apply these diffs:

-      redis.call('hdel', QlessJob.ns .. jid, 'grace', 0)
+      redis.call('hdel', QlessJob.ns .. jid, 'grace')

-            local attempt_index = total_retries - retries_left
-            local delay = initial_delay * (backoff_factor ^ attempt_index)
+            -- 0-based attempt index so first retry uses initial_delay
+            local attempt_index = math.max(0, total_retries - retries_left - 1)
+            local delay = initial_delay * (backoff_factor ^ attempt_index)
+            -- Optional cap if configured
+            local max_delay = tonumber(Qless.config.get('backoff-max-delay', 0))
+            if max_delay and max_delay > 0 and delay > max_delay then
+              delay = max_delay
+            end

For completeness, also fix the same hdel call earlier in invalidate_lock_jid:

-  redis.call('hdel', QlessJob.ns .. jid, 'grace', 0)
+  redis.call('hdel', QlessJob.ns .. jid, 'grace')

Notes:

• Reading backoff via QlessJob.ns .. jid is now correct — nice catch.
• If you keep the current attempt indexing, add a brief comment stating that initial_delay applies from the second retry onward to avoid confusion.

Also applies to: 1861-1865

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 71ba6aa and 987019c.

📒 Files selected for processing (1)

• src/qless-core/qless.lua (3 hunks)

🔇 Additional comments (1)

src/qless-core/qless.lua (1)

1590-1609: LGTM: hmset/unpack fix is valid and backoff is JSON-encoded upstream (src/Queues/Queue.php:126-127).