Let other tasks run between retries, so we're more likely to choose d…

…ifferent peers
ZcashFoundation · Dec 17, 2021 · 0c5fce7 · 0c5fce7
1 parent 637de54
commit 0c5fce7
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 27 deletions.
diff --git a/zebra-network/src/lib.rs b/zebra-network/src/lib.rs
@@ -73,7 +73,7 @@ pub use crate::{
     meta_addr::PeerAddrState,
     peer::{HandshakeError, PeerError, SharedPeerError},
     peer_set::init,
-    policies::{RetryErrors, RetryLimit},
+    policies::RetryLimit,
     protocol::internal::{Request, Response},
 };
 

diff --git a/zebra-network/src/policies.rs b/zebra-network/src/policies.rs
@@ -1,4 +1,6 @@
-use futures::future;
+use std::pin::Pin;
+
+use futures::{Future, FutureExt};
 use tower::retry::Policy;
 
 /// A very basic retry policy with a limited number of retry attempts.
@@ -19,14 +21,26 @@ impl RetryLimit {
 }
 
 impl<Req: Clone + std::fmt::Debug, Res, E: std::fmt::Debug> Policy<Req, Res, E> for RetryLimit {
-    type Future = future::Ready<Self>;
+    type Future = Pin<Box<dyn Future<Output = Self> + Send + 'static>>;
+
     fn retry(&self, req: &Req, result: Result<&Res, &E>) -> Option<Self::Future> {
         if let Err(e) = result {
             if self.remaining_tries > 0 {
                 tracing::debug!(?req, ?e, remaining_tries = self.remaining_tries, "retrying");
-                Some(future::ready(RetryLimit {
-                    remaining_tries: self.remaining_tries - 1,
-                }))
+                let remaining_tries = self.remaining_tries - 1;
+
+                Some(
+                    async move {
+                        // Let other tasks run, so we're more likely to choose a different peer,
+                        // and so that any notfound inv entries win the race to the PeerSet.
+                        //
+                        // TODO: move syncer retries into the PeerSet,
+                        //       so we always choose different peers (#3235)
+                        tokio::task::yield_now().await;
+                        RetryLimit { remaining_tries }
+                    }
+                    .boxed(),
+                )
             } else {
                 None
             }
@@ -39,24 +53,3 @@ impl<Req: Clone + std::fmt::Debug, Res, E: std::fmt::Debug> Policy<Req, Res, E>
         Some(req.clone())
     }
 }
-
-/// A very basic retry policy that always retries failed requests.
-///
-/// XXX remove this when https://github.com/tower-rs/tower/pull/414 lands.
-#[derive(Clone, Debug)]
-pub struct RetryErrors;
-
-impl<Req: Clone, Res, E> Policy<Req, Res, E> for RetryErrors {
-    type Future = future::Ready<Self>;
-    fn retry(&self, _: &Req, result: Result<&Res, &E>) -> Option<Self::Future> {
-        if result.is_err() {
-            Some(future::ready(RetryErrors))
-        } else {
-            None
-        }
-    }
-
-    fn clone_request(&self, req: &Req) -> Option<Req> {
-        Some(req.clone())
-    }
-}