Connection pool design advice

[PopFlamingo]

I’m seeking for design advice on how to build a connection pool, I would like to spend some time implementing it and hopefully contribute them if I get good results.
I don't know exactly where to start so I thought it might be good to expose what I understand on how the connection pool should work, and show design ideas I found by exploring already existing implementations.

First, the general concept of the connection pool: on each new request, the client asks its connection pool for a connection to the server. When it receives a new connection request, the pool has essentially three choices:

• Use an already opened, unused connection
• Open a new connection if no other connection is available
• If there are already too many concurrent connections to the server, wait until an already opened connection becomes available
  In practice the connection is a NIO Channel that is either directly a TCP connection in the case of HTTP/1.1, or a stream for an HTTP/2 multiplexed connection.

After some research, I stumbled upon the Apache Foundation HttpClient. It uses an HttpClientConnectionManager interface that lets the user provide the manager they wish. A PoolingHttpClientConnectionManager class implements this interface.
I wonder wether the principle of a user provided connection manager makes sense for the Swift NIO HTTP client? In the case of the Apache, I see the other class that implements HttpClientConnectionManager is BasicHttpClientConnectionManager, a connection manager that maintains only a single active connection at a time, but if I understand it correctly, this manager purpose is to enable to have one client per thread, which doesn’t seem appliable/useful with NIO? We can also imagine concurrent yet unpooled connection management, which is how the client currently works, but I don’t see the advantage of this over a pooled connection management?

Speaking more specifically about the connection pool, I have a few questions:

• How to give back a connection to the pool? Should we pass it as an argument to a pool method (such as this) , or could we have a design where the pool passes an object (maybe a promise?) to the client on which it calls the appropriate method when it has finished leasing the connection?
• How should we implement the thread safety of the pool? The already existing APIs based on Future/Promises should make this easy for many parts, but the data structure storing the connections must still be protected. Regarding the threading model of NIO, is this a good practice to hold a reference to a specific event loop and to use .execute to ensure what we want is executed on the right thread?
• Speaking of the data structure storing the connections, could this be as simple as a dictionary that keys the connections per host/scheme?
• I saw that some pools have a global cleanup delay where the manager will loop over all opened connections to see which ones have been idle for too long in order to evict them. How does this « global cleanup interval » strategy compare to scheduling timeouts per connection using NIO tasks? From what I see, for instance with the IdleStateHandler, NIO doesn’t hesitate to schedule tasks frequently, so I guess the performance impact isn’t too high?
• Some pool designs enable specifying a delay after which a connection request is failed if the pool didn’t provide one in time, is this something we would also want for a pool?

Thank you!

[weissi]

Adding @Lukasa & @tomerd

[Lukasa]

Before I dive in further, I just want to say that I think this is a good starting point for a discussion about designs, and I look forward to seeing a bunch of input here.

Some responses:

I wonder wether the principle of a user provided connection manager makes sense for the Swift NIO HTTP client?

Someday, yes. But I'd begin by not doing that, as it makes it much easier to make changes in the face of testing. It's rarely a good idea to define an interface before you have a specific requirement for a second implementation, as the overlap between the two implementations helps guide design thinking for what the interface should look like.

So I'd begin by implementing just the default pool, and wait for concrete alternative use cases before worrying about how to support those.

How to give back a connection to the pool?

Start with the simplest design, which is that the code that is done with the connection calls a function on the pool.

How should we implement the thread safety of the pool?

The pool should not be on an event loop. A pool has nothing to do with I/O, and so shouldn't block the I/O of any loop it's on.

In the future, we may want to try to maintain the locality of connections on an event loop, but an MVP design has the pool guarded by a lock. It may run on a background thread if we're worried about the cost of the pool operations, but even then I'd begin by simply using a lock to get the code to work right before worrying about how it performs.

Speaking of the data structure storing the connections, could this be as simple as a dictionary that keys the connections per host/scheme?

So long as you're careful to normalise the scheme, yes. Again, in future this data structure will want to evolve in complexity, but beginning with something stupid is usually a good plan.

How does this « global cleanup interval » strategy compare to scheduling timeouts per connection using NIO tasks?

If the pool inserts IdleStateHandlers in all connections then by default this system will basically "just work", so I'd start there. Again, no need to put the cart before the horse: do the thing that is stupidest and easiest to ensure we get right, and then worry later about whether it performs well.

Some pool designs enable specifying a delay after which a connection request is failed if the pool didn’t provide one in time, is this something we would also want for a pool?

For v1, no. Maybe later.

[weissi]

Thanks @adtrevor for starting to think about this. AFAIK, this is one of the key things the community needs next!

So I'd begin by implementing just the default pool, and wait for concrete alternative use cases before worrying about how to support those.

+1, I think starting with something minimal that we land and then iterate on is the winning strategy. As Cory points out, we can define an interface later when we learned much more about connection pooling and what the community really needs.

How should we implement the thread safety of the pool?

The pool should not be on an event loop. A pool has nothing to do with I/O, and so shouldn't block the I/O of any loop it's on.

In the future, we may want to try to maintain the locality of connections on an event loop, but an MVP design has the pool guarded by a lock. It may run on a background thread if we're worried about the cost of the pool operations, but even then I'd begin by simply using a lock to get the code to work right before worrying about how it performs.

100% happy with what Cory proposes but we could also start with an EventLoop-local connection pool. Ie. one that pools connections per EventLoop. So if you request a connection from an EventLoop that doesn't have an existing connection in the pool, it would create a fresh one. Then you wouldn't need any locks but obviously we wouldn't keep the connections as hot as possible :).
I'm really happy either way.

tl;dr: Start with something really dumb first, we can always evolve. I would bootstrap it in this package, at the beginning it can remain totally private so we can change implementation and API at any time.

[tanner0101]

we could also start with an EventLoop-local connection pool. Ie. one that pools connections per EventLoop.

This is how Vapor 4's ConnectionPool operates. A nice side effect as mentioned is that we don't need to worry about locking and the implementation is quite simple.

From Vapor's perspective at least, connection pools should always be associated to a single event loop. An HTTP request coming into a handler may need to pull connections from multiple pools--maybe Redis, PostgreSQL, and HTTP client for example. If each of these pools has connections associated with different event loops, that could incur unnecessary loop hopping.

However, for the sake of HTTPClient, I don't think the same restrictions apply. Vapor for example always initializes HTTPClient using EventLoopGroupProvider.shared with a single event loop anyway. So regardless of how HTTPClient implements its connection pool, Vapor won't have to worry about extraneous loop hopping.

For people using HTTPClient directly with a supplied ELG, a pool that spans multiple event loops could actually be more powerful. For example, if you supply HTTPClient an ELG w/ 8 loops, and you want to configure HTTPClient to have at most 4 connections open at once, you could not have one pool per event loop. You would need a pool that could span the event loops. I'm not sure this is something we need to support, but it's worth considering. (This is a limitation in Vapor fwiw).

[PopFlamingo]

@weissi @Lukasa @tanner0101 Thank you for your feedback! 🙂

@Lukasa Nice to know many things can and should be kept as much simple for starting!

Speaking of the data structure storing the connections, could this be as simple as a dictionary that keys the connections per host/scheme?

So long as you're careful to normalise the scheme, yes.

Do you refer to case normalisation etc... ?

If the pool inserts IdleStateHandlers in all connections then by default this system will basically "just work", so I'd start there.

That's great! Hadn't realised it could be enough!

@weissi If we used an EventLoop local pool, it would mean that we may have multiple pools per client, right? If so, does this mean that it would be harder to ensure that a maximum concurrent connections per host limit is respected? Maybe I'm missing something, so here is an example: if a request to a server is given to event loop 1, but a subsequent request to the same server is given to event loop 2, do we now have two separate pools that both hold connections to the same server?

If I understand correctly the source code you posted @tanner0101, a ConnectionPoolSource is not only associated with a specific event loop but also generates connections to a specific host and not multiple ones, right? Do you have a link to an example of how it is used in practice?

[tanner0101]

a ConnectionPoolSource is not only associated with a specific event loop but also generates connections to a specific host and not multiple ones, right?

@adtrevor yes exactly. Each connection in the pool must be the same (connected to the same host, port, same auth state, etc)

For HTTP client, I imagine you'd have a dictionary where host/port is the key and value is a pool. Here are some examples of how the connection pool API looks like in use:
https://github.com/vapor/nio-kit/blob/master/Tests/NIOKitTests/ConnectionPoolTests.swift

[Lukasa]

From Vapor's perspective at least, connection pools should always be associated to a single event loop. An HTTP request coming into a handler may need to pull connections from multiple pools--maybe Redis, PostgreSQL, and HTTP client for example. If each of these pools has connections associated with different event loops, that could incur unnecessary loop hopping.

The downside of this approach is that it performs poorly in some workload types.

In particular, in cases where a connection is needed with moderate frequency (e.g. every 2s) but for short times (e.g. a few hundred ms), it is highly possible that systems with large numbers of event loops will spend unnecessary resources creating and maintaining many times more connections than are actually required. For example, if you have 16 event loop threads, you will need to create 16 connections to support this case, despite the fact that these connections spend the vast majority of their time idle (average connection idle time is 32s, as only one connection is needed every 2s). Worse, these connections may be idle long enough to be timed out, which leads to the connection pools having an effective size of zero and providing no pooling at all.

In general the "pool per event loop" model has troubling properties as connection utilisation drops or as loop count increases. It's a useful model for some use cases, but not for all. This is why, down the road, we will probably need to invest in multiple pooling models. However, for now, we are best served building only one.

I don't much mind whether we build the loop-local or global pool first. The advantage in my mind of building the global pool first is that it requires all consumers of the code to ensure that they are guaranteeing their own thread-safety, rather than allowing code to be written that accidentally relies on that fact and cannot be easily tightened up when using a global pool. But I don't care strongly, and if we can move faster by building a per-loop pool then that's fine too.

Speaking of the data structure storing the connections, could this be as simple as a dictionary that keys the connections per host/scheme?
So long as you're careful to normalise the scheme, yes.
Do you refer to case normalisation etc... ?

No, I refer to key normalisation. This includes case normalisation, but is not limited to it.

For HTTP client, I imagine you'd have a dictionary where host/port is the key and value is a pool.

We're beginning to overload the phrase "connection pool", however, so we should aim to clarify. When I have used the phrase "connection pool", I am referring to an object with the following member function on it:

func obtainedPooledConnection(_ key: PoolKey) -> EventLoopFuture<Channel>

What @tanner0101 is describing as a "connection pool" is a sub-component of the above that applies within a single key, and it has a function like:

func obtainConnection() -> EventLoopFuture<Channel>

We should endeavour to distinguish these two things. I'd make the case that we should call the outer layer the "connection pool", as it's the correct level of abstraction for the rest of swift-nio-http-client to operate on. The internals of that object should not be considered relevant for the rest of the client.

Also, re "For HTTP client, I imagine you'd have a dictionary where host/port is the key": that's insufficient for a HTTP client. In general on this issue we should aim to cleave as closely as possible to the fetch specification for web browsers, as that behaviour will reduce the risk of users seeing surprising or insecure behaviour. We need to take it with a grain of salt as many things will be unnecessary outside of a browser context, but it's a good start.

Fetch wants the pool key to be a tuple of origin and credentials where credentials indicates whether the connection is associated with whether the requests on that connection have been credentialed (e.g. with cookies or Auth headers). Right now swift-nio-http-client is not well placed to make this determination so we can potentially omit the credentials aspect to get off the ground, but we should revisit it.

However, note that the other part of the key is not host/port, it's origin. Origin is a 4-tuple of (scheme, host, port, domain). We can omit domain from our notion of origin because it's largely not relevant to us (it'll always be present and always be equal to host), so we want the three-tuple of (scheme, host, port). It's vital to include scheme here as http and https connections must never be treated as the same. As we aren't a browser, we don't need the other set of schemes they support in our pool, so for us scheme is a two-case enumeration.

[PopFlamingo]

@tanner0101 Thanks! Interesting to see how you implemented it for Vapor, I also like the pool.withConnection { ... } method.

@Lukasa While it's my first time implementing a connection pool, the global pool is something that I find easier to reason about in terms of performance benefits compared to having multiple pools. FWIW I also believe that my personal use case for this lib (web crawling) would likely encounter the performance issues you are mentioning if using event loop local pools.

The advantage in my mind of building the global pool first is that it requires all consumers of the code to ensure that they are guaranteeing their own thread-safety

What does "all consumers of the code" refer to? Doesn't the pool handle itself it's thread safety using locks or a specific thread? Were you just thinking about Future hopping?

We're beginning to overload the phrase "connection pool" [...]

Thanks for the clarification.
I see you gave the example of a func obtainedPooledConnection(_ key: PoolKey) method to which we pass a PoolKey. Does this mean we want the pool to expose the keys to the outside and require callers to init the keys themselves? The alternative I had thought of was maybe to build it internally and instead let callers get a connection to a server by just passing a URL? What would be best?

Also, while a connection is a Channel, do we want to directly return a Future<Channel> from the pool, or to instead return a channel warper object that would for instance contain the key the channel is associated with? The issue (?) with directly returning a Channel is that when the pool consumer releases it, we cannot reconstruct a PoolKey from a channel alone AFAIK, so it may require hacky solutions for it to work properly.

Regarding the connection providing component (the one on which we'd call func obtainConnection() -> EventLoopFuture<Channel>), would we essentially have two kinds of it, one for HTTP/1.1 providing concurrent TCP connections to the same server, and another for HTTP/2 providing multiplexed connections?

Thank you

[Lukasa]

What does "all consumers of the code" refer to? Doesn't the pool handle itself it's thread safety using locks or a specific thread?

While the pool does, the users of the Channel may not. If users know that the Channel they receive is always on the same event loop to them, they may be incautious about the thread-safety of communicating with that Channel. For example, they may communicate directly between ChannelHandlers, or avoid using EventLoopFuture.hop(to:). That's not ideal: in some senses it's better to have the more adversarial environment be the first one, at least from a correctness perspective.

The alternative I had thought of was maybe to build it internally and instead let callers get a connection to a server by just passing a URL?

That's certainly fine: I was only drawing out the fundamental portion of the API. It's not a problem at all to provide an even-higher-level one instead.

Also, while a connection is a Channel, do we want to directly return a Future<Channel> from the pool, or to instead return a channel warper object that would for instance contain the key the channel is associated with?

You're quite right, we probably want a HTTPConnection object that provides this kind of abstraction.

Regarding the connection providing component (the one on which we'd call func obtainConnection() -> EventLoopFuture<Channel>), would we essentially have two kinds of it, one for HTTP/1.1 providing concurrent TCP connections to the same server, and another for HTTP/2 providing multiplexed connections?

At the top level, probably not. The issue is that you don't know which one of those things you'll have to be until you've made your first outbound connection. So you'd probably want an abstract one that is backed by an enum that stores the concrete pool type.

[PopFlamingo]

@Lukasa
Thank you! With all of this, I think I now have a pretty good starting point to prototype an implementation.

So you'd probably want an abstract one that is backed by an enum that stores the concrete pool type.

At first I thought I'd have a protocol such as

protocol ConnectionProvider {
    func obtainConnection() -> EventLoopFuture<Channel>
}

And, on first connection request to a server, determine what HTTP protocol to use and instantiate the correct concrete type conforming to ConnectionProvider.
Were you proposing an enum for the potential performance gain?

[Lukasa]

Were you proposing an enum for the potential performance gain?

Yes.

Your version will require the use of a protocol existential, which is not ideal when we know the exact concrete types we need.

[PopFlamingo]

My latest PR is still extremely WIP, many things can probably be improved and I'll be happy to get your feedback on this! 🙂