elasticsearch-rs
elasticsearch-rs copied to clipboard
Implement Multinode connection pool
-
This commit adds a connection pool that takes a static list of nodes and distributes the load.
-
trait for setting connection distribution. Defaults to RoundRobin.
#57
I believe your assessment is correct as far as changing the ConnectionPool interface to account for interior mutability. It seems the only way to do that across threads is a mutex type. I think there’s overhead in requiring every ConnectionPool type to require a Mutex even if it’s not dynamic but it’s only way I see how to do this. Any thoughts?
Thinking about sniffing further and discussing with folks on the clients team, the connection pool is responsible for managing one or more connections (connections being details about nodes in the cluster such as Uri) and data related to connections such as whether the pool supports sniffing, but the Transport
is responsible for acting on this data i.e. the Transport
should perform sniffing and (re)seed the pool if the pool supports it.
With this in mind, a sniffing connection pool would likely have a Arc<RwLock<Vec<Connection>>>
(or similar construct) to allow multi-threaded reads through immutable references, with a single writer through a mutable reference. A question that I have is how this kind of connection pool advertises itself e.g. through the ConnectionPool
trait with something like
pub trait ConnectionPool: Debug + dyn_clone::DynClone + Sync + Send {
/// Gets a reference to the next [Connection]
fn next(&self) -> &Connection;
fn reseedable(&self) -> bool {
false
}
fn reseed(&self, connections: Vec<Connection>) {}
}
Would need to play around to get the right type and function design for this. StaticNodeListConnectionPool
might then be more aptly named MultiNodeConnectionPool
with functions to construct sniffable and non-sniffable i.e. static versions.
WIP as I still need to implement the NodeInfo
parts, but thought I'd get feedback on the current reseed implementation and the changes to the ConnectionPool
trait. :)
Really appreciate your effort, @srleyva! I've left some comments
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?
First pass at node sniff request following elastic docs. I thought even if aren't sure of the final design, having the request parts up would be easier.
Manual tests for round robin load balancing on a local docker-compose ES cluster look quite promising when seeded with http://localhost:9200
:smile:
Ping Response: 200 OK Url: http://172.23.0.3:9200/
Ping Response: 200 OK Url: http://172.23.0.6:9200/
Ping Response: 200 OK Url: http://172.23.0.5:9200/
Ping Response: 200 OK Url: http://172.23.0.2:9200/
Ping Response: 200 OK Url: http://172.23.0.3:9200/
Ping Response: 200 OK Url: http://172.23.0.6:9200/
Ping Response: 200 OK Url: http://172.23.0.5:9200/
Ping Response: 200 OK Url: http://172.23.0.2:9200/
Ping Response: 200 OK Url: http://172.23.0.3:9200/
I'll play around with using a background thread to reseed over the next few days. Thanks for your patience and guidance on this as I am still very new to rust. :smile:
Hey @russcam is there any update on this PR? Apologies for the lack of communication. Its feature complete according to the original requirements. If there's anything else that's needed please let me know 😄
Thanks for the ping, @srleyva 👍 Hoping to get a chance to look at this again this week, unless @swallez beats me to it!
What is holding this PR back? We would like to use the official client, but without at least a way to target multiple elastic nodes, this is not feasible for us. Is there any way I could help out to get this over the finish line?
I'm also interested in getting this merged. The branch looks stale and based on old dependencies - I'll rebase it on master and update to newer dependencies, and open as a new PR.
Great! @tommilligan ping me, if you need another hand or pair of eyes. :)
@mfelsche I've opened the rebased PR at https://github.com/elastic/elasticsearch-rs/pull/189