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

Potential for dynamic backoffs #63

Open
GeeWee opened this issue Aug 3, 2023 · 4 comments
Open

Potential for dynamic backoffs #63

GeeWee opened this issue Aug 3, 2023 · 4 comments

Comments

@GeeWee
Copy link

GeeWee commented Aug 3, 2023

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
Copy link
Owner

Xuanwo commented Aug 3, 2023

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

@GeeWee
Copy link
Author

GeeWee commented Mar 13, 2024

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
Copy link
Owner

Xuanwo commented Mar 13, 2024

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
Copy link
Author

GeeWee commented Mar 13, 2024

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.

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

2 participants