Hello!
On Fri, Sep 06, 2024 at 11:23:03AM +0000, PR Bot wrote:
> From d9d994eb1c6615b1f7ce7a0493be669ad8bb4ab0 Mon Sep 17 00:00:00 2001
> From: shakedm <shakedmig...@gmail.com>
> Date: Fri, 6 Sep 2024 13:38:25 +0300
> Subject: [PATCH] fix a BUG in stream.c where counters will zero because of
> failed updates
>
> The current code only checked if bytes is initialized but during monitoring I
> found many instances in which the metrics will randomly drop to zero.
> this fix handles that scenario. I tested it on compiling and running the new
> binary and seems like it fixed the problem.
I disagree, it tests if bytes is non-zero, which is basically what you're
doing as well, look below:
> @@ -792,7 +792,7 @@ void stream_process_counters(struct stream *s)
>
> bytes = s->req.total - s->logs.bytes_in;
> s->logs.bytes_in = s->req.total;
> - if (bytes) {
> + if (((signed long long) bytes) > 0) {
> _HA_ATOMIC_ADD(&sess->fe->fe_counters.bytes_in, bytes);
> _HA_ATOMIC_ADD(&s->be->be_counters.bytes_in, bytes);
The code checks if s->req.total changed since last time it was copied
into s->logs.bytes_in, and copies it there each time.
Your change istesting that it changed positively, but given that these
are 64-bit numbers, it's practically impossible to get negative numbers
here in one call. And I would even argue that the current test is better
since it could actually detect any change (even those large impractical
ones). So there's no problem.
You said you found instances where metrics will randomly drop to zero.
Did you find them when reading the code, or did you observe them yourself?
If the latter, then you've observed something else. The vast majority of
us are already monitoring stats, so if you've encountered a case where
such a problem happens, we'll need your config, version, and to know all
the specificities of your environment to figure how this could happen at
all.
Just thinking loud, are you sure that you're not experiencing reloads?
A reload starts a new process, and stats will restart from zero. In 3.1
there is a new option to restart the process from the current stats
counters, you might be interested in using this if that's what you're
facing.
Thanks!
Willy
