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

Use the default retry predicate in transport #1502

Merged
merged 1 commit into from
Dec 6, 2022

Commits on Dec 6, 2022

  1. Use the default retry predicate in transport

    We have two levels of retries here.
    
    The first is in the retryTransport, which currently just tests
    IsTemporary. This has a short backoff and was intended to handle blips
    in the network.
    
    The second level was intended for higher level issues, e.g. if an upload
    PUT failed, we have to retry more than just a single HTTP request,
    because we might have to restart the upload with a new POST and PATCH.
    
    This change propagates the default retryable predicate (from the scond
    level) down to the retryTransport. This also adds a list of HTTP codes
    that are retryable, so that the retryTransport can also handle
    server-side errors that should be retried.
    
    Note that this doesn't respect remote.WithRetry{Backoff,Predicate}
    because callers can just override the entire transport via
    remote.WithTransport. This change tries to do the right thing by default
    without changing behavior for folks who seem to already know what
    they're doing.
    
    Originally, I started down the path of wrapping various remote methods
    in a retry.Retry function (similar to writes) and decided against it due
    to the size of the change and how difficult it would be to maintain.
    
    In order to reduce retry ampliciation, this adds a little context
    wrapper to signal to retryTransport not to retry (because the retry is
    happening at a higher level).
    
    This also reduces the number of steps in the default backoff for
    retryTransport because more kinds of things are being retried, and I
    want to reduce the blast radius if something is erroneously retried.
    jonjohnsonjr committed Dec 6, 2022
    Configuration menu
    Copy the full SHA
    71df22f View commit details
    Browse the repository at this point in the history