From 398598aed24d03c9c023bc2b88bb891bfa2e55fe Mon Sep 17 00:00:00 2001 From: Jason Carver Date: Wed, 13 Sep 2023 07:10:30 -0700 Subject: [PATCH] fix: better utp connection rates w/ default config We were seeing failed connection attempts fairly regularly on the testnet. This is one step of resolving that issue. utp-rs is mostly tested using the default configurations, which have changed a few times to improve results. As far as I can tell, the configuration choices here are just an old copy of the config. So instead of copying in new values, I switched the config to use defaults whenever possible. (which requires using lazy_static) The only custom configuration that seems to matter is that discv5 enforces a custom maximum packet size. Some reasons I think this will help: - Number of connection attempts doubles from 3 -> 6 - Idle connection timeout ~doubles from 32->60 - Maximum packet timeout is cut in a quarter from 60->15, for more retries before triggering an idle timeout. (This will affect mid-stream timeouts, but won't affect the connection timeouts we're seeing) --- Cargo.lock | 1 + portalnet/Cargo.toml | 1 + portalnet/src/overlay.rs | 2 +- portalnet/src/overlay_service.rs | 23 +++++++++-------------- 4 files changed, 12 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2f54678f3..06ce58104 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4011,6 +4011,7 @@ dependencies = [ "ethportal-api", "fnv", "futures 0.3.28", + "lazy_static", "leb128", "lru", "parking_lot 0.11.2", diff --git a/portalnet/Cargo.toml b/portalnet/Cargo.toml index 5ae28a338..f31676c61 100644 --- a/portalnet/Cargo.toml +++ b/portalnet/Cargo.toml @@ -25,6 +25,7 @@ ethereum-types = "0.12.1" ethportal-api = { path="../ethportal-api" } fnv = "1.0.7" futures = "0.3.21" +lazy_static = "1.4.0" leb128 = "0.2.1" lru = "0.7.8" parking_lot = "0.11.2" diff --git a/portalnet/src/overlay.rs b/portalnet/src/overlay.rs index c1b5d1a9a..fc6f9c1a9 100644 --- a/portalnet/src/overlay.rs +++ b/portalnet/src/overlay.rs @@ -496,7 +496,7 @@ where }; let mut stream = self .utp_socket - .connect_with_cid(cid, UTP_CONN_CFG) + .connect_with_cid(cid, *UTP_CONN_CFG) .await .map_err(|err| OverlayRequestError::UtpError(format!("{err:?}")))?; let mut data = vec![]; diff --git a/portalnet/src/overlay_service.rs b/portalnet/src/overlay_service.rs index 928e5bb0b..ad4db50d7 100644 --- a/portalnet/src/overlay_service.rs +++ b/portalnet/src/overlay_service.rs @@ -20,6 +20,7 @@ use discv5::{ rpc::RequestId, }; use futures::{channel::oneshot, future::join_all, prelude::*}; +use lazy_static::lazy_static; use parking_lot::RwLock; use rand::seq::{IteratorRandom, SliceRandom}; use smallvec::SmallVec; @@ -82,16 +83,10 @@ const EXPECTED_NON_EMPTY_BUCKETS: usize = 17; /// Bucket refresh lookup interval in seconds const BUCKET_REFRESH_INTERVAL_SECS: u64 = 60; -/// The default configuration to use for uTP connections. -pub const UTP_CONN_CFG: ConnectionConfig = ConnectionConfig { - max_packet_size: 1024, - max_conn_attempts: 3, - max_idle_timeout: Duration::from_secs(32), - max_timeout: Duration::from_secs(60), - initial_timeout: Duration::from_millis(1500), - min_timeout: Duration::from_millis(500), - target_delay: Duration::from_millis(250), -}; +lazy_static! { + /// The default configuration to use for uTP connections. + pub static ref UTP_CONN_CFG: ConnectionConfig = ConnectionConfig { max_packet_size: 1024, ..Default::default()}; +} /// A network-based action that the overlay may perform. /// @@ -836,7 +831,7 @@ where tokio::spawn(async move { metrics.report_utp_active_inc(UtpDirectionLabel::Inbound); let mut stream = match utp - .connect_with_cid(cid.clone(), UTP_CONN_CFG) + .connect_with_cid(cid.clone(), *UTP_CONN_CFG) .await { Ok(stream) => stream, @@ -1109,7 +1104,7 @@ where let metrics = Arc::clone(&self.metrics); tokio::spawn(async move { metrics.report_utp_active_inc(UtpDirectionLabel::Outbound); - let stream = match utp.accept_with_cid(cid.clone(), UTP_CONN_CFG).await { + let stream = match utp.accept_with_cid(cid.clone(), *UTP_CONN_CFG).await { Ok(stream) => stream, Err(err) => { metrics.report_utp_outcome( @@ -1237,7 +1232,7 @@ where // Wait for an incoming connection with the given CID. Then, read the data from the uTP // stream. metrics.report_utp_active_inc(UtpDirectionLabel::Inbound); - let mut stream = match utp.accept_with_cid(cid.clone(), UTP_CONN_CFG).await { + let mut stream = match utp.accept_with_cid(cid.clone(), *UTP_CONN_CFG).await { Ok(stream) => stream, Err(err) => { metrics.report_utp_outcome( @@ -1500,7 +1495,7 @@ where tokio::spawn(async move { metrics.report_utp_active_inc(UtpDirectionLabel::Outbound); - let stream = match utp.connect_with_cid(cid.clone(), UTP_CONN_CFG).await { + let stream = match utp.connect_with_cid(cid.clone(), *UTP_CONN_CFG).await { Ok(stream) => stream, Err(err) => { metrics.report_utp_outcome(