Hi,
consistent-hash and "balance random" load balancing pick a natural
target server on the ring, then silently fall back to another one
when that target can't be used: a previous connect attempt failed and
"option redispatch" kicked in, "hash-balance-factor" judged it over
its load quota, or "hash-preserve-affinity maxconn/maxqueue" judged it
saturated. This is a signal we want to track as a standing operational
metric.
This series adds that visibility: a single "lbbypass" stats counter
(incremented on both the bypassed server and the backend, mirroring
the existing retries/redispatches pattern) plus a per-request
"txn.lb_bypass_reason" sample fetch usable in log-format/ACLs. Full
rationale and implementation details are in patch 1's commit message;
patch 2 adds reg-test coverage for both.
Sending as RFC because this touches chash_get_server_hash() and 8
call sites in backend.c, and I'd like feedback before polishing
further, specifically on:
- Shape: chash_get_server_hash() and the get_server_*() wrappers
each gain a trailing "enum lb_bypass_reason *reason_out"
out-parameter. Is this the preferred way to surface this, or
would you rather see it handled differently (e.g. stashed
directly on the stream from within lb_chash.c, or exposed via
the existing trace/term_events mechanism instead of a new
parameter threaded through 8 call sites)?
- Scope: this intentionally does NOT detect a naive target being
skipped for being in drain mode or marked unhealthy, since such
servers are removed from the ring entirely and there's nothing
left to compare against without a second, always-populated
shadow ring. Is landing the cheaply-detectable reasons now,
with drain/unhealthy detection as a documented follow-up,
reasonable, or should that gap block this series?
- Counter granularity: one aggregate "lbbypass" counter rather
than one per reason, matching an explicit request to keep this
simple. Would a per-reason breakdown be preferred instead?
- Naming: "lbbypass" for the stats column, "txn.lb_bypass_reason"
for the sample fetch, and "redispatch" / "hash-balance-factor" /
"preserve-affinity-maxconn" / "preserve-affinity-maxqueue" for
its possible values. Open to better names for any of these.
Built and tested on macOS/arm64 (TARGET=osx); reg-tests pass against
a locally built vtest, including the two new ones and the existing
reg-tests/balance/ and reg-tests/log/ suites.
Thanks for taking a look.
David Birdsong (2):
MEDIUM: backend: report consistent-hash LB bypass reason
REGTEST: balance: add coverage for lb_bypass counter and reason
doc/configuration.txt | 27 ++++-
doc/management.txt | 3 +
include/haproxy/backend-t.h | 14 +++
include/haproxy/backend.h | 24 +++--
include/haproxy/counters-t.h | 1 +
include/haproxy/lb_chash.h | 3 +-
include/haproxy/stats-file-t.h | 2 +-
include/haproxy/stats-t.h | 1 +
include/haproxy/stream-t.h | 1 +
.../balance/balance-hash-bypass-maxconn.vtc | 101 ++++++++++++++++++
.../balance-hash-bypass-redispatch.vtc | 71 ++++++++++++
src/backend.c | 79 +++++++++-----
src/lb_chash.c | 46 +++++++-
src/log.c | 2 +-
src/stats-proxy.c | 1 +
src/stream.c | 27 +++++
16 files changed, 360 insertions(+), 43 deletions(-)
create mode 100644 reg-tests/balance/balance-hash-bypass-maxconn.vtc
create mode 100644 reg-tests/balance/balance-hash-bypass-redispatch.vtc
--
2.52.0
