]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MINOR: stats: fix STAT_STARTED behavior with full htx
authorAurelien DARRAGON <adarragon@haproxy.com>
Thu, 2 Feb 2023 18:01:02 +0000 (19:01 +0100)
committerChristopher Faulet <cfaulet@haproxy.com>
Mon, 6 Feb 2023 06:53:03 +0000 (07:53 +0100)
When stats_putchk() fails to peform the dump because available data space in
htx is less than the number of bytes pending in the dump buffer, we wait
for more room in the htx (ie: sc_need_room()) to retry the dump attempt
on the next applet invocation.

To provide consistent output, we have to make sure that the stat ctx is not
updated (or at least correctly reverted) in case stats_putchk() fails so
that the new dumping attempt behaves just like the previous (failed) one.

STAT_STARTED is not following this logic, the flag is set in
stats_dump_fields_json() as soon as some data is written to the output buffer.

It's done too early: we need to delay this step after the stats_putchk() has
successfully returned if we want to correctly handle the retries attempts.

Because of this, JSON output could suffer from extraneous ',' characters which
could make json parsers unhappy.

For example, this is the kind of errors you could get when using
`python -m json.tool` on such badly formatted outputs:

   "Expecting value: line 1 column 2 (char 1)"

Unfortunately, fixing this means that the flag needs to be enabled at
multiple places, which is what we're doing in this patch.
(in stats_dump_proxy_to_buffer() where stats_dump_one_line() is involved
by underlying stats_dump_{fe,li,sv,be} functions)

Thereby, this raises the need for a cleanup to reduce code duplication around
stats_dump_proxy_to_buffer() function and simplify things a bit.

It could be backported to 2.6 and 2.7

src/stats.c

index 7bd42ed96143cbe91d403a8da5c3ac439e8cc365..62e372a7ba6549319b04a9a38891ae554cf7c72c 100644 (file)
@@ -1690,9 +1690,6 @@ int stats_dump_one_line(const struct field *stats, size_t stats_count,
        else
                ret = stats_dump_fields_csv(&trash_chunk, stats, stats_count, ctx);
 
-       if (ret)
-               ctx->flags |= STAT_STARTED;
-
        return ret;
 }
 
@@ -3209,6 +3206,7 @@ more:
                if (stats_dump_fe_stats(sc, px)) {
                        if (!stats_putchk(rep, htx))
                                goto full;
+                       ctx->flags |= STAT_STARTED;
                        if (ctx->field)
                                goto more;
                }
@@ -3246,6 +3244,7 @@ more:
                        if (stats_dump_li_stats(sc, px, l)) {
                                if (!stats_putchk(rep, htx))
                                        goto full;
+                               ctx->flags |= STAT_STARTED;
                                if (ctx->field)
                                        goto more;
                        }
@@ -3311,6 +3310,7 @@ more:
                        if (stats_dump_sv_stats(sc, px, sv)) {
                                if (!stats_putchk(rep, htx))
                                        goto full;
+                               ctx->flags |= STAT_STARTED;
                                if (ctx->field)
                                        goto more;
                        }
@@ -3325,6 +3325,7 @@ more:
                if (stats_dump_be_stats(sc, px)) {
                        if (!stats_putchk(rep, htx))
                                goto full;
+                       ctx->flags |= STAT_STARTED;
                        if (ctx->field)
                                goto more;
                }