fix(utils): never sleep longer than interval_max in retry_over_time - #2588
fix(utils): never sleep longer than interval_max in retry_over_time#2588abhijeet117 wants to merge 8 commits into
Conversation
retry_over_time builds its interval range with fxrange(interval_start, interval_max + interval_start, interval_step, repeatlast=True), so the steady-state value repeats above interval_max by construction (with the default 2/2/30 it reaches 32 seconds). Clamp the computed sleep time at interval_max to honor the documented contract. Fixes celery#914
There was a problem hiding this comment.
Pull request overview
This PR fixes kombu.utils.functional.retry_over_time() so its computed sleep time never exceeds the interval_max argument, aligning runtime behavior with the documented contract and addressing the overflow described in #914.
Changes:
- Clamp the computed “time to sleep” (
tts) tointerval_maxbefore sleeping. - Add a unit test that records total slept time per retry attempt and asserts it never exceeds
interval_max.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
kombu/utils/functional.py |
Clamp tts to interval_max to prevent fxrange-generated overflow beyond the configured maximum. |
t/unit/utils/test_functional.py |
Add regression test ensuring per-retry sleep never exceeds interval_max when retrying. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
auvipy
left a comment
auvipy
left a comment
There was a problem hiding this comment.
is it possible to add integration test for this as well?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2588 +/- ##
=======================================
Coverage 83.17% 83.17%
=======================================
Files 79 79
Lines 10636 10637 +1
Branches 1243 1243
=======================================
+ Hits 8846 8847 +1
Misses 1584 1584
Partials 206 206 ☔ View full report in Codecov by Harness. |
| # the last value of fxrange can be above interval_max, | ||
| # as its stop argument is inflated by interval_start. | ||
| tts = min(tts, float(interval_max)) |
…2465) * Add missing retry to publisher.publish in gcpubsub _put The publish call in _put was missing a retry parameter, unlike _put_fanout which already had one. This could cause transient failures to propagate immediately instead of being retried. * Add unit tests for retry parameter in gcpubsub _put --------- Co-authored-by: Asif Saif Uddin {"Auvi":"অভি"} <auvipy@gmail.com>
from_dict passed a hardcoded subset of kwargs to Queue, so alias, no_declare, expires, message_ttl, max_length, max_length_bytes and max_priority were silently lost on a round-trip through as_dict. Forward every attribute declared in Queue.attrs instead, so options added later are preserved automatically. Closes celery#1236. Co-authored-by: VXNCXNX <VXNCXNX@users.noreply.github.com> Co-authored-by: Asif Saif Uddin {"Auvi":"অভি"} <auvipy@gmail.com>
TopicExchange documents '*' as any single word, but compiled it to
'.*?[^\.]', whose leading '.*?' also matches dots. Only the final
character was constrained, so 'a.*.c' matched 'a.b.x.c' and every
virtual transport delivered messages to queues that never bound them.
Compile '*' to a single dot-free word instead. '#' is unchanged.
Co-authored-by: Asif Saif Uddin {"Auvi":"অভি"} <auvipy@gmail.com>
|
@auvipy Added the interval_max regression coverage and validated the errback-observed interval is clamped before sleeping. |
|
thanks for additional unit tests. but I was talking about addtional integration tests in another file. |
Summary
retry_over_time()could sleep longer thaninterval_maxbecause its backoff range is built asfxrange(interval_start, interval_max + interval_start, interval_step, repeatlast=True), andfxrangeyields every value up to and including its stop before repeating the last one forever. The steady-state sleep therefore exceededinterval_maxby construction (defaults 2/2/30 reached 32s). This clamps the computed sleep atinterval_maxbefore sleeping, matching the documented contract. Fixes #914.Testing
test_never_sleeps_longer_than_interval_max, which captures per-retry sleep totals with a patchedsleepand asserts they never exceedinterval_max. It failed before the fix (max sleep 7.0 > 6) and passes after (max exactly 6.0).t/unitsuite run after the fix matches the pristine baseline.Checklist