Xuanwo / backon

Make retry like a built-in feature provided by Rust.
https://docs.rs/backon/
Apache License 2.0
684 stars 25 forks source link

Potential for dynamic backoffs #63

Open GeeWee opened 1 year ago

GeeWee commented 1 year ago

I'm interesting in using backon for retrying HTTP requests. I'd like to handle the retry-after HTTP header, where the server essentially can hint at which point it'd like you to retry. Which means I'd like to adjust my backoff time depending on that value.

Currently I can't figure out how I'd accomplish this as Backoff is just an iterator over Durations. The trait would need to change to have a method implementation that e.g. took in the previous backoffs and the current error, so the error could be inspected for a retry time.

Is this a feasible change? Or perhaps this is already possible and I'm not understanding the API correctly?

Xuanwo commented 1 year ago

Seems an interesting case. Let me take a look if it possible.

GeeWee commented 6 months ago

Just stumbled upon this again. While you can make this work with backon via some hacky Arc-trickery, it's.. well not very pretty.

use backon::{BackoffBuilder, BlockingRetryable};
use rand::prelude::*;
use std::time::Duration;
#[cfg(test)]
mod tests {
    use super::*;
    use backon::{Backoff, BackoffBuilder, BlockingRetryable, ConstantBackoff, ConstantBuilder};
    use std::collections::VecDeque;
    use std::mem;
    use std::rc::Rc;
    use std::sync::{Arc, Mutex};
    use std::time::{Duration, Instant};
    use test_log::test;

    #[derive(Debug, Default, Clone)]
    pub struct DynamicBackoff {
        backoff_queue: Arc<Mutex<VecDeque<Duration>>>,
        total_retries: i32,
    }

    impl DynamicBackoff {
        pub fn set_next_backoff(&mut self, duration: Duration) {
            let mut mutable_queue = self.backoff_queue.lock().unwrap();

            match mutable_queue.pop_front() {
                None => {
                    println!("  Pushing new timeout to queue {duration:?}");
                    mutable_queue.push_front(duration)
                }
                Some(existing) => {
                    println!("  Replacing existing duration {existing:?} with {duration:?}");
                    mutable_queue.push_front(duration)
                }
            }

            self.total_retries += 1
        }
    }

    impl Iterator for DynamicBackoff {
        type Item = Duration;

        fn next(&mut self) -> Option<Self::Item> {
            let mut mutable_queue = self.backoff_queue.lock().unwrap();
            let res = mutable_queue.pop_front();
            res
        }
    }

    #[derive(Debug, Clone, Default)]
    pub struct DynamicBackoffBuilder {
        backoffs: DynamicBackoff,
    }

    impl DynamicBackoffBuilder {
        fn set_next_backoff(&mut self, d: Duration) {
            self.backoffs.set_next_backoff(d)
        }

        fn new(initial_backoffs: Vec<Duration>) -> Self {
            Self {
                backoffs: DynamicBackoff {
                    backoff_queue: Arc::new(Mutex::new(VecDeque::from(initial_backoffs))),
                    total_retries: 0,
                },
            }
        }
    }

    impl BackoffBuilder for DynamicBackoffBuilder {
        type Backoff = DynamicBackoff;

        fn build(&self) -> Self::Backoff {
            self.backoffs.clone()
        }
    }

    #[test]
    fn dynamic_backoff() {
        let method = || -> Result<&str, &str> {
            println!("Invoked");

            if rand::random() {
                Err("extra_time")
            } else {
                Err("regular time")
            }
        };

        let mut builder = DynamicBackoffBuilder::new(vec![
            Duration::from_millis(100),
            Duration::from_millis(300),
            Duration::from_millis(500),
            Duration::from_millis(800),
        ]);

        let duration = Instant::now();

        let result = method
            .retry(&builder)
            .notify(|e, d| {
                println!(
                    "Notify, {e:?}, {d:?} - total duration: {:?}",
                    duration.elapsed()
                );
            })
            .when(|e| {
                println!("error called: {}", e);
                if e == &"extra_time" {
                    // could also replace here
                    builder.set_next_backoff(Duration::from_millis(500));
                }
                true
            })
            .call();
    }
}
Xuanwo commented 6 months ago

I think we can maintain an Arc<Mutex<T>> in builder and backoff for updating them with the Retry-After returned in responses:

#[cfg(test)]
mod tests {
    use super::*;
    use crate::{BackoffBuilder, BlockingRetryable};
    use std::sync::{Arc, Mutex};
    use std::time::{Duration, Instant};

    #[derive(Debug, Default, Clone)]
    pub struct DynamicBackoff {
        retry_after: Arc<Mutex<Duration>>,
        retries: usize,
    }

    impl DynamicBackoff {
        fn new(retry_after: Arc<Mutex<Duration>>) -> Self {
            Self {
                retry_after,
                retries: 3,
            }
        }
    }

    impl Iterator for DynamicBackoff {
        type Item = Duration;

        fn next(&mut self) -> Option<Self::Item> {
            if self.retries == 0 {
                None
            } else {
                self.retries -= 1;
                Some(*self.retry_after.lock().unwrap())
            }
        }
    }

    #[derive(Debug, Clone, Default)]
    pub struct DynamicBackoffBuilder(pub Arc<Mutex<Duration>>);

    impl BackoffBuilder for DynamicBackoffBuilder {
        type Backoff = DynamicBackoff;

        fn build(&self) -> Self::Backoff {
            DynamicBackoff::new(self.0.clone())
        }
    }

    #[test]
    fn dynamic_backoff() {
        let method = || -> Result<&str, &str> {
            println!("Invoked");

            if rand::random() {
                Err("extra_time")
            } else {
                Err("regular time")
            }
        };

        let retry_after = Arc::new(Mutex::new(Duration::from_secs(1)));

        let builder = DynamicBackoffBuilder(retry_after.clone());

        let duration = Instant::now();

        let result = method
            .retry(&builder)
            .notify(|e, d| {
                println!(
                    "Notify, {e:?}, {d:?} - total duration: {:?}",
                    duration.elapsed()
                );
            })
            .when(|e| {
                println!("error called: {}", e);
                if e == &"extra_time" {
                    // We can inspect the `Retry-After` header here.
                    *retry_after.lock().unwrap() = Duration::from_millis(500);
                }
                true
            })
            .call();
    }
}
GeeWee commented 6 months ago

Yeah that's also what I'd end up doing I think if I had to use backon, albeit I'm not super crazy about having to mutate the builder inside the when method.