feat(queue): add full jitter to the reclaimer interval

HA
10h ago
+282-8

Summary

The queue reclaimer now waits a random 0–30s between cycles (full jitter, re-drawn every cycle) instead of a fixed 30s ticker, and a new
ac_reclaimer_claims_total{stream, result} counter records each claimed message as recovered or poison.

Motivation

runReclaimer used time.NewTicker(30 * time.Second). A ticker keeps a fixed phase, so nodes that restart together (rollout, Redis/network recovery)
run XAUTOCLAIM + XPENDING against Redis in the same instant on every cycle, forever. Operators also had no metric for how many stuck messages the
reclaimer recovers vs. dead-letters.

Closes #122

Implementation notes

  • Added fullJitter(base, r), which returns r() * base, i.e. a wait in [0, base). The random source is a jitter func() float64 field on
    DualConsumer (default math/rand/v2.Float64) so tests can inject a fixed sequence.
  • Replaced the ticker with a time.Timer that is Reset with a fresh draw after each cycle. Drawing once at startup would only offset the nodes while
    keeping a shared fixed period; re-drawing every cycle keeps them from re-aligning.
  • Trade-off: full jitter halves the mean interval (15s instead of 30s), so each node scans about twice as often, but the scans no longer arrive as
    synchronized bursts.
  • ac_reclaimer_claims_total increments only after the Redis side succeeds: recovered after the handler succeeds and the message is ACKed; poison
    after the DLQ XADD and the ACK. A handler error or failed DLQ write leaves the message pending and is not counted. Label cardinality is bounded (3
    streams × 2 results).
  • math/rand use carries //nolint:gosec (G404): timer spreading does not need cryptographic randomness.
  • Updated docs/ARCHITECTURE.md (reclaimer description and metrics table) and CHANGELOG.md.

Questions for the reviewer

Q1 — What existing tests cover the code you changed?
A: TestDualConsumer_Reclaimer_ReprocessesIdleMessages (calls reclaimStream directly) and TestDualConsumer_PoisonMessage_DeadLettered in
internal/queue/stream_test.go. Note: the latter never makes the message pending and stops after 200ms, before the first reclaim cycle, so it does not
exercise dead-lettering. The new poison test below covers that path.

Q2 — What new tests did you add, and what cases do they cover?
A: In internal/queue/reclaimer_test.go, all hermetic (miniredis plus seeded PRNGs):

  • TestFullJitter_StaysBelowBase: exact values for fixed inputs; 10k draws all within [0, 30s).
  • TestFullJitter_IsUniform: 10k draws spread across 10 buckets (each within ±20% of expected).
  • TestFullJitter_SpreadsSimultaneousStarts: 20 nodes starting together; at most 4 fire their first cycle in any one second.
  • TestReclaimer_RedrawsJitterEveryCycle: drives StartReclaimer and asserts a new wait is drawn after each cycle (fails against the old ticker).
  • TestReclaimer_CountsRecoveredClaims / TestReclaimer_FailedHandlerIsNotCounted: recovered +1 on success, +0 on handler error.
  • TestReclaimer_CountsPoisonClaims: message delivered 5 times is dead-lettered, handler not called, poison +1, ac:dlq:poison length 1.

Q3 — Does this change affect any authenticated or unauthenticated API paths? If yes, how?
A: No. The change is limited to the background reclaimer goroutine and a new Prometheus counter on the existing metrics endpoint.

Q4 — Did you run golangci-lint run locally and get 0 issues?
A: Yes. golangci-lint v2.15.0 on ./internal/queue/... and ./internal/metrics/... reports no issues in the changed code. The only reports were gofmt
line-ending warnings caused by my Windows CRLF checkout; changed files pass gofmt -s when normalized to LF.

Q5 — Is this the only open PR you have against development?
A: Yes.

Checklist

  • go test -race -count=1 -timeout=120s passes for internal/queue, internal/metrics, internal/worker and cmd/... (run in a golang:1.27.2
    Linux container; internal/db testcontainers tests run in CI)
  • golangci-lint run --timeout=5m reports 0 issues in changed code
  • gofmt -s -l reports no unformatted files among the changed files
  • All five questions above are answered (not left as …)
  • No secrets or credentials in code / config
  • New dependencies reviewed and justified (none added; math/rand/v2 is stdlib)
  • Related issue linked in "Closes #" above

A: Yes. golangci-lint v2.15.0 on ./internal/queue/... and ./internal/metrics/... reports no issues in the changed code. The only reports were gofmt
line-ending warnings caused by my Windows CRLF checkout; changed files pass gofmt -s when normalized to LF.

Q5 — Is this the only open PR you have against development?
A: Yes.

Checklist

  • go test -race -count=1 -timeout=120s passes for internal/queue, internal/metrics, internal/worker and cmd/... (run in a golang:1.27.2
    Linux container; internal/db testcontainers tests run in CI)
  • golangci-lint run --timeout=5m reports 0 issues in changed code
  • gofmt -s -l reports no unformatted files among the changed files
  • All five questions above are answered (not left as …)
  • No secrets or credentials in code / config
  • New dependencies reviewed and justified (none added; math/rand/v2 is stdlib)
  • Related issue linked in "Closes #" above

Summary by cubic

Adds full jitter to the reclaimer loop and a new metric for claimed messages.

The reclaimer used a fixed 30s ticker, so nodes that restarted together stayed phase-locked and hit Redis with XAUTOCLAIM and XPENDING in the same instant every cycle. It now re-arms a timer with a fresh wait drawn from [0, 30s) after each cycle, breaking the lock. A new ac_reclaimer_claims_total{stream, result} counter records each claim as recovered or poison.

Notes

  • Full jitter halves the mean interval (15s instead of 30s): each node scans about twice as often, but scans no longer arrive in synchronized bursts.
  • The counter increments only after Redis succeeds: recovered when the handler returns and the message is ACKed, poison after the DLQ XADD and ACK. Handler errors or failed DLQ writes leave the message pending and uncounted.
  • Closes #122.

Summary by CodeRabbit

  • New Features
    • Reclaimer claim outcomes are now tracked with a metric that distinguishes recovered messages from poison messages, broken down by stream.
  • Improvements
    • Reclaimer cycles now begin after a newly randomized delay of up to 30 seconds, instead of using a fixed 30-second interval.
  • Documentation
    • Updated architecture and changelog documentation to reflect the reclaimer timing and claim metrics.