tower icon indicating copy to clipboard operation
tower copied to clipboard

Retry middleware improvements

Open LucioFranco opened this issue 3 years ago • 4 comments

This issue scopes out the changes we are proposing to the retry middleware to improve its ergonomics. Currently, the retry middleware is quite hard to use and requires implementing a custom Policy. Writing this policy is non-trivial and is more work than it should be.

These improvements are aimed at setting up retries with tower to be easier and more user friendly. As well as providing good defaults that work out of the box.

List of improvements to tower and tower-http:

  • [x] Simplify Policy (https://github.com/tower-rs/tower/pull/681).
    • Change trait fn to take &mut self.
    • Change Future output to ().
    • Alllow Policy to be object safe.
  • [x] Add generic backoff utilities. https://github.com/tower-rs/tower/pull/685
    • [x] Add some trait Backoff that has an associated future type that allows others to use this utility without being tied to tokio::time.
    • [x] Add a ExponentialBackoff type that implements Backoff, ported from linkerd2-proxy.
    • [x] Add Rng utilities https://github.com/tower-rs/tower/pull/686
  • [ ] Budget improvements.
    • [x] Merge budget docs PR (https://github.com/tower-rs/tower/pull/613).
    • [ ] Implement simpler non-time based token bucket budget.
    • [ ] Add some Budget trait to allow users to choose which implementation to use.
  • [ ] Add a new batteries included standard retry policy https://github.com/tower-rs/tower/pull/698
    • [ ] New StandardRetryPolicy combining impl Backoff and impl Budget.
    • [ ] Add StandardRetryPolicyBuilder that accept closures (?) for is_retryable(&mut Req, &mut Result<Res, E>) -> bool and a clone_request(&Req) -> Option<Req>.
  • [ ] tower-http improvements.
    • [ ] Add new retry module
    • [ ] Implement ReplayBody similar to the one implemented in linkerd2-proxy.
    • [ ] Add new HttpRetry layer that accepts higher level constructs for retrying, like ClassifyResponse, and will wrap http request bodies with ReplayBody.
  • [ ] Documentation
    • [ ] Blog post on how to setup retries with tower and tower-http.
    • [ ] Examples for thick clients with retries in both tower and tower-http.

Example code

tower examples with no http:

let policy = StandardRetryPolicy::builder()
    .should_retry(|res| match res {
        Ok(_) => false,
        Err(_) => true,
    })
    .clone_request(|r| Some(*r))
    .build();

let mut svc = ServiceBuilder::new()
    .retry(policy)
    .buffer(10)
    .timeout(Duration::from_secs(10))
    .service(svc);

tower-http examples:

let make_classifier = ServerErrorsAsFailures::make_classifier();

let mut svc = ServiceBuilder::new()
    .set_request_id("Request-Id".try_into().unwrap(), MakeRequestUuid)
    .retry(StandardHttpPolicy::new(
        make_classifier,
        ExponentialBackoff::default(),
        Budget::default(),
        |e| match e {
            ServerErrorsFailureClass::Status(s) => true,
            ServerErrorsFailureClass::Error(s) => false,
        },
    ))
    .timeout(Duration::from_secs(5))
    .service(client);

cc @rcoh @hawkw @jonhoo @seanmonstar @davidpdrsn

LucioFranco avatar Aug 11 '22 15:08 LucioFranco

It would be cool if this could integrate with 429/Retry-After header out-of-the-box.

lovesegfault avatar Aug 11 '22 16:08 lovesegfault

@lovesegfault yeah, good idea, this seems like a config option that could live in HttpRetry.

LucioFranco avatar Aug 11 '22 16:08 LucioFranco

+1 to lovesegfault, I found this crate will looking for a way to handle Retry-After easily

DontBreakAlex avatar Sep 12 '22 21:09 DontBreakAlex

so this issue delay ?

xj524598 avatar Dec 13 '23 12:12 xj524598