Skip to content

Commit

Permalink
feat(client): rename client::Builder pool options (#2142)
Browse files Browse the repository at this point in the history
- Renamed `keep_alive_timeout` to `pool_idle_timeout`.
- Renamed `max_idle_per_host` to `pool_max_idle_per_host`.
- Deprecated `keep_alive(bool)` due to confusing name. To disable the
  connection pool, call `pool_max_idle_per_host(0)`.
  • Loading branch information
seanmonstar authored Feb 27, 2020
1 parent 48102d6 commit a82fd6c
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 45 deletions.
76 changes: 49 additions & 27 deletions src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,11 @@ impl Client<(), Body> {
/// ```
/// # #[cfg(feature = "runtime")]
/// # fn run () {
/// use std::time::Duration;
/// use hyper::Client;
///
/// let client = Client::builder()
/// .keep_alive(true)
/// .pool_idle_timeout(Duration::from_secs(30))
/// .http2_only(true)
/// .build_http();
/// # let infer: Client<_, hyper::Body> = client;
Expand Down Expand Up @@ -842,10 +843,11 @@ fn set_scheme(uri: &mut Uri, scheme: Scheme) {
/// ```
/// # #[cfg(feature = "runtime")]
/// # fn run () {
/// use std::time::Duration;
/// use hyper::Client;
///
/// let client = Client::builder()
/// .keep_alive(true)
/// .pool_idle_timeout(Duration::from_secs(30))
/// .http2_only(true)
/// .build_http();
/// # let infer: Client<_, hyper::Body> = client;
Expand All @@ -870,38 +872,69 @@ impl Default for Builder {
},
conn_builder: conn::Builder::new(),
pool_config: pool::Config {
enabled: true,
keep_alive_timeout: Some(Duration::from_secs(90)),
max_idle_per_host: ::std::usize::MAX,
idle_timeout: Some(Duration::from_secs(90)),
max_idle_per_host: std::usize::MAX,
},
}
}
}

impl Builder {
/// Enable or disable keep-alive mechanics.
///
/// Default is enabled.
#[inline]
#[doc(hidden)]
#[deprecated(
note = "name is confusing, to disable the connection pool, call pool_max_idle_per_host(0)"
)]
pub fn keep_alive(&mut self, val: bool) -> &mut Self {
self.pool_config.enabled = val;
self
if !val {
// disable
self.pool_max_idle_per_host(0)
} else if self.pool_config.max_idle_per_host == 0 {
// enable
self.pool_max_idle_per_host(std::usize::MAX)
} else {
// already enabled
self
}
}

#[doc(hidden)]
#[deprecated(note = "renamed to `pool_idle_timeout`")]
pub fn keep_alive_timeout<D>(&mut self, val: D) -> &mut Self
where
D: Into<Option<Duration>>,
{
self.pool_idle_timeout(val)
}

/// Set an optional timeout for idle sockets being kept-alive.
///
/// Pass `None` to disable timeout.
///
/// Default is 90 seconds.
#[inline]
pub fn keep_alive_timeout<D>(&mut self, val: D) -> &mut Self
pub fn pool_idle_timeout<D>(&mut self, val: D) -> &mut Self
where
D: Into<Option<Duration>>,
{
self.pool_config.keep_alive_timeout = val.into();
self.pool_config.idle_timeout = val.into();
self
}

#[doc(hidden)]
#[deprecated(note = "renamed to `pool_max_idle_per_host`")]
pub fn max_idle_per_host(&mut self, max_idle: usize) -> &mut Self {
self.pool_config.max_idle_per_host = max_idle;
self
}

/// Sets the maximum idle connection per host allowed in the pool.
///
/// Default is `usize::MAX` (no limit).
pub fn pool_max_idle_per_host(&mut self, max_idle: usize) -> &mut Self {
self.pool_config.max_idle_per_host = max_idle;
self
}
// HTTP/1 options

/// Set whether HTTP/1 connections should try to use vectored writes,
/// or always flatten into a single buffer.
///
Expand All @@ -910,7 +943,6 @@ impl Builder {
/// support vectored writes well, such as most TLS implementations.
///
/// Default is `true`.
#[inline]
pub fn http1_writev(&mut self, val: bool) -> &mut Self {
self.conn_builder.h1_writev(val);
self
Expand All @@ -921,7 +953,6 @@ impl Builder {
/// Note that setting this option unsets the `http1_max_buf_size` option.
///
/// Default is an adaptive read buffer.
#[inline]
pub fn http1_read_buf_exact_size(&mut self, sz: usize) -> &mut Self {
self.conn_builder.h1_read_buf_exact_size(Some(sz));
self
Expand All @@ -936,7 +967,6 @@ impl Builder {
/// # Panics
///
/// The minimum value allowed is 8192. This method panics if the passed `max` is less than the minimum.
#[inline]
pub fn http1_max_buf_size(&mut self, max: usize) -> &mut Self {
self.conn_builder.h1_max_buf_size(max);
self
Expand Down Expand Up @@ -1006,14 +1036,6 @@ impl Builder {
self
}

/// Sets the maximum idle connection per host allowed in the pool.
///
/// Default is `usize::MAX` (no limit).
pub fn max_idle_per_host(&mut self, max_idle: usize) -> &mut Self {
self.pool_config.max_idle_per_host = max_idle;
self
}

/// Set whether to retry requests that get disrupted before ever starting
/// to write.
///
Expand Down Expand Up @@ -1060,8 +1082,8 @@ impl Builder {
B::Data: Send,
{
let mut connector = HttpConnector::new();
if self.pool_config.enabled {
connector.set_keepalive(self.pool_config.keep_alive_timeout);
if self.pool_config.is_enabled() {
connector.set_keepalive(self.pool_config.idle_timeout);
}
self.build(connector)
}
Expand Down
19 changes: 11 additions & 8 deletions src/client/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,19 @@ struct WeakOpt<T>(Option<Weak<T>>);

#[derive(Clone, Copy, Debug)]
pub(super) struct Config {
pub(super) enabled: bool,
pub(super) keep_alive_timeout: Option<Duration>,
pub(super) idle_timeout: Option<Duration>,
pub(super) max_idle_per_host: usize,
}

impl Config {
pub(super) fn is_enabled(&self) -> bool {
self.max_idle_per_host > 0
}
}

impl<T> Pool<T> {
pub fn new(config: Config, __exec: &Exec) -> Pool<T> {
let inner = if config.enabled {
let inner = if config.is_enabled() {
Some(Arc::new(Mutex::new(PoolInner {
connecting: HashSet::new(),
idle: HashMap::new(),
Expand All @@ -105,7 +110,7 @@ impl<T> Pool<T> {
waiters: HashMap::new(),
#[cfg(feature = "runtime")]
exec: __exec.clone(),
timeout: config.keep_alive_timeout,
timeout: config.idle_timeout,
})))
} else {
None
Expand Down Expand Up @@ -797,8 +802,7 @@ mod tests {
fn pool_max_idle_no_timer<T>(max_idle: usize) -> Pool<T> {
let pool = Pool::new(
super::Config {
enabled: true,
keep_alive_timeout: Some(Duration::from_millis(100)),
idle_timeout: Some(Duration::from_millis(100)),
max_idle_per_host: max_idle,
},
&Exec::Default,
Expand Down Expand Up @@ -900,8 +904,7 @@ mod tests {

let pool = Pool::new(
super::Config {
enabled: true,
keep_alive_timeout: Some(Duration::from_millis(10)),
idle_timeout: Some(Duration::from_millis(10)),
max_idle_per_host: std::usize::MAX,
},
&Exec::Default,
Expand Down
10 changes: 3 additions & 7 deletions tests/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1282,13 +1282,9 @@ mod dispatch_impl {
let _ = rx2.recv();
});

let client =
Client::builder()
.keep_alive(false)
.build(DebugConnector::with_http_and_closes(
HttpConnector::new(),
closes_tx,
));
let client = Client::builder().pool_max_idle_per_host(0).build(
DebugConnector::with_http_and_closes(HttpConnector::new(), closes_tx),
);

let req = Request::builder()
.uri(&*format!("http://{}/a", addr))
Expand Down
3 changes: 0 additions & 3 deletions tests/support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use std::sync::{
atomic::{AtomicUsize, Ordering},
Arc, Mutex,
};
use std::time::Duration;

use hyper::client::HttpConnector;
use hyper::service::{make_service_fn, service_fn};
Expand Down Expand Up @@ -326,7 +325,6 @@ async fn async_test(cfg: __TestConfig) {

let connector = HttpConnector::new();
let client = Client::builder()
.keep_alive_timeout(Duration::from_secs(10))
.http2_only(cfg.client_version == 2)
.build::<_, Body>(connector);

Expand Down Expand Up @@ -450,7 +448,6 @@ struct ProxyConfig {

fn naive_proxy(cfg: ProxyConfig) -> (SocketAddr, impl Future<Output = ()>) {
let client = Client::builder()
.keep_alive_timeout(Duration::from_secs(10))
.http2_only(cfg.version == 2)
.build_http::<Body>();

Expand Down

0 comments on commit a82fd6c

Please sign in to comment.