search

bufbuild / httplb

Client-side load balancing for net/http
https://pkg.go.dev/github.com/bufbuild/httplb
Apache License 2.0
48 stars 2 forks source link

Allow configuring the definition of "warm" #34

Closed jhump closed 1 year ago

jhump commented 1 year ago

Resolves TCN-1919

linear[bot] commented 1 year ago
TCN-1919 Add ability to configure the definition of "warm"

Currently, a client is "warm" if **all** leaf transports are pre-warmed (currently a no-op… but in the future if not a no-op, it would just make sure a connection is actually established) and each transport pool believes it has at least one healthy (or "healthy enough"/usable) connection. Ideally, we'd have configuration of "at least N healthy", that can be configured on a per-target-backend basis. The balancer would have to be the one to implement the check, and that means we'd need to move pre-warming of leaf transports from the `http.transportPool` and into the `balancer.defaultBalancer`, so it can apply the same configuration, and only pre-warm the leaf transports necessary per the "warm" config.

jhump commented 1 year ago

Oof, I think I understand the test failure. Hold off on review until I can fix it.

jhump commented 1 year ago

Oof, I think I understand the test failure. Hold off on review until I can fix it.

As the connections warm up, the picker could be re-created. Since the round robin picker always shuffles (and, even if it didn't, since its index would reset each time), it's possible that concurrent changes of the picker could lead to a result where we don't observe perfect round-robin, distributing load uniformly to the different backends. I'm pretty sure that's what happened in the failing test.

So I just pushed a commit so that we don't bother creating a new picker if the set of usable connections doesn't actually change.