From bd2dbc055cf311b624af2e8ddcd97aabcd92c4b1 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Mon, 19 Oct 2020 21:43:32 +0200 Subject: [PATCH] client/network: Remove option to disable yamux flow control (#7358) With the `OnRead` flow control option yamux "send[s] window updates only when data is read on the receiving end" and not as soon as "a Stream's receive window drops to 0". Yamux flow control has proven itself. This commit removes the feature flag. Yamux flow control is now always enabled. --- substrate/client/cli/src/params/network_params.rs | 6 ------ substrate/client/network/src/config.rs | 3 --- substrate/client/network/src/service.rs | 10 +++++----- substrate/client/network/src/transport.rs | 10 +++------- substrate/client/service/test/src/lib.rs | 1 - substrate/utils/browser/src/lib.rs | 1 - 6 files changed, 8 insertions(+), 23 deletions(-) diff --git a/substrate/client/cli/src/params/network_params.rs b/substrate/client/cli/src/params/network_params.rs index faaf2c2bd2..79e7a70024 100644 --- a/substrate/client/cli/src/params/network_params.rs +++ b/substrate/client/cli/src/params/network_params.rs @@ -92,11 +92,6 @@ pub struct NetworkParams { #[structopt(flatten)] pub node_key_params: NodeKeyParams, - /// Disable the yamux flow control. This option will be removed in the future once there is - /// enough confidence that this feature is properly working. - #[structopt(long)] - pub no_yamux_flow_control: bool, - /// Enable peer discovery on local networks. /// /// By default this option is true for `--dev` and false otherwise. @@ -158,7 +153,6 @@ impl NetworkParams { enable_mdns: !is_dev && !self.no_mdns, allow_private_ipv4: !self.no_private_ipv4, wasm_external_transport: None, - use_yamux_flow_control: !self.no_yamux_flow_control, }, max_parallel_downloads: self.max_parallel_downloads, allow_non_globals_in_dht: self.discover_local || is_dev, diff --git a/substrate/client/network/src/config.rs b/substrate/client/network/src/config.rs index 7445ea0534..c144bebb5f 100644 --- a/substrate/client/network/src/config.rs +++ b/substrate/client/network/src/config.rs @@ -451,7 +451,6 @@ impl NetworkConfiguration { enable_mdns: false, allow_private_ipv4: true, wasm_external_transport: None, - use_yamux_flow_control: false, }, max_parallel_downloads: 5, allow_non_globals_in_dht: false, @@ -519,8 +518,6 @@ pub enum TransportConfig { /// This parameter exists whatever the target platform is, but it is expected to be set to /// `Some` only when compiling for WASM. wasm_external_transport: Option, - /// Use flow control for yamux streams if set to true. - use_yamux_flow_control: bool, }, /// Only allow connections within the same process. diff --git a/substrate/client/network/src/service.rs b/substrate/client/network/src/service.rs index af123e94fa..0139739ad2 100644 --- a/substrate/client/network/src/service.rs +++ b/substrate/client/network/src/service.rs @@ -334,12 +334,12 @@ impl NetworkWorker { behaviour.register_notifications_protocol(*engine_id, protocol_name.clone()); } let (transport, bandwidth) = { - let (config_mem, config_wasm, flowctrl) = match params.network_config.transport { - TransportConfig::MemoryOnly => (true, None, false), - TransportConfig::Normal { wasm_external_transport, use_yamux_flow_control, .. } => - (false, wasm_external_transport, use_yamux_flow_control) + let (config_mem, config_wasm) = match params.network_config.transport { + TransportConfig::MemoryOnly => (true, None), + TransportConfig::Normal { wasm_external_transport, .. } => + (false, wasm_external_transport) }; - transport::build_transport(local_identity, config_mem, config_wasm, flowctrl) + transport::build_transport(local_identity, config_mem, config_wasm) }; let mut builder = SwarmBuilder::new(transport, behaviour, local_peer_id.clone()) .peer_connection_limit(crate::MAX_CONNECTIONS_PER_PEER) diff --git a/substrate/client/network/src/transport.rs b/substrate/client/network/src/transport.rs index a18aa3d56f..10b374a4f2 100644 --- a/substrate/client/network/src/transport.rs +++ b/substrate/client/network/src/transport.rs @@ -41,7 +41,6 @@ pub fn build_transport( keypair: identity::Keypair, memory_only: bool, wasm_external_transport: Option, - use_yamux_flow_control: bool ) -> (Boxed<(PeerId, StreamMuxerBox), io::Error>, Arc) { // Build the base layer of the transport. let transport = if let Some(t) = wasm_external_transport { @@ -109,12 +108,9 @@ pub fn build_transport( mplex_config.max_buffer_len(usize::MAX); let mut yamux_config = libp2p::yamux::Config::default(); - - if use_yamux_flow_control { - // Enable proper flow-control: window updates are only sent when - // buffered data has been consumed. - yamux_config.set_window_update_mode(libp2p::yamux::WindowUpdateMode::OnRead); - } + // Enable proper flow-control: window updates are only sent when + // buffered data has been consumed. + yamux_config.set_window_update_mode(libp2p::yamux::WindowUpdateMode::OnRead); core::upgrade::SelectUpgrade::new(yamux_config, mplex_config) .map_inbound(move |muxer| core::muxing::StreamMuxerBox::new(muxer)) diff --git a/substrate/client/service/test/src/lib.rs b/substrate/client/service/test/src/lib.rs index 7c4e28f4dd..29e5e4bd86 100644 --- a/substrate/client/service/test/src/lib.rs +++ b/substrate/client/service/test/src/lib.rs @@ -225,7 +225,6 @@ fn node_config