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

Revert "Enable QUIC client by default. Add arg to disable QUIC client… #26913

Merged
merged 1 commit into from
Aug 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions banking-bench/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,10 @@ fn main() {
.help("Number of threads to use in the banking stage"),
)
.arg(
Arg::new("tpu_disable_quic")
.long("tpu-disable-quic")
Arg::new("tpu_use_quic")
.long("tpu-use-quic")
.takes_value(false)
.help("Disable forwarding messages to TPU using QUIC"),
.help("Forward messages to TPU using QUIC"),
)
.get_matches();

Expand Down
10 changes: 5 additions & 5 deletions bench-tps/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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_disable_quic")
.long("tpu-disable-quic")
Arg::with_name("tpu_use_quic")
.long("tpu-use-quic")
.takes_value(false)
.help("Do not submit transactions via QUIC; only affects ThinClient (default) \
.help("Submit transactions via QUIC; only affects ThinClient (default) \
or TpuClient sends"),
)
.arg(
Expand Down Expand Up @@ -348,8 +348,8 @@ pub fn extract_args(matches: &ArgMatches) -> Config {
args.external_client_type = ExternalClientType::RpcClient;
}

if matches.is_present("tpu_disable_quic") {
args.use_quic = false;
if matches.is_present("tpu_use_quic") {
args.use_quic = true;
}

if let Some(v) = matches.value_of("tpu_connection_pool_size") {
Expand Down
30 changes: 7 additions & 23 deletions client/src/connection_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = true;
pub const DEFAULT_TPU_USE_QUIC: bool = false;

/// Default TPU connection pool size per remote address
pub const DEFAULT_TPU_CONNECTION_POOL_SIZE: usize = 4;
Expand Down Expand Up @@ -683,11 +683,6 @@ 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(|_| {
Expand All @@ -700,29 +695,18 @@ mod tests {
let map = connection_cache.map.read().unwrap();
assert!(map.len() == MAX_CONNECTIONS);
addrs.iter().for_each(|a| {
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 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 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_with_quic_port).expect("Address not found");
let _conn = map.get(&addr).expect("Address not found");
}

#[test]
Expand Down
1 change: 1 addition & 0 deletions dos/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ mod tests {
"--valid-signatures",
"--num-signatures",
"8",
"--tpu-use-quic",
"--send-batch-size",
"1",
])
Expand Down
2 changes: 1 addition & 1 deletion multinode-demo/bootstrap-validator.sh
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ while [[ -n $1 ]]; do
elif [[ $1 = --enable-rpc-bigtable-ledger-storage ]]; then
args+=("$1")
shift
elif [[ $1 = --tpu-disable-quic ]]; then
elif [[ $1 = --tpu-use-quic ]]; then
args+=("$1")
shift
elif [[ $1 = --rpc-send-batch-ms ]]; then
Expand Down
10 changes: 1 addition & 9 deletions validator/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1213,16 +1213,8 @@ 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")
Expand Down Expand Up @@ -2322,7 +2314,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_disable_quic");
let tpu_use_quic = matches.is_present("tpu_use_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);

Expand Down