Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to inspect upcoming sleep in stop funcs, and add stop_before_delay #423

Merged
merged 8 commits into from
Dec 18, 2023

Conversation

christek91
Copy link
Contributor

@christek91 christek91 commented Nov 9, 2023

This seeks to solve a specific problem: if using a wait function like wait_exponential or wait_random_exponential, along with stop_after_delay, the overall time spent retrying is >= max_delay.

However, in situations where you have a strict deadline and you can't block for longer than max_delay, it may be preferable to abort 1 retry attempt early so that you don't exceed that deadline.

This PR seeks to achieve that behavior by implementing stop_before_delay, which stops retries if the next attempt would take place after the max_delay time has elapsed, thereby ensuring that max_delay is never surpassed.

Examples of problem using exponential delays, and a 5 second deadline w/ stop_after_delay:

In [5]: @retry(stop=stop_after_delay(5), wait=wait_exponential())
   ...: def stop_after_w_e():
   ...:     print(datetime.now())
   ...:     raise Exception

In [11]: stop_after_w_e()
2023-11-08 23:18:12.259584
2023-11-08 23:18:13.261515
2023-11-08 23:18:15.262064
2023-11-08 23:18:19.263860
---------------------------------------------------------------------------
Exception                                 Traceback (most recent call last)

Note how the execution timestamp is 7 seconds after the initial one, despite the 5 second max_delay.

In [6]: @retry(stop=stop_after_delay(5), wait=wait_random_exponential())
   ...: def stop_after_w_r_e():
   ...:     print(datetime.now())
   ...:     raise Exception
   ...:

In [12]: stop_after_w_r_e()
2023-11-08 23:19:02.682304
2023-11-08 23:19:02.991432
2023-11-08 23:19:04.510066
2023-11-08 23:19:07.398501
2023-11-08 23:19:14.172982
---------------------------------------------------------------------------
Exception                                 Traceback (most recent call last)

Note the same thing w/ random exponential backoff. In this case the final attempt is made 11.49 seconds after the first one, despite a 5 second max_delay. That's over 2x the amount of time spent blocking on retries than our strict deadline allows!

This situation with random exponential backoff is the one that I'm interested in improving upon for my own use case (avoiding thundering herds during connection retries from many parallel clients). This extra time spent waiting after max_delay can be quite long with higher amounts exponential backoff. eg. a supposedly 30s max_delay that actually blocks for over 1 minute can cause some confusion.

With stop_before, the max_delay is never exceeded:

In [12]: @retry(stop=stop_before_delay(5), wait=wait_exponential())
    ...: def stop_before_w_e():
    ...:     print(datetime.now())
    ...:     raise Exception

In [13]: stop_before_w_e()
2023-11-08 23:21:56.437116
2023-11-08 23:21:57.439288
2023-11-08 23:21:59.441351
---------------------------------------------------------------------------
Exception                                 Traceback (most recent call last)

Total elapsed time is < 5 seconds (3 seconds).

In [14]: @retry(stop=stop_before_delay(5), wait=wait_random_exponential())
    ...: def stop_before_w_r_e():
    ...:     print(datetime.now())
    ...:     raise Exception

In [15]: stop_before_w_r_e()
2023-11-08 23:23:13.550395
2023-11-08 23:23:14.344831
2023-11-08 23:23:15.533050
2023-11-08 23:23:17.907447
---------------------------------------------------------------------------
Exception                                 Traceback (most recent call last)

Total elapsed time is < 5 seconds (4.35).

jd
jd previously approved these changes Nov 9, 2023
@mergify mergify bot dismissed jd’s stale review November 9, 2023 14:02

Pull request has been modified.

@mergify mergify bot merged commit 99e7482 into jd:main Dec 18, 2023
11 checks passed
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.

2 participants