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

ExponentialRetry timeout is off by a factor of factor #91

Open
shrekris-anyscale opened this issue Apr 5, 2023 · 0 comments
Open

ExponentialRetry timeout is off by a factor of factor #91

shrekris-anyscale opened this issue Apr 5, 2023 · 0 comments

Comments

@shrekris-anyscale
Copy link

shrekris-anyscale commented Apr 5, 2023

Background and Expected Behavior

ExponentialRetry takes in the following arguments:

class ExponentialRetry(RetryOptionsBase):
    def __init__(
        self,
        attempts: int = 3,  # How many times we should retry
        start_timeout: float = 0.1,  # Base timeout time, then it exponentially grow
        max_timeout: float = 30.0,  # Max possible timeout between tries
        factor: float = 2.0,  # How much we increase timeout each time
        statuses: Optional[Set[int]] = None,  # On which statuses we should retry
        exceptions: Optional[Set[Type[Exception]]] = None,  # On which exceptions we should retry
        retry_all_server_errors: bool = True,
        evaluate_response_callback: Optional[EvaluateResponseCallbackType] = None,
    ):

Based on the comments, I expect the first retry to happen start_timeout seconds after the original try. I expect the second retry to happen start_timeout * factor seconds after the first retry, and so on.

Observed Behavior

Instead, the first retry happens start_timeout * factor seconds after the original try. The second retry happens start_timeout * factor**2 seconds after that and so on. All the retries seem to be off by a factor of factor.

I confirmed this by logging requests I sent to a local server. I used this RetryClient:

RetryClient(retry_options=ExponentialRetry(attempts=1, start_timeout=1))

and I observed the following timings (note that the first retry happens 2 seconds after the first attempt– not 1 second):

2023-04-04 20:28:00,317 http_proxy 10.0.0.5 http_proxy.py:361 - Got request
2023-04-04 20:28:02,326 http_proxy 10.0.0.5 http_proxy.py:361 - Got request
2023-04-04 20:28:06,334 http_proxy 10.0.0.5 http_proxy.py:361 - Got request
2023-04-04 20:28:14,340 http_proxy 10.0.0.5 http_proxy.py:361 - Got request
2023-04-04 20:28:30,348 http_proxy 10.0.0.5 http_proxy.py:361 - Got request

Potential Cause

I believe the issue is that _RequestContext 1-indexes each attempt, but the math in ExponentialRetry expects 0-indexing.

@shrekris-anyscale shrekris-anyscale changed the title ExponentialRetry timeout if off by one factor ExponentialRetry timeout is off by a factor of factor Apr 5, 2023
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

No branches or pull requests

1 participant