Skip to content

Commit

Permalink
feat!: support disabling keep-alives
Browse files Browse the repository at this point in the history
This changes the semantics of setting `qp2p::Config::keep_alive_interval
= None` from falling back to the default value, to actually setting
`None` in the underlying transport config, thereby disabling
keep-alives.

BREAKING CHANGE: Setting `qp2p::Config::keep_alive_interval = None` will
now disable keep-alives, rather than falling back to the default
interval.
  • Loading branch information
Chris Connelly authored and connec committed Oct 27, 2021
1 parent 567e680 commit dca54b3
Showing 1 changed file with 25 additions and 8 deletions.
33 changes: 25 additions & 8 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ pub const DEFAULT_IDLE_TIMEOUT: Duration = Duration::from_secs(60);
/// Default for [`Config::keep_alive_interval`] (20 seconds).
pub const DEFAULT_KEEP_ALIVE_INTERVAL: Duration = Duration::from_secs(20);

// serde needs a function when specifying a path for `default`
fn default_keep_alive_interval() -> Option<Duration> {
Some(DEFAULT_KEEP_ALIVE_INTERVAL)
}

/// Default for [`Config::upnp_lease_duration`] (2 minutes).
pub const DEFAULT_UPNP_LEASE_DURATION: Duration = Duration::from_secs(120);

Expand Down Expand Up @@ -92,7 +97,7 @@ pub struct CertificateGenerationError(
);

/// QuicP2p configurations
#[derive(Clone, Debug, Default, Serialize, Deserialize)]
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct Config {
/// Specify if port forwarding via UPnP should be done or not. This can be set to false if the network
/// is run locally on the network loopback or on a local area network.
Expand Down Expand Up @@ -123,7 +128,7 @@ pub struct Config {
/// Keep-alives prevent otherwise idle connections from timing out.
///
/// If unspecified, this will default to [`DEFAULT_KEEP_ALIVE_INTERVAL`].
#[serde(default)]
#[serde(default = "default_keep_alive_interval")]
pub keep_alive_interval: Option<Duration>,

/// How long UPnP port mappings will last.
Expand All @@ -141,6 +146,21 @@ pub struct Config {
pub retry_config: RetryConfig,
}

impl Default for Config {
fn default() -> Self {
Self {
#[cfg(feature = "igd")]
forward_port: Default::default(),
external_port: Default::default(),
external_ip: Default::default(),
idle_timeout: Default::default(),
keep_alive_interval: default_keep_alive_interval(),
upnp_lease_duration: Default::default(),
retry_config: Default::default(),
}
}
}

/// Retry configurations for establishing connections and sending messages.
/// Determines the retry behaviour of requests, by setting the back off strategy used.
#[derive(Clone, Debug, Copy, Serialize, Deserialize)]
Expand Down Expand Up @@ -226,14 +246,11 @@ pub(crate) struct InternalConfig {
impl InternalConfig {
pub(crate) fn try_from_config(config: Config) -> Result<Self> {
let idle_timeout = config.idle_timeout.unwrap_or(DEFAULT_IDLE_TIMEOUT);
let keep_alive_interval = config
.keep_alive_interval
.unwrap_or(DEFAULT_KEEP_ALIVE_INTERVAL);
let upnp_lease_duration = config
.upnp_lease_duration
.unwrap_or(DEFAULT_UPNP_LEASE_DURATION);

let transport = Self::new_transport_config(idle_timeout, keep_alive_interval);
let transport = Self::new_transport_config(idle_timeout, config.keep_alive_interval);
let client = Self::new_client_config(transport.clone());
let server = Self::new_server_config(transport)?;

Expand All @@ -251,15 +268,15 @@ impl InternalConfig {

fn new_transport_config(
idle_timeout: Duration,
keep_alive_interval: Duration,
keep_alive_interval: Option<Duration>,
) -> Arc<quinn::TransportConfig> {
let mut config = quinn::TransportConfig::default();

// QUIC encodes idle timeout in a varint with max size 2^62, which is below what can be
// represented by Duration. For now, just ignore too large idle timeouts.
// FIXME: don't ignore (e.g. clamp/error/panic)?
let _ = config.max_idle_timeout(Some(idle_timeout)).ok();
let _ = config.keep_alive_interval(Some(keep_alive_interval));
let _ = config.keep_alive_interval(keep_alive_interval);

Arc::new(config)
}
Expand Down

0 comments on commit dca54b3

Please sign in to comment.