From 4c297500959368342a94e953f59e28de60195dd5 Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Wed, 3 Aug 2022 10:39:55 -0500 Subject: [PATCH] Enable QUIC client by default. Add arg to disable QUIC client. (#26879) --- banking-bench/src/main.rs | 6 +++--- bench-tps/src/cli.rs | 10 ++++----- client/src/connection_cache.rs | 30 ++++++++++++++++++++------- dos/src/cli.rs | 1 - multinode-demo/bootstrap-validator.sh | 2 +- validator/src/main.rs | 10 ++++++++- 6 files changed, 41 insertions(+), 18 deletions(-) diff --git a/banking-bench/src/main.rs b/banking-bench/src/main.rs index 51b0042abed374..2806a8a9e05a7a 100644 --- a/banking-bench/src/main.rs +++ b/banking-bench/src/main.rs @@ -214,10 +214,10 @@ fn main() { .help("Number of threads to use in the banking stage"), ) .arg( - Arg::new("tpu_use_quic") - .long("tpu-use-quic") + Arg::new("tpu_disable_quic") + .long("tpu-disable-quic") .takes_value(false) - .help("Forward messages to TPU using QUIC"), + .help("Disable forwarding messages to TPU using QUIC"), ) .get_matches(); diff --git a/bench-tps/src/cli.rs b/bench-tps/src/cli.rs index e9b4faa848a835..8c5c22ac09d17d 100644 --- a/bench-tps/src/cli.rs +++ b/bench-tps/src/cli.rs @@ -290,10 +290,10 @@ pub fn build_args<'a, 'b>(version: &'b str) -> App<'a, 'b> { .help("Submit transactions with a TpuClient") ) .arg( - Arg::with_name("tpu_use_quic") - .long("tpu-use-quic") + Arg::with_name("tpu_disable_quic") + .long("tpu-disable-quic") .takes_value(false) - .help("Submit transactions via QUIC; only affects ThinClient (default) \ + .help("Do not submit transactions via QUIC; only affects ThinClient (default) \ or TpuClient sends"), ) .arg( @@ -348,8 +348,8 @@ pub fn extract_args(matches: &ArgMatches) -> Config { args.external_client_type = ExternalClientType::RpcClient; } - if matches.is_present("tpu_use_quic") { - args.use_quic = true; + if matches.is_present("tpu_disable_quic") { + args.use_quic = false; } if let Some(v) = matches.value_of("tpu_connection_pool_size") { diff --git a/client/src/connection_cache.rs b/client/src/connection_cache.rs index f0628d3e32b9de..9e5efff3f3e061 100644 --- a/client/src/connection_cache.rs +++ b/client/src/connection_cache.rs @@ -32,7 +32,7 @@ static MAX_CONNECTIONS: usize = 1024; /// Used to decide whether the TPU and underlying connection cache should use /// QUIC connections. -pub const DEFAULT_TPU_USE_QUIC: bool = false; +pub const DEFAULT_TPU_USE_QUIC: bool = true; /// Default TPU connection pool size per remote address pub const DEFAULT_TPU_CONNECTION_POOL_SIZE: usize = 4; @@ -683,6 +683,11 @@ mod tests { // be lazy and not connect until first use or handle connection errors somehow // (without crashing, as would be required in a real practical validator) let connection_cache = ConnectionCache::default(); + let port_offset = if connection_cache.use_quic() { + QUIC_PORT_OFFSET + } else { + 0 + }; let addrs = (0..MAX_CONNECTIONS) .into_iter() .map(|_| { @@ -695,18 +700,29 @@ mod tests { let map = connection_cache.map.read().unwrap(); assert!(map.len() == MAX_CONNECTIONS); addrs.iter().for_each(|a| { - let conn = &map.get(a).expect("Address not found").connections[0]; - let conn = conn.new_blocking_connection(*a, connection_cache.stats.clone()); - assert!(a.ip() == conn.tpu_addr().ip()); + let port = a + .port() + .checked_add(port_offset) + .unwrap_or_else(|| a.port()); + let addr = &SocketAddr::new(a.ip(), port); + + let conn = &map.get(addr).expect("Address not found").connections[0]; + let conn = conn.new_blocking_connection(*addr, connection_cache.stats.clone()); + assert!(addr.ip() == conn.tpu_addr().ip()); }); } - let addr = get_addr(&mut rng); - connection_cache.get_connection(&addr); + let addr = &get_addr(&mut rng); + connection_cache.get_connection(addr); + let port = addr + .port() + .checked_add(port_offset) + .unwrap_or_else(|| addr.port()); + let addr_with_quic_port = SocketAddr::new(addr.ip(), port); let map = connection_cache.map.read().unwrap(); assert!(map.len() == MAX_CONNECTIONS); - let _conn = map.get(&addr).expect("Address not found"); + let _conn = map.get(&addr_with_quic_port).expect("Address not found"); } #[test] diff --git a/dos/src/cli.rs b/dos/src/cli.rs index 9e82c3f06e0c19..0fe3890af01285 100644 --- a/dos/src/cli.rs +++ b/dos/src/cli.rs @@ -248,7 +248,6 @@ mod tests { "--valid-signatures", "--num-signatures", "8", - "--tpu-use-quic", "--send-batch-size", "1", ]) diff --git a/multinode-demo/bootstrap-validator.sh b/multinode-demo/bootstrap-validator.sh index 9245f507c394e2..deb82f106fae04 100755 --- a/multinode-demo/bootstrap-validator.sh +++ b/multinode-demo/bootstrap-validator.sh @@ -61,7 +61,7 @@ while [[ -n $1 ]]; do elif [[ $1 = --enable-rpc-bigtable-ledger-storage ]]; then args+=("$1") shift - elif [[ $1 = --tpu-use-quic ]]; then + elif [[ $1 = --tpu-disable-quic ]]; then args+=("$1") shift elif [[ $1 = --rpc-send-batch-ms ]]; then diff --git a/validator/src/main.rs b/validator/src/main.rs index ce8c12c06fe3e7..8f11c8c692dcdc 100644 --- a/validator/src/main.rs +++ b/validator/src/main.rs @@ -1213,8 +1213,16 @@ pub fn main() { Arg::with_name("tpu_use_quic") .long("tpu-use-quic") .takes_value(false) + .hidden(true) + .conflicts_with("tpu_disable_quic") .help("Use QUIC to send transactions."), ) + .arg( + Arg::with_name("tpu_disable_quic") + .long("tpu-disable-quic") + .takes_value(false) + .help("Do not use QUIC to send transactions."), + ) .arg( Arg::with_name("disable_quic_servers") .long("disable-quic-servers") @@ -2314,7 +2322,7 @@ pub fn main() { let restricted_repair_only_mode = matches.is_present("restricted_repair_only_mode"); let accounts_shrink_optimize_total_space = value_t_or_exit!(matches, "accounts_shrink_optimize_total_space", bool); - let tpu_use_quic = matches.is_present("tpu_use_quic"); + let tpu_use_quic = !matches.is_present("tpu_disable_quic"); let enable_quic_servers = !matches.is_present("disable_quic_servers"); let tpu_connection_pool_size = value_t_or_exit!(matches, "tpu_connection_pool_size", usize);