Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

make the ws buffer size configurable #10013

Merged
12 commits merged into from
Oct 14, 2021
16 changes: 16 additions & 0 deletions client/cli/src/commands/run_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,14 @@ pub struct RunCmd {
#[structopt(long = "ws-max-connections", value_name = "COUNT")]
pub ws_max_connections: Option<usize>,

/// Set the the maximum RPC output buffer size. Default is 10MiB.
#[structopt(long = "rpc-max-in-buffer-capacity")]
pub ws_max_in_buffer_capacity: Option<usize>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a use case for in buffer? We already have max_rpc_payload which limits that, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in my case, but I thought it would be cleaner to make both configurable.


/// Set the the maximum RPC output buffer size. Default is 10MiB.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does it relate to max payload setting? Wouldn't restricting it here further restrict the output max payload?

Suggested change
/// Set the the maximum RPC output buffer size. Default is 10MiB.
/// Set the the maximum RPC output buffer size in MiB. Default is 10.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The buffer seems to be the pre-cursor to the max-payload, at least for the output buffer. I presume you want to have your buffer size be x and your max payload size 80% of x in most cases. But now, we allow max payload to be high like 100 MiB (don't ask why, assume some teams that don't care about DOS need it :p), but with a buffer of 10 MiB your max payload is useless.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but with a buffer of 10 MiB your max payload is useless.

Right, perhaps best to detect such misconfiguration and quit with an error then? I think it might be confusing to see large requests fail because they are limited by some other factor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do this if you agree with setting the default buffer size to 16 MiB, with the default max payload remaining 15 MiB. Then I can add a warning if the max out buffer is ever less than max payload.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

applied. I think it is pretty safe, WDYT?

#[structopt(long = "rpc-max-out-buffer-capacity")]
pub ws_max_out_buffer_capacity: Option<usize>,

/// Specify browser Origins allowed to access the HTTP & WS RPC servers.
///
/// A comma-separated list of origins (protocol://domain or special `null`
Expand Down Expand Up @@ -434,6 +442,14 @@ impl CliConfiguration for RunCmd {
Ok(self.rpc_max_payload)
}

fn ws_max_in_buffer_capacity(&self) -> Result<Option<usize>> {
Ok(self.ws_max_in_buffer_capacity)
}

fn ws_max_out_buffer_capacity(&self) -> Result<Option<usize>> {
Ok(self.ws_max_out_buffer_capacity)
}

fn transaction_pool(&self) -> Result<TransactionPoolOptions> {
Ok(self.pool_config.transaction_pool())
}
Expand Down
12 changes: 12 additions & 0 deletions client/cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,16 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
Ok(None)
}

/// Get maximum WS input buffer capacity.
fn ws_max_in_buffer_capacity(&self) -> Result<Option<usize>> {
Ok(None)
}

/// Get maximum WS output buffer capacity.
fn ws_max_out_buffer_capacity(&self) -> Result<Option<usize>> {
Ok(None)
}

/// Get the prometheus configuration (`None` if disabled)
///
/// By default this is `None`.
Expand Down Expand Up @@ -513,6 +523,8 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
rpc_ws_max_connections: self.rpc_ws_max_connections()?,
rpc_cors: self.rpc_cors(is_dev)?,
rpc_max_payload: self.rpc_max_payload()?,
ws_max_in_buffer_capacity: self.ws_max_in_buffer_capacity()?,
ws_max_out_buffer_capacity: self.ws_max_out_buffer_capacity()?,
prometheus_config: self.prometheus_config(DCV::prometheus_listen_port())?,
telemetry_endpoints,
default_heap_pages: self.default_heap_pages()?,
Expand Down
20 changes: 18 additions & 2 deletions client/rpc-servers/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ const MEGABYTE: usize = 1024 * 1024;
/// Maximal payload accepted by RPC servers.
pub const RPC_MAX_PAYLOAD_DEFAULT: usize = 15 * MEGABYTE;

/// Maximal buffer size in RPC servers.
///
/// Only relevant in the WS context.
pub const RPC_MAX_BUFFER_CAPACITY_DEFAULT: usize = 10 * MEGABYTE;

/// Default maximum number of connections for WS RPC servers.
const WS_MAX_CONNECTIONS: usize = 100;

Expand Down Expand Up @@ -172,18 +177,29 @@ pub fn start_ws<
cors: Option<&Vec<String>>,
io: RpcHandler<M>,
maybe_max_payload_mb: Option<usize>,
maybe_max_in_buffer_capacity_mb: Option<usize>,
maybe_max_out_buffer_capacity_mb: Option<usize>,
server_metrics: ServerMetrics,
tokio_handle: tokio::runtime::Handle,
) -> io::Result<ws::Server> {
let rpc_max_payload = maybe_max_payload_mb
let max_payload = maybe_max_payload_mb
.map(|mb| mb.saturating_mul(MEGABYTE))
.unwrap_or(RPC_MAX_PAYLOAD_DEFAULT);
let max_in_buffer_capacity = maybe_max_in_buffer_capacity_mb
.map(|mb| mb.saturating_mul(MEGABYTE))
.unwrap_or(RPC_MAX_BUFFER_CAPACITY_DEFAULT);
let max_out_buffer_capacity = maybe_max_out_buffer_capacity_mb
.map(|mb| mb.saturating_mul(MEGABYTE))
.unwrap_or(RPC_MAX_BUFFER_CAPACITY_DEFAULT);

ws::ServerBuilder::with_meta_extractor(io, |context: &ws::RequestContext| {
context.sender().into()
})
.event_loop_executor(tokio_handle)
.max_payload(rpc_max_payload)
.max_payload(max_payload)
.max_connections(max_connections.unwrap_or(WS_MAX_CONNECTIONS))
.max_in_buffer_capacity(max_in_buffer_capacity)
.max_out_buffer_capacity(max_out_buffer_capacity)
.allowed_origins(map_cors(cors))
.allowed_hosts(hosts_filtering(cors.is_some()))
.session_stats(server_metrics)
Expand Down
2 changes: 2 additions & 0 deletions client/service/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ pub struct Configuration {
pub rpc_methods: RpcMethods,
/// Maximum payload of rpc request/responses.
pub rpc_max_payload: Option<usize>,
pub ws_max_in_buffer_capacity: Option<usize>,
pub ws_max_out_buffer_capacity: Option<usize>,
/// Prometheus endpoint configuration. `None` if disabled.
pub prometheus_config: Option<PrometheusConfig>,
/// Telemetry service URL. `None` if disabled.
Expand Down
2 changes: 2 additions & 0 deletions client/service/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,8 @@ fn start_rpc_servers<
),
)?,
config.rpc_max_payload,
config.ws_max_in_buffer_capacity,
config.ws_max_out_buffer_capacity,
server_metrics.clone(),
config.tokio_handle.clone(),
)
Expand Down
2 changes: 2 additions & 0 deletions client/service/test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,8 @@ fn node_config<
rpc_cors: None,
rpc_methods: Default::default(),
rpc_max_payload: None,
ws_max_in_buffer_capacity: None,
ws_max_out_buffer_capacity: None,
prometheus_config: None,
telemetry_endpoints: None,
default_heap_pages: None,
Expand Down
2 changes: 2 additions & 0 deletions test-utils/test-runner/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ pub fn default_config(tokio_handle: Handle, mut chain_spec: Box<dyn ChainSpec>)
rpc_cors: None,
rpc_methods: Default::default(),
rpc_max_payload: None,
ws_max_in_buffer_capacity: None,
ws_max_out_buffer_capacity: None,
prometheus_config: None,
telemetry_endpoints: None,
default_heap_pages: None,
Expand Down