Closed jamesmunns closed 3 days ago
Notes on configuration file contents:
PeerOptions
)Cool to see work on load balancing. This is a key feature and I was surprised initially when I saw connector
as a struct and not an array - https://github.com/memorysafety/river/blob/main/docs/toml-configuration.md?plain=1#L22-L24 :+1:
I've been working on configuration a bit, as I think there will be quite a bit of configuration surface for this. You can see the changes here: https://github.com/memorysafety/river/pull/42#issuecomment-2154679100
I've been looking at this the past couple of days, and I hadn't realized that the various pingora-load-balancing
traits were not Object Safe.
I also took a bit of a look to see how straightforwards this would be to work around, but the ability for trait implementors to create new instances, generally for the purpose of updating via ArcSwap
items, is pretty fundamental to the current API.
I have a chat scheduled today with the pingora engineers, but I can see the outcomes being one of the following:
This would be a pretty significant API change for pingora-load-balancing, but would allow us to use dyn Trait
to load the implementations at runtime, based on the content of the configuration file.
This would entail wrapping the existing trait implementors with a hand-rolled version of dynamic dispatch, rather than use dyn Trait
directly. This could be an enum
of the different options, or something more invasive or unsafe.
This would allow us to avoid changing p-l-b, but might incur additional performance costs, as well as maintenance overhead. This is likely not a blocker, but might be relevant as this dispatching would be required on a relatively "hot" path (ProxyHttp::upstream_peer()
).
We could define our own traits that are similar to p-l-b, but are object safe, or work in a way that is suitable for River's needs.
This would make it more challenging to take newer algorithms and options from pingora, as well as making it challenging to upstream any we create for River.
I'll update after speaking with the pingora dev team.
I see you've started work, so I'm interested to hear how the Pingora discussion went and if you've got an idea which direction you're going (no pressure if you're not ready to share yet!)
Hey @mcpherrinm! Thanks for the reminder!
The outcome was noted here: https://github.com/cloudflare/pingora/issues/275#issuecomment-2173725488
But the short answer is "I was too zoomed in" - It actually wasn't necessary to make the P-L-B items object-safe, since we already have type erasure one level up, as pingora takes (type erased) dyn Service
items.
If anyone is interested in how I handle the "trickery" here, the biggest parts are:
First: Adding the generics to the RiverProxyService, here:
Then: A function that picks the right "type" of service based on the appropriate selection algorithm, here:
Neat, thanks!
Short progress update here:
I've gone ahead and merged #46, which corrected the loose ends in #45.
There are still quite a few configurables that need to be "wired up" for health checking and load balancing. I may go ahead and focus on static page hosting to be able to set up a "fleet" of river instances for easier testing.
Closing this for now, I'll track follow-on items in new issues!
The
pingora-load-balancing
crate actually provides three key features related to managing upstreams:river
will be integratingpingora-load-balancing
in order to add these features. This post discusses what does and does not exist today.Load Balancing
Load balancing functionality is largely contained within the
selection
module.There are currently four supported algorthms:
All of these algorithms support weighting of different backends. TODO: I'm unsure how weighting will play with health checks and service discovery
It is likely we will need/want more load balancing algorithms in the future. For example, NGINX supports quite a few.
To begin with, we will want to add support for all four currently supported algorithms, selected on a per-service basis. Configuration will need to include weighting cost for statically selected server lists.
Health Checks
Health check functionality is largely contained within the
health_check
module.There are currently two supported methods of health check:
Both methods have some basic shared configuration, such as hysteresis for healthy/unhealthy determination, as well as the connection information for each peer.
Additionally, HTTP health checks have a quite a few more options, including what to request from the server, how to validate the response, and additional checks like whether a TCP/TLS connection can be re-used (this makes checking more efficient, but can mask firewall or routing issues).
We will want to support both options, some effort is expected for determining reasonable configuration APIs for the health checks.
Service Discovery
Service discovery functionality is largely contained within the
discovery
module.Currently, the only provided algorithm is "Static", which uses a fixed array of backends (e.g. "no discovery").
There does exist a
ServiceDiscovery
trait that allows for implementing and providing service discovery functionality.We have a couple of required service discovery abilities in
river
's requirements. We may want to implement DNS-SD or polling of SRV records, though that may not occur during the initial implementation.Summary
This tracking issue will likely cover multiple PRs for implementing this functionality. We'll need to design quite a bit of configuration surface for these new features, and it will likely be a breaking change from the existing "one listener" configuration file.
We should also cross check the requirements after implementation that pingora-load-balancing has sufficient API surface to allow for the things we've mentioned, such as DNS TTL tracking to invalidate hosts.