Hi Alexander,
On Thu, Jul 16, 2026 at 02:50:38PM +0000, Stephan, Alexander wrote:
> Hi list,
>
> This series adds a 'clear counters server <backend>/<server>' CLI command to
> reset the statistics counters of a single server at runtime, without touching
> the rest of the proxy.
OK!
> The motivation is the same operational context as my earlier 'set server
> name' series [1] (thanks again, Amaury, for the review): in Kubernetes-style
> setups a server slot is recycled and renamed to point at a fresh endpoint
> ('set server <b>/<s> name'), but the old cumulative counters stay attached to
> the slot and pollute the new entity's attribution. Being able to clear just
> that server's counters after a rename gives clean per-entity stats without a
> reload.
While I do see the value in being able to clear an individual server's
counters, I'd like to point out that once shared stats are enabled, it
becomes a bad idea to do so because the purpose of the shared stats is
for monitoring tools to see monotonic updates and no longer deal with
jumps. I'm wondering if we should not add an option like "force" to force
clearing such a server's counters when it's registered in a shared stats
memory to avoid accidents.
> While implementing this I found and fixed a pre-existing bug, so the first
> patch is a BUG/MEDIUM that stands on its own and is a backport candidate:
>
> 1. BUG/MEDIUM: proxy_stats_clear_counters() did a blanket memset() on
> the counter structs for 'clear counters all'. Since the switch to
> per-thread-group shared counters that also zeroed the shared.tg
> pointer array, which the hot path dereferences with no NULL check ->
> segfault on the next request after the clear. It also silently failed
> to reset the cumulative counters. Replaced with helpers that preserve
> the pointer array and reset only the contents. Includes a regtest
> that reproduces the segfault.
Ahah good point, I'm not even surprised (I didn't remmber that we'd have
placed a pointer there but the consequence does make sense). It's a bit
annoying because for ~20 years we managed to keep this area memset-safe
so as to be certain never to miss a field when we add more, and it means
it's no longer the case. Maybe we'd have to move that pointer somewhere
else, I don't know (didn't check the code).
> 2. MINOR: factor the max-only reset (plain 'clear counters', OPER level)
> into counters_{fe,be}_reset_max() helpers, removing the open-coded
> inline field lists. While consolidating, the backend and server
> branches turned out to clear different subsets of the (identical)
> struct; the helper now clears the union, closing those latent gaps.
> No functional change for the 'all' path.
Hmmm note that "clear counters" only clears max and requires oper level
while "clear counters all" also clears other counters and requires admin
level. We'd need to preserve this. One reason for this distinction was
to make sure that someone with oper-only level couldn't for example try
to hide their activity by clearing cumulative counters.
> Now the patches that are actually related to the new feature:
>
> 3. MINOR: the new command itself. OPER level, opt-in nothing, takes a
> SERVER_LOCK, resets both the native be_counters and the clearable
> module-registered extra counters, and leaves runtime state (address,
> weight, admin state) and health checks untouched.
>
> 4. REGTESTS: drives traffic pinned to one server (a second, backup
> server stays idle), clears only the served server, and checks via
> 'show stat' that its sessions dropped to zero while the backup is
> untouched.
>
> 5. DOC: management.txt entry alongside 'clear counters' / 'clear
> counters all'.
>
> The series builds at every step and both regtests pass. You can find it with
> the CI in this PR: https://github.com/sap-contributions/haproxy/pull/19
>
> Curious to know what you think! This would also be part of
> https://github.com/jcmoraisjr/haproxy-ingress/pull/1481 and finalizes the
> server rename support of the ingress-controller.
>
> Best regards,
>
> Alexander
>
> [1] https://www.mail-archive.com/[email protected]/msg47211.html
>
>
I'm generally fine with the feature (have not checked the code now for
being in a train) but if you could confirm/deny/discuss the points
above, that would be great!
Thanks,
Willy
