Hi Willy, Thanks a lot for the quick read despite being on a train — much appreciated!
1.
Good catch, I hadn't considered the monotonicity contract that shared stats
provide to monitoring tools. I agree that silently clearing a server that's
registered in a shared-memory stats object is a footgun for sure. I'll add a
force option, so the command refuses by default when the counters aren't
process-local (i.e. !(shared.flags & COUNTERS_SHARED_F_LOCAL)) and only clears
such a server when the operator explicitly passes clear counters server <b>/<s>
force. The regular process-local counters stay clearable without force.
2.
Yes, the blanket-memset was indeed quite convenient. I investigated this and I
don't think a code change is needed here. The full-reset helpers
(counters_fe_reset / counters_be_reset) already preserve that self-maintaining
property: shared is the first member, and we now skip it like this:
memset((char *)counters + sizeof(counters->shared), 0,
sizeof(*counters) - sizeof(counters->shared));
Only danger is that somebody adds new fields before the pointer, in
the beginning of the struct. I added a comment, so we can be confident that
this invariant will stay enforced.
The helper method itself seems unavoidable since we need to clear
the struct behind the shared.tg pointer as well. I also thought about moving
the ptr to the end of the struct, but I didn’t want to change field order, if
not required.
1.
Ah, yes, my bad... It now requires ACCESS_LVL_ADMIN, matching clear counters
all. I updated the Doc and the Regtest to reflect this.
I attached the updated patch series.
Cheers,
Alexander
From: Willy Tarreau <[email protected]>
Date: Thursday, 16. July 2026 at 20:02
To: Stephan, Alexander <[email protected]>
Cc: [email protected] <[email protected]>; Amaury Denoyelle
<[email protected]>
Subject: Re: [PATCH] server: add 'clear counters server' command to reset a
single server's stats
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://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fsap-contributions%2Fhaproxy%2Fpull%2F19&data=05%7C02%7Calexander.stephan%40sap.com%7C67afc8c791aa4cbe297008dee36456e1%7C42f7676cf455423c82f6dc2d99791af7%7C0%7C0%7C639198217318626036%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=EACMisgZbrzH7svejgTYEkDT%2BVNnkEQuGjRYNM1VPps%3D&reserved=0<https://github.com/sap-contributions/haproxy/pull/19>
>
> Curious to know what you think! This would also be part of
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fjcmoraisjr%2Fhaproxy-ingress%2Fpull%2F1481&data=05%7C02%7Calexander.stephan%40sap.com%7C67afc8c791aa4cbe297008dee36456e1%7C42f7676cf455423c82f6dc2d99791af7%7C0%7C0%7C639198217318680271%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=yYzYE9XDdxSp3L%2FO0S4cSKlHHo%2Fh8pb0wFe732I5jWc%3D&reserved=0<https://github.com/jcmoraisjr/haproxy-ingress/pull/1481>
> and finalizes the
> server rename support of the ingress-controller.
>
> Best regards,
>
> Alexander
>
> [1]
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.mail-archive.com%2Fhaproxy%40formilux.org%2Fmsg47211.html&data=05%7C02%7Calexander.stephan%40sap.com%7C67afc8c791aa4cbe297008dee36456e1%7C42f7676cf455423c82f6dc2d99791af7%7C0%7C0%7C639198217318759725%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=1evu6T%2FFNd9OEpE7jwwXwn4lw%2F0kMnSknMJWrZYCiSA%3D&reserved=0<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
0001-BUG-MEDIUM-counters-preserve-shared.tg-pointer-on-cl.patch
Description: 0001-BUG-MEDIUM-counters-preserve-shared.tg-pointer-on-cl.patch
0002-MINOR-counters-add-max-only-reset-helpers-and-use-th.patch
Description: 0002-MINOR-counters-add-max-only-reset-helpers-and-use-th.patch
0003-MINOR-server-add-clear-counters-server-backend-serve.patch
Description: 0003-MINOR-server-add-clear-counters-server-backend-serve.patch
0004-REGTESTS-stats-add-test-for-clear-counters-server.patch
Description: 0004-REGTESTS-stats-add-test-for-clear-counters-server.patch
0005-DOC-management-document-clear-counters-server.patch
Description: 0005-DOC-management-document-clear-counters-server.patch
