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

Clarify docstring for Retry backoff factor #3037

Merged
merged 2 commits into from
May 30, 2023
Merged

Conversation

fhightower
Copy link
Contributor

@fhightower fhightower commented May 18, 2023

Proposed Changes

This PR makes two changes to the docs for Retry's backoff_factor:

  1. Change "number of total retries" to "number of previous retries"
    • The current description ("number of total retries") could be misunderstood to refer to the total argument provided to a Retry instance
  2. Remove - 1 from the end of the formula
    • As written, I don't think this formula is correct (at least it is prone to misunderstanding)... consider a backoff factor of 1:
      • Since the first retry occurs immediately, let's consider how many seconds the current formula predicts between the second and third retries:
        • The time between the first and second retries (where the number of total retries is 1) is predicted to be: 1 * (2 ** (1 - 1)) or 1 second
        • The time between the second and third retries (where the number of total retries is 2) is predicted to be: 1 * (2 ** (2 - 1)) or 2 seconds
      • In actuality, however, the time between the first and second retries is 2 seconds with 4 seconds between the second and third retries
      • The table below summarizes my findings
Retry # Number of total retries (one of the variables in the current formula) Elapsed seconds predicted by current formula Actual elapsed seconds
1 0 n/a (occurs immediately) n/a (occurs immediately)
2 1 1 2
3 2 2 4
4 3 4 0

Validating this Change

To validate that the updated formula is correct, run this in one process:

python3 -m http.server

And this in another process:

from urllib3.util.retry import Retry
import requests
from requests.adapters import HTTPAdapter

session = requests.Session()
retry = Retry(
    total=3,
    backoff_factor=1,
    status_forcelist=[200],
)
adapter = HTTPAdapter(max_retries=retry)
session.mount('http://', adapter)
session.mount('https://', adapter)

session.get("http://localhost:8000")

This script will make requests to the webserver with a backoff factor of 1 and you can note the interval between each retry.

Conclusion

All in all, I think making these two changes will clarify how the backoff factor works and how long it will wait between retries.

Please let me know if you have any questions or suggestions!

Clarifying docs for the backoff_factor.
@illia-v illia-v added the Skip Changelog Pull requests that don't require a changelog entry label May 22, 2023
@illia-v illia-v enabled auto-merge (squash) May 30, 2023 14:03
@illia-v illia-v merged commit b63cc4c into urllib3:main May 30, 2023
30 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog Pull requests that don't require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants