perf(retrymq): cut idle Redis polling to one command per interval - #1026
Merged
Conversation
The retry monitor polled every 100ms, and each poll cost 5 client-observable commands over 2 round trips: a MULTI/HMGET/TIME/EXEC to fetch the queue's vt/delay/maxsize and the server clock, then the EVALSHA that used them. The cost was per monitor instance and independent of traffic, so an empty queue cost as much as a busy one and total load scaled with replica count. Make receiveMessage self-contained — it reads vt from the :Q hash and calls TIME itself, which upstream RSMQ could not do under pre-Redis-5 verbatim script replication. When nothing is due it also returns the time until the zset's earliest score, so the monitor sleeps until the next message comes due instead of a flat interval. The zset holds both not-yet-due retries and in-flight messages hidden by vt, so that score is the correct wake time in either case, and every sleep is still capped. RETRY_POLL_BACKOFF_MS is redefined as that cap, default 100 -> 30000, and the effective value is clamped to the shortest configured retry delay so the idle interval can never make a retry late. The consecutive-error backoff ladder no longer derives from it, keeping the ~1 minute of transient-infra tolerance fixed. At a 30s cap: 129.6M -> 86.4K commands per month, per monitor. Closes #1014 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
See comment on issue regarding next message timestamp |
…ew tests - IdleSleepWakesOnDueMessage: wait for the Monitor goroutine to exit after cancel so it cannot log via t after the test completes, and assert the execution window as an elapsed range with a looser upper bound (3s) for loaded CI. - MonitorRetriesTransientErrors: guard the msgs slice with a lock (msgLog helper) — exec runs on the monitor's goroutines, so the require.Eventually read raced with appends under -race. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… tests The scheduler has always run exec on the monitor's goroutines, so the unsynchronized msgs slices in TestScheduler_Basic, ParallelMonitor, VisibilityTimeout, CustomID, and Cancel raced with test-side reads. These races predate this branch (reproduced on main) but fail the package under -race. Reuse the msgLog helper everywhere. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Test cleanups cancelled the monitor context and called Shutdown without waiting for the Monitor goroutine to exit. Shutdown breaks the Redis client mid-poll, so the still-running monitor logs a receive-error Warn through the zaptest logger after the test has finished — flagged by the race detector as a write to testing.T past completion. Add a startMonitor helper that returns a wait function and apply the cancel → wait → Shutdown ordering at every monitor spawn site. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Capping the configured backoff at the shortest retry delay could silently override an explicit user value downward (retry_schedule [5, ...] + explicit 10000ms → forced to 5s, doubling the intended idle Redis cost). Replace the cap with a sentinel: default 0 means auto — min(30s, shortest configured retry delay), so retries are never late — while an explicit positive value is honored as-is as a fixed maximum idle sleep. Validation now rejects retry_schedule entries < 1, retry_interval_seconds < 1 when no schedule is set, and negative retry_poll_backoff_ms at startup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
alexbouchardd
approved these changes
Aug 10, 2026
This was referenced Aug 13, 2026
alexluong
added a commit
that referenced
this pull request
Aug 13, 2026
…ue (#1040) * docs(config): document RETRY_POLL_BACKOFF_MS and log its resolved value The variable was redefined in #1026 — a fixed poll interval defaulting to 100 became a maximum idle sleep defaulting to 0 (auto) — but it appears in neither the configuration reference nor the startup log, so an operator has no way to see what the monitor is actually using. The logged value is the resolved one, since 0 means the cap is derived from the configured retry delays. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(config): drop the group comment added with retry_poll_backoff_ms The neighboring entries are bare group labels, and the GetRetryPollBackoff call on the line already shows the value is resolved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements #1014. Makes the rsmq
receiveMessagescript self-contained — it readsvtfrom the:Qhash and callsTIMEitself, dropping theMULTI/HMGET/TIME/EXECthat preceded every receive (5 commands / 2 RTTs → 1 / 1). When nothing is due the script also returns the time until the zset's earliest score, so the monitor sleeps until the next message comes due instead of every 100ms. At the new default that's 129.6M → 86.4K commands per month, per monitor.RETRY_POLL_BACKOFF_MSkeeps its meaning — how long the monitor waits when idle — and is redefined as the maximum idle sleep, default100→0, a sentinel meaning auto. Auto resolves tomin(30s, shortest configured retry delay)(GetRetryPollBackoff), which makes added retry latency zero by construction for every config, including a customretry_schedulewith sub-30s entries. An explicit positive value is honored as-is — no cap — so a user can knowingly trade retry latency for idle cost (an earlier revision clamped explicit values to the shortest retry delay, which could silently lower a deliberately large value and raise the user's Redis cost). For any fixed value the new sleep ismin(nextDue − now, X)≤ the old flatX, so existing values only improve. Startup validation now rejectsretry_scheduleentries < 1,retry_interval_seconds< 1 (when no schedule is set), and negativeretry_poll_backoff_ms.Heads-up — the consecutive-error ladder no longer derives from
pollBackoff: it now uses an internal 100ms base, so the ~1 minute of transient-infra tolerance documented atscheduler.goholds however the idle interval is configured. Previously raising the poll interval silently stretched (and lowering it shrank) that window.Behavior changes (release notes)
RETRY_POLL_BACKOFF_MSdefault100→0(auto). Auto sleeps until the next due message, at mostmin(30s, shortest configured retry delay). Retry timing is unchanged by construction; the idle Redis command rate drops ~600× (10 polls/s × 5 commands → ~1 command per interval).retry_scheduleentries < 1,retry_interval_seconds< 1 (when no schedule is set), or a negativeretry_poll_backoff_msnow fail at startup instead of being silently accepted.🤖 Generated with Claude Code