sfackler / r2d2

A generic connection pool for Rust
Apache License 2.0
1.51k stars 82 forks source link

Ability to reset connection pool? #39

Closed saghm closed 7 years ago

saghm commented 7 years ago

Hi! I'm currently looking into using r2d2 for the connection pooling in the MongoDB Rust driver, and I've hit a minor roadblock with regards to be able to reset the connection pool in the case that the topology changes. Specifically, the driver uses a background thread per server to monitor the state of the topology, and if one of the monitoring threads loses the ability to connect to a server, then the driver needs to refresh all connections in the pool to establish the new topology. As far as I can tell, r2d2 doesn't currently provide this functionality publicly, although it appears that there's some functionality for it internally. Is there any chance that an ability to reset the connection pool could be made public in some form? I'd be happy to contribute a pull request if you're open to this idea but don't have the time to work on it.

sfackler commented 7 years ago

By refresh do you mean discard all connections? The simple thing would just be to add a method which clears the idle connection queue, but that will leave any currently checked out connections alive once they're returned. It might make more sense to use the ManageConnection::is_valid function to discard those old connections on demand.

saghm commented 7 years ago

Yeah, I'd like to be able to discard all connections if possible. There's a spec for MongoDB drivers pertaining to topology monitoring, and it specifies that if one of the monitoring threads loses connection to a server, the connection pool should be cleared and connections should be reestablished from scratch (you can read the specific language here if you're curious).

The way our driver is architected, users don't deal directly with the connection pool. There's a single thread-safe client object, and when users perform an operation, under the hood the client selects one of the connections in the pool to perform the operation with. In order to replace our connection pool with r2d2, we'd need our new connection pool to have the ability to "reset" all the connections (presumably by having them all disconnect and then reconnect, or just dropping all the connection objects and creating new ones if that's simpler).

Is this something you'd be open to having as part of r2d2's API? We likely wouldn't be able to switch out our old connection pool to r2d2 without something like this, although I can understand if you're prefer to keep the API as-is.

sfackler commented 7 years ago

A method that discards all idle connections seems reasonable to add if that's all you need.

saghm commented 7 years ago

It's possible that the function I linked to in the codebase might not be doing what I originally thought it did. What we're looking for is to have the ability to cause all connections to be disconnected and then reconnect again. One thing that might not be clear is that users of MongoDB drivers will generally only give a URI for one of the members of a replica set, and then the driver will discover all the other members and assign connections accordingly. The purpose of having all the connections dropped and reestablished when a monitor loses connection to a server is to ensure that there aren't any connections to servers that have gone down (or are disconnected from the rest of the replica set for another reason). I'm not sure that discarding only the idle connections would be sufficient for our use case, as some of the non-idle connections might still be connected to replica set members that aren't in contact with the rest of the set anymore.

sfackler commented 7 years ago

You may want a combination of clearing the idle connections and using ManageConnection health check methods then - the connection manager to ensure that no old connections are used, and the idle clear to make the pool immediately start to build up new connections.

saghm commented 7 years ago

Thanks for the suggestion! We were able to work around the problem by just discarding the old pool and creating a new one with the same options, so I'll close this issue.