Skip to content

Commit

Permalink
refactor!: disable keep-alives by default
Browse files Browse the repository at this point in the history
This changes the `keep_alive_interval` set for `Config::default()` to
`None`, disabling keep-alives. This is thought to be a more sane default
for a library, since keep-alives may otherwise cause connections to stay
open forever, leaking resources. It remains easy to opt-in to
keep-alives by simply setting `Some(duration)`.

BREAKING CHANGE: The `qp2p::config::DEFAULT_KEEP_ALIVE_INTERVAL`
constant has been removed, and the default value for
`keep_alive_interval` set for `Config::default()` is now `None`, meaning
keep-alives are disabled by default.
  • Loading branch information
Chris Connelly authored and connec committed Oct 28, 2021
1 parent 7f23422 commit 7788fda
Showing 1 changed file with 3 additions and 26 deletions.
29 changes: 3 additions & 26 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,6 @@ use std::{future::Future, net::IpAddr, sync::Arc, time::Duration};
/// see no conversation between them.
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 @@ -97,7 +89,7 @@ pub struct CertificateGenerationError(
);

/// QuicP2p configurations
#[derive(Clone, Debug, Serialize, Deserialize)]
#[derive(Clone, Debug, Default, 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 @@ -127,8 +119,8 @@ pub struct Config {
///
/// Keep-alives prevent otherwise idle connections from timing out.
///
/// If unspecified, this will default to [`DEFAULT_KEEP_ALIVE_INTERVAL`].
#[serde(default = "default_keep_alive_interval")]
/// If unspecified, this will default to `None`, disabling keep-alives.
#[serde(default)]
pub keep_alive_interval: Option<Duration>,

/// How long UPnP port mappings will last.
Expand All @@ -146,21 +138,6 @@ 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

0 comments on commit 7788fda

Please sign in to comment.