From 143505b5278dff82c5b3d2387ffaa56869e31018 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 15 Jun 2023 09:58:42 +0200 Subject: [PATCH 1/3] Fail lesss often when computing random port --- tools/test-framework/src/util/random.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tools/test-framework/src/util/random.rs b/tools/test-framework/src/util/random.rs index 4f8e8f3645..13d4200bd2 100644 --- a/tools/test-framework/src/util/random.rs +++ b/tools/test-framework/src/util/random.rs @@ -48,9 +48,7 @@ pub fn random_string() -> String { /// Generates a random non-privileged port that is greater than 1024. fn random_port() -> u16 { let mut rng = rand::thread_rng(); - rng.gen::() - .checked_add(1024) - .unwrap_or_else(random_port) + rng.gen_range(1024..=u16::MAX) } /// Find a random unused non-privileged TCP port. From b9183ec07e82033713ecac4b3afc55dfc6a636e5 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 15 Jun 2023 10:43:57 +0200 Subject: [PATCH 2/3] Assign some ports via the OS instead of randomly --- tools/test-framework/src/bootstrap/single.rs | 1 + tools/test-framework/src/chain/builder.rs | 9 +++++++-- tools/test-framework/src/chain/config.rs | 13 +++++++++++++ tools/test-framework/src/chain/driver.rs | 7 +++++++ 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/tools/test-framework/src/bootstrap/single.rs b/tools/test-framework/src/bootstrap/single.rs index 71cc299603..2e341b9de2 100644 --- a/tools/test-framework/src/bootstrap/single.rs +++ b/tools/test-framework/src/bootstrap/single.rs @@ -97,6 +97,7 @@ pub fn bootstrap_single_node( config::set_log_level(config, &log_level)?; config::set_rpc_port(config, chain_driver.rpc_port)?; config::set_p2p_port(config, chain_driver.p2p_port)?; + config::set_pprof_port(config, chain_driver.pprof_port)?; config::set_timeout_commit(config, Duration::from_secs(1))?; config::set_timeout_propose(config, Duration::from_secs(1))?; config::set_mode(config, "validator")?; diff --git a/tools/test-framework/src/chain/builder.rs b/tools/test-framework/src/chain/builder.rs index f7a0de904e..913143ed0e 100644 --- a/tools/test-framework/src/chain/builder.rs +++ b/tools/test-framework/src/chain/builder.rs @@ -103,8 +103,12 @@ impl ChainBuilder { let rpc_port = random_unused_tcp_port(); let grpc_port = random_unused_tcp_port(); - let grpc_web_port = random_unused_tcp_port(); - let p2p_port = random_unused_tcp_port(); + + // We let the OS choose a random port for these, since we do not need to know them before + // starting the chain, nor do we use them for anything afterwards. + let grpc_web_port = 0; + let p2p_port = 0; + let pprof_port = 0; let home_path = format!("{}/{}", self.base_store_dir, chain_id); @@ -118,6 +122,7 @@ impl ChainBuilder { grpc_port, grpc_web_port, p2p_port, + pprof_port, self.runtime.clone(), )?; diff --git a/tools/test-framework/src/chain/config.rs b/tools/test-framework/src/chain/config.rs index 1ba134e1cf..f0b6a8e385 100644 --- a/tools/test-framework/src/chain/config.rs +++ b/tools/test-framework/src/chain/config.rs @@ -67,6 +67,19 @@ pub fn set_p2p_port(config: &mut Value, port: u16) -> Result<(), Error> { Ok(()) } +/// Set the `pprof_laddr` field in the full node config. +pub fn set_pprof_port(config: &mut Value, port: u16) -> Result<(), Error> { + config + .as_table_mut() + .ok_or_else(|| eyre!("expect object"))? + .insert( + "pprof_laddr".to_string(), + format!("tcp://0.0.0.0:{port}").into(), + ); + + Ok(()) +} + pub fn set_mempool_version(config: &mut Value, version: &str) -> Result<(), Error> { config .get_mut("mempool") diff --git a/tools/test-framework/src/chain/driver.rs b/tools/test-framework/src/chain/driver.rs index f95929349a..f853ad10b9 100644 --- a/tools/test-framework/src/chain/driver.rs +++ b/tools/test-framework/src/chain/driver.rs @@ -87,6 +87,11 @@ pub struct ChainDriver { */ pub p2p_port: u16, + /** + The port used for pprof. (Currently unused other than for setup) + */ + pub pprof_port: u16, + pub tx_config: TxConfig, pub runtime: Arc, @@ -113,6 +118,7 @@ impl ChainDriver { grpc_port: u16, grpc_web_port: u16, p2p_port: u16, + pprof_port: u16, runtime: Arc, ) -> Result { let tx_config = new_tx_config_for_test( @@ -132,6 +138,7 @@ impl ChainDriver { grpc_port, grpc_web_port, p2p_port, + pprof_port, tx_config, runtime, }) From c6b3f9e30f4051c88e1d164bd8d1a13264d04d10 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 15 Jun 2023 10:28:08 +0200 Subject: [PATCH 3/3] Ensure we don't allocate the same port twice --- Cargo.lock | 5 +++-- tools/test-framework/Cargo.toml | 1 + tools/test-framework/src/chain/builder.rs | 9 +++------ tools/test-framework/src/util/random.rs | 13 ++++++++++--- 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 464f6da00e..2e3d103095 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1690,6 +1690,7 @@ dependencies = [ "ibc-relayer-cli", "ibc-relayer-types", "itertools", + "once_cell", "prost", "rand 0.8.5", "semver", @@ -2060,9 +2061,9 @@ dependencies = [ [[package]] name = "once_cell" -version = "1.17.2" +version = "1.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9670a07f94779e00908f3e686eab508878ebb390ba6e604d3a284c00e8d0487b" +checksum = "dd8b5dd2ae5ed71462c540258bedcb51965123ad7e7ccf4b9a8cafaa4a63576d" [[package]] name = "oneline-eyre" diff --git a/tools/test-framework/Cargo.toml b/tools/test-framework/Cargo.toml index acb2c3c551..47f82a51d6 100644 --- a/tools/test-framework/Cargo.toml +++ b/tools/test-framework/Cargo.toml @@ -41,3 +41,4 @@ flex-error = "0.4.4" prost = { version = "0.11" } tonic = { version = "0.9", features = ["tls", "tls-roots"] } hdpath = "0.6.3" +once_cell = "1.18.0" diff --git a/tools/test-framework/src/chain/builder.rs b/tools/test-framework/src/chain/builder.rs index 913143ed0e..d78fe390ac 100644 --- a/tools/test-framework/src/chain/builder.rs +++ b/tools/test-framework/src/chain/builder.rs @@ -103,12 +103,9 @@ impl ChainBuilder { let rpc_port = random_unused_tcp_port(); let grpc_port = random_unused_tcp_port(); - - // We let the OS choose a random port for these, since we do not need to know them before - // starting the chain, nor do we use them for anything afterwards. - let grpc_web_port = 0; - let p2p_port = 0; - let pprof_port = 0; + let grpc_web_port = random_unused_tcp_port(); + let p2p_port = random_unused_tcp_port(); + let pprof_port = random_unused_tcp_port(); let home_path = format!("{}/{}", self.base_store_dir, chain_id); diff --git a/tools/test-framework/src/util/random.rs b/tools/test-framework/src/util/random.rs index 13d4200bd2..8f7d299d25 100644 --- a/tools/test-framework/src/util/random.rs +++ b/tools/test-framework/src/util/random.rs @@ -3,8 +3,13 @@ */ use ibc_relayer_types::applications::transfer::amount::Amount; +use once_cell::sync::Lazy; use rand::Rng; -use std::net::{Ipv4Addr, SocketAddrV4, TcpListener}; +use std::{ + collections::HashSet, + net::{Ipv4Addr, SocketAddrV4, TcpListener}, + sync::Mutex, +}; /// Generates a random `u32` value. pub fn random_u32() -> u32 { @@ -53,11 +58,13 @@ fn random_port() -> u16 { /// Find a random unused non-privileged TCP port. pub fn random_unused_tcp_port() -> u16 { + static ALLOCATED_PORTS: Lazy>> = Lazy::new(|| Mutex::new(HashSet::new())); + let port = random_port(); let loopback = Ipv4Addr::new(127, 0, 0, 1); let address = SocketAddrV4::new(loopback, port); match TcpListener::bind(address) { - Ok(_) => port, - Err(_) => random_unused_tcp_port(), + Ok(_) if ALLOCATED_PORTS.lock().unwrap().insert(port) => port, + _ => random_unused_tcp_port(), } }