From 29948ea60cacc865c55814fc3741a7d058553ac5 Mon Sep 17 00:00:00 2001 From: Vladimir Kostyukov Date: Fri, 3 May 2019 01:05:20 +0000 Subject: [PATCH] finagle-core: Clarify that session pool settings are per-host Problem / Solution Session pooling docs could be more explicit about being applied per-host and not per-client. JIRA Issues: CSL-7758 Differential Revision: https://phabricator.twitter.biz/D308564 --- doc/src/sphinx/Clients.rst | 26 +++++++++++++------ .../finagle/param/ClientSessionParams.scala | 8 ++++-- .../finagle/param/ClientTransportParams.scala | 5 ++-- .../twitter/finagle/param/SessionParams.scala | 2 +- .../finagle/param/SessionPoolingParams.scala | 2 +- .../finagle/param/WithSessionPool.scala | 3 +++ 6 files changed, 32 insertions(+), 14 deletions(-) diff --git a/doc/src/sphinx/Clients.rst b/doc/src/sphinx/Clients.rst index 953bf84c8b..f3451eb788 100644 --- a/doc/src/sphinx/Clients.rst +++ b/doc/src/sphinx/Clients.rst @@ -808,10 +808,15 @@ resources open. Depending on the configuration, a Finagle client's stack might contain up to _three_ connection pools stacked on each other: watermark, caching and buffering pools. -The only Finagle protocol that doesn't require any connection pooling (a multiplexing protocol) is -`Mux` so it uses :src:`SingletonPool ` that maintains -a single connection per endpoint. For every other Finagle-supported protocol (i.e., HTTP/1.1, Thrift), -there a connection pooling setup built with watermark and caching pools. +The two Finagle protocols that don't require any connection pooling (multiplexing protocols) are +Mux and HTTP/2 as they both maintain just one connection per remote peer. For every other Finagle- +supported protocol (i.e., HTTP/1.1, Thrift), there a connection pooling setup built with watermark +and caching pools in front of each remote peer. + +.. note:: + + When HTTP/2 is enabled on an HTTP client (and the transport is successfully upgraded), the session + pool caches streams (not connections) multiplexed over a single connection. The default client stack layers caching and watermark pools which amounts to maintaining the low watermark (i.e., ``0``, as long as request concurrency exists), queuing requests above the unbounded high @@ -833,13 +838,18 @@ example [#example]_. .withSessionPool.ttl(5.seconds) .newService("twitter.com") +.. note:: + + All session pool settings are applied to each host in the replica set. Put this way, these settings + are per-host as opposed to per-client. + Thus all the three pools are configured with a single param that takes the following arguments: 1. `minSize` and `maxSize` - low and high watermarks for the watermark pool (note that a Finagle - client will not maintain more connections than `maxSize`) -2. `maxWaiters` - the maximum number of connection requests that are queued when the connection - concurrency exceeds the high watermark -3. `ttl`- the maximum amount of time a session is allowed to be cached in a pool + client will not maintain more connections than `maxSize` per host) +2. `maxWaiters` - the maximum number of connection requests that are queued per host when the + connection concurrency exceeds the high watermark +3. `ttl`- the maximum amount of time a per-host session is allowed to be cached in a pool :ref:`Related stats ` diff --git a/finagle-core/src/main/scala/com/twitter/finagle/param/ClientSessionParams.scala b/finagle-core/src/main/scala/com/twitter/finagle/param/ClientSessionParams.scala index 9857d41418..17e6e52a21 100644 --- a/finagle-core/src/main/scala/com/twitter/finagle/param/ClientSessionParams.scala +++ b/finagle-core/src/main/scala/com/twitter/finagle/param/ClientSessionParams.scala @@ -28,8 +28,12 @@ class ClientSessionParams[A <: Stack.Parameterized[A]](self: Stack.Parameterized * TCP connection time. Futures returned from `factory()` will always be satisfied * within this timeout plus any applied [[com.twitter.finagle.client.LatencyCompensation]]. * - * This timeout also includes resolving logical destinations, but the cost of - * resolution is amortized. + * This timeout also includes the following, but the cost is amortized over subsequent + * acquisitions of the same (possibly cached) session: + * + * - Resolving logical destinations + * - SSL handshake if configured + * - HTTP proxy handshake if configured * * @see [[https://twitter.github.io/finagle/guide/Clients.html#timeouts-expiration]] */ diff --git a/finagle-core/src/main/scala/com/twitter/finagle/param/ClientTransportParams.scala b/finagle-core/src/main/scala/com/twitter/finagle/param/ClientTransportParams.scala index 56654b16fd..88fbf9e542 100644 --- a/finagle-core/src/main/scala/com/twitter/finagle/param/ClientTransportParams.scala +++ b/finagle-core/src/main/scala/com/twitter/finagle/param/ClientTransportParams.scala @@ -24,10 +24,11 @@ class ClientTransportParams[A <: Stack.Parameterized[A]](self: Stack.Parameteriz extends TransportParams(self) { /** - * Configures the TCP connection `timeout` of this client (default: 1 second). + * Configures the socket connect `timeout` of this client (default: 1 second). * * The connection timeout is the maximum amount of time a transport is allowed - * to spend establishing a TCP connection. + * to spend connecting to a remote socket. This does not include an actual session creation + * (SSL handshake, HTTP proxy handshake, etc.) only raw socket connect. */ def connectTimeout(timeout: Duration): A = self.configured(Transporter.ConnectTimeout(timeout)) diff --git a/finagle-core/src/main/scala/com/twitter/finagle/param/SessionParams.scala b/finagle-core/src/main/scala/com/twitter/finagle/param/SessionParams.scala index 295dbf05e2..d530b28c57 100644 --- a/finagle-core/src/main/scala/com/twitter/finagle/param/SessionParams.scala +++ b/finagle-core/src/main/scala/com/twitter/finagle/param/SessionParams.scala @@ -18,7 +18,7 @@ class SessionParams[A <: Stack.Parameterized[A]](self: Stack.Parameterized[A]) { /** * Configures the session lifetime `timeout` - the maximum amount of time a given - * service is allowed to live before it is closed (default: unbounded). + * connection is allowed to live before it is closed (default: unbounded). * * @see [[https://twitter.github.io/finagle/guide/Clients.html#timeouts-expiration]] */ diff --git a/finagle-core/src/main/scala/com/twitter/finagle/param/SessionPoolingParams.scala b/finagle-core/src/main/scala/com/twitter/finagle/param/SessionPoolingParams.scala index 5cebb6673e..c71c019ac2 100644 --- a/finagle-core/src/main/scala/com/twitter/finagle/param/SessionPoolingParams.scala +++ b/finagle-core/src/main/scala/com/twitter/finagle/param/SessionPoolingParams.scala @@ -49,7 +49,7 @@ class SessionPoolingParams[A <: Stack.Parameterized[A]](self: Stack.Parameterize /** * Configures the session pool TTL timeout, the maximum amount of time - * a given _temporary_ session is allowed to be cached in a pool (default: unbounded). + * a given _temporary_ per-host session is allowed to be cached in a pool (default: unbounded). * * @note TTL does not apply to permanent sessions (up to `minSize`). * diff --git a/finagle-core/src/main/scala/com/twitter/finagle/param/WithSessionPool.scala b/finagle-core/src/main/scala/com/twitter/finagle/param/WithSessionPool.scala index 2954916c13..db7f8caf02 100644 --- a/finagle-core/src/main/scala/com/twitter/finagle/param/WithSessionPool.scala +++ b/finagle-core/src/main/scala/com/twitter/finagle/param/WithSessionPool.scala @@ -12,6 +12,9 @@ trait WithSessionPool[A <: Stack.Parameterized[A]] { self: Stack.Parameterized[A /** * An entry point for configuring the client's session pool. * + * All session pool settings are applied to each host in the replica set. Put this way, these + * settings are per-host as opposed to per-client. + * * @see [[https://twitter.github.io/finagle/guide/Clients.html#pooling]] */ val withSessionPool: SessionPoolingParams[A] = new SessionPoolingParams(self)