feature: Add multi-node connection pool #189
Conversation
|
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? |
tommilligan
changed the title
elasticsearch/src/http/transport.rs
Outdated
| timeout, | ||
| )?; | ||
|
|
||
| spawn(move || { |
There was a problem hiding this comment.
if we require tokio to be around anyways, wouldn't it be better (in terms of resource usage etc.) to spawn a tokio task for the reseeding?
elasticsearch/src/http/transport.rs
Outdated
| return Err(crate::error::lib("Bound Address is empty")); | ||
| } | ||
|
|
||
| let matches = ADDRESS_REGEX |
There was a problem hiding this comment.
I am a little uncomfortable using regex based parsing here. Wouldn't it be easier to check the address str for a valid scheme prefix? That would be http:// https:// and if it doesn't have one, prepend the given scheme, parse the url and then use Url::set_scheme?
|
From the perspective of a user of the library, I'm a bit concerned over the number of A library that uwraps/panics is really hard to deal with when it comes to providing helpful errors to users, using Result and some kind of Error allows the application to provide detail, hints, and additional help to a user as to what went wrong also it can then handle a failure gracefully - after all a hard stop of an application is not always the desired behavior on a failure in a library. |
* 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.
* The connection should be owned by the current user of said connection
* make review edits
* Style changes per code review
swallez
force-pushed
the
StaticNodeList
branch
from
9d54c64
to
2f64bea
Compare
Closes #57
Based on/supersedes #139 by @srleyva
Initially reviewed by @russcam and @swallez
This PR rebases #139 on top of current master, updates the added dependencies to current versions, and fixes clippy/deprecation warnings present in the diff.
I have maintained the original commit history for ease of picking up review from the existing PR - I believe everything after and including
Add regex parsing bound_address to URLis unreviewed.I have a couple of thoughts of potential improvements, but would like to hear from others before I sink any more time into this.