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

retry: Add generic backoff utilities #685

Merged
merged 10 commits into from
Aug 30, 2022
Merged

retry: Add generic backoff utilities #685

merged 10 commits into from
Aug 30, 2022

Conversation

LucioFranco
Copy link
Member

This adds a new Backoff trait and a ExponentialBackoff
implementation borrwoed from linkerd2-proxy. This provides the initial
building blocks for a more fully batteries included retry policy.

cc @rcoh

This adds a new `Backoff` trait and a `ExponentialBackoff`
implementation borrwoed from `linkerd2-proxy`. This provides the initial
building blocks for a more fully batteries included retry policy.
@LucioFranco LucioFranco requested review from seanmonstar and a team August 19, 2022 18:22
tower/src/retry/backoff.rs Outdated Show resolved Hide resolved
tower/src/retry/backoff.rs Show resolved Hide resolved
tower/src/retry/backoff.rs Show resolved Hide resolved
Comment on lines 1 to 16
//! This module contains generic backoff utlities to be used with the retry
//! layer.
//!
//! The [`Backoff`] trait is a generic way to represent backoffs that can use
//! any timer type.
//!
//! [`ExponentialBackoff`] implements [`Backoff`] and provides a batteries
//! included exponential backoff and jitter strategy.

use rand::Rng;
use rand::{rngs::SmallRng, thread_rng, SeedableRng};
use std::fmt::Display;
use std::future::Future;
use std::time::Duration;
use tokio::time;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the doc comment either for the module or the Backoff trait should probably at least mention what the term "backoff" means, for readers who've never seen it before?

tower/src/retry/backoff.rs Show resolved Hide resolved
tower/src/retry/backoff.rs Outdated Show resolved Hide resolved
tower/src/retry/backoff.rs Outdated Show resolved Hide resolved
LucioFranco added a commit that referenced this pull request Aug 23, 2022
This adds new PRNG utilities that only use libstd and not the external
`rand` crate. This change's motivation are that in tower middleware that
need PRNG don't need the complexity and vast utilities of the `rand`
crate.

This adds a `Rng` trait which abstracts the simple PRNG features tower
needs. This also provides a `HasherRng` which uses the `RandomState`
type from libstd to generate random `u64` values. In addition, there is
an internal only `sample_inplace` which is used within the balance p2c
middleware to randomly pick a ready service. This implementation is
crate private since its quite specific to the balance implementation.

The goal of this in addition to the balance middlware getting `rand`
removed is for the upcoming `Retry` changes. The `next_f64` will be used
in the jitter portion of the backoff utilities in #685.
LucioFranco added a commit that referenced this pull request Aug 25, 2022
This adds new PRNG utilities that only use libstd and not the external
`rand` crate. This change's motivation are that in tower middleware that
need PRNG don't need the complexity and vast utilities of the `rand`
crate.

This adds a `Rng` trait which abstracts the simple PRNG features tower
needs. This also provides a `HasherRng` which uses the `RandomState`
type from libstd to generate random `u64` values. In addition, there is
an internal only `sample_inplace` which is used within the balance p2c
middleware to randomly pick a ready service. This implementation is
crate private since its quite specific to the balance implementation.

The goal of this in addition to the balance middlware getting `rand`
removed is for the upcoming `Retry` changes. The `next_f64` will be used
in the jitter portion of the backoff utilities in #685.

Co-authored-by: Eliza Weisman <[email protected]>
@LucioFranco LucioFranco mentioned this pull request Aug 25, 2022
19 tasks
@LucioFranco
Copy link
Member Author

I think this is ready for another round of reviews!

Copy link
Collaborator

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good!

tower/src/retry/backoff.rs Outdated Show resolved Hide resolved
tower/src/retry/backoff.rs Show resolved Hide resolved
@LucioFranco LucioFranco merged commit b12f148 into master Aug 30, 2022
@LucioFranco LucioFranco deleted the lucio/backoff branch August 30, 2022 17:59
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.

3 participants