MEDIUM: connection: Add private connections synchronously in session server list
When a connection is marked as private, it is now added in the session server
list. We don't wait a stream is detached from the mux to do so. When the
connection is created, this happens after the mux creation. Otherwise, it is
performed when the connection is marked as private.
To allow that, when a connection is created, the session is systematically set
as the connectin owner. Thus, a backend connection has always a owner during its
creation. And a private connection has always a owner until its death.
Note that outside the detach() callback, if the call to session_add_conn()
failed, the error is ignored. In this situation, we retry to add the connection
into the session server list in the detach() callback. If this fails at this
step, the multiplexer is destroyed and the connection is closed.
MINOR: connection: Add a wrapper to mark a connection as private
To set a connection as private, the conn_set_private() function must now be
called. It sets the CO_FL_PRIVATE flags, but it also remove the connection from
the available connection list, if necessary. For now, it never happens because
only HTTP/1 connections may be set as private after their creation. And these
connections are never inserted in the available connection list.
MINOR: connection: Set new connection as private on reuse never
When a new connection is created, it may immediatly be set as private if
http-reuse never is configured for the backend. There is no reason to wait the
call to mux->detach() to do so.
MINOR: connection: Set the SNI on server connections before installing the mux
If an expression is configured to set the SNI on a server connection, the
connection is marked as private. To not needlessly add it in the available
connection list when the mux is installed, the SNI is now set on the connection
before installing the mux, just after the call to si_connect().
BUG/MEDIUM: mux-fcgi: Don't add private connections in available connection list
When a stream is detached from a backend private connection, we must not insert
it in the available connection list. In addition, we must be sure to remove it
from this list. To ensure it is properly performed, this part has been slightly
refactored to clearly split processing of private connections from the others.
BUG/MEDIUM: mux-h2: Don't add private connections in available connection list
When a stream is detached from a backend private connection, we must not insert
it in the available connection list. In addition, we must be sure to remove it
from this list. To ensure it is properly performed, this part has been slightly
refactored to clearly split processing of private connections from the others.
CI: travis-ci: speed up osx build by running brew scripted, switch to latest osx image
we used to use travis-ci brew plugin to install "socat", travis-ci brew
plugin works predictable in "all update" mode. sometimes it might take 12 minutes.
let us improve developer velocity by running brew from command line. It takes 2 minutes
instead of 12 minutes
latest osx seems to have more stable brew, let us switch to latest osx available.
osx images list: https://docs.travis-ci.com/user/reference/osx/#macos-version
CONTRIB: da: fix memory leak in dummy function da_atlas_open()
The dummy function takes care of doing a bit of work using a malloc()
to avoid returning a constant but it doesn't free the tested pointer,
which coverity noticed in issue #741. Let's free it before testing it
for the return value.
This may be backported but is not important since this code is only
present to allow to build the device detection code and not to actually
run it.
MINOR: tasks: use MT_LIST_ADDQ() when killing tasks.
A bug in task_kill() was fixed by commy 54d31170a ("BUG/MAJOR: sched:
make sure task_kill() always queues the task") which added a list
initialization before adding an element. But in fact an inconditional
addition would have done the same and been simpler than first
initializing then checking the element was initialized. Let's use
MT_LIST_ADDQ() there to add the task to kill into the shared queue
and kill the dirty LIST_INIT().
MINOR: connection: use MT_LIST_ADDQ() to add connections to idle lists
When a connection is added to an idle list, it's already detached and
cannot be seen by two threads at once, so there's no point using
TRY_ADDQ, there will never be any conflict. Let's just use the cheaper
ADDQ.
MINOR: buffer: use MT_LIST_ADDQ() for buffer_wait lists additions
The TRY_ADDQ there was not needed since the wait list is exclusively
owned by the caller. There's a preliminary test on MT_LIST_ADDED()
that might have been eliminated by keeping MT_LIST_TRY_ADDQ() but
it would have required two more expensive writes before testing so
better keep the test the way it is.
MINOR: lists: rename some MT_LIST operations to clarify them
Initially when mt_lists were added, their purpose was to be used with
the scheduler, where anyone may concurrently add the same tasklet, so
it sounded natural to implement a check in MT_LIST_ADD{,Q}. Later their
usage was extended and MT_LIST_ADD{,Q} started to be used on situations
where the element to be added was exclusively owned by the one performing
the operation so a conflict was impossible. This became more obvious with
the idle connections and the new macro was called MT_LIST_ADDQ_NOCHECK.
But this remains confusing and at many places it's not expected that
an MT_LIST_ADD could possibly fail, and worse, at some places we start
by initializing it before adding (and the test is superflous) so let's
rename them to something more conventional to denote the presence of the
check or not:
MT_LIST_ADD{,Q} : inconditional operation, the caller owns the
element, and doesn't care about the element's
current state (exactly like LIST_ADD)
MT_LIST_TRY_ADD{,Q}: only perform the operation if the element is not
already added or in the process of being added.
This means that the previously "safe" MT_LIST_ADD{,Q} are not "safe"
anymore. This also means that in case of backport mistakes in the
future causing this to be overlooked, the slower and safer functions
will still be used by default.
Note that the missing unchecked MT_LIST_ADD macro was added.
The rest of the code will have to be reviewed so that a number of
callers of MT_LIST_TRY_ADDQ are changed to MT_LIST_ADDQ to remove
the unneeded test.
BUILD: tcp: condition TCP keepalive settings to platforms providing them
Previous commit b24bc0d ("MINOR: tcp: Support TCP keepalive parameters
customization") broke non-Linux builds as TCP_KEEP{CNT,IDLE,INTVL} are
not necessarily defined elsewhere.
This patch adds the required #ifdefs to condition the visibility of the
keywords, and adds a mention in the doc about their dependency on Linux.
MINOR: tcp: Support TCP keepalive parameters customization
It is now possible to customize TCP keepalive parameters.
These correspond to the socket options TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL
and are valid for the defaults, listen, frontend and backend sections.
BUG/MEDIUM: lists: add missing store barrier in MT_LIST_ADD/MT_LIST_ADDQ
The torture test run for previous commit 787dc20 ("BUG/MEDIUM: lists: add
missing store barrier on MT_LIST_BEHEAD()") finally broke again after 34M
connections. It appeared that MT_LIST_ADD and MT_LIST_ADDQ were suffering
from the same missing barrier when restoring the original pointers before
giving up, when checking if the element was already added. This is indeed
something which seldom happens with the shared scheduler, in case two
threads simultaneously try to wake up the same tasklet.
With a store barrier there after reverting the pointers, the torture test
survived 750M connections on the NanoPI-Fire3, so it looks good this time.
Probably that MT_LIST_BEHEAD should be added to test-list.c since it seems
to be more sensitive to concurrent accesses with MT_LIST_ADDQ.
It's worth noting that there is no barrier between the last two pointers
update, while there is one in MT_LIST_POP and MT_LIST_BEHEAD, the latter
having shown to be needed, but I cannot demonstrate why we would need
one here. Given that the code seems solid here, let's stick to what is
shown to work.
This fix should be backported to 2.1, just for the sake of safety since
the issue couldn't be triggered there, but it could change with the
compiler or when backporting a fix for example.
BUG/MEDIUM: lists: add missing store barrier on MT_LIST_BEHEAD()
When running multi-threaded tests on my NanoPI-Fire3 (8 A53 cores), I
managed to occasionally get either a bus error or a segfault in the
scheduler, but only when running at a high connection rate (injecting
on a tcp-request connection reject rule). The bug is rare and happens
around once per million connections. I could never reproduce it with
less than 4 threads nor on A72 cores.
Haproxy 2.1.0 would also fail there but not 2.1.7.
Every time the crash happened with the TL_URGENT task list corrupted,
though it was not immediately after the LIST_SPLICE() call, indicating
background activity survived the MT_LIST_BEHEAD() operation. This queue
is where the shared runqueue is transferred, and the shared runqueue
gets fast inter-thread tasklet wakeups from idle conn takeover and new
connections.
Comparing the MT_LIST_BEHEAD() and MT_LIST_DEL() implementations, it's
quite obvious that a few barriers are missing from the former, and these
will simply fail on weakly ordered caches.
Two store barriers were added before the break() on failure, to match
what is done on the normal path. Missing them almost always results in
a segfault which is quite rare but consistent (after ~3M connections).
The 3rd one before updating n->prev seems intuitively needed though I
coudln't make the code fail without it. It's present in MT_LIST_DEL so
better not be needlessly creative. The last one is the most important
one, and seems to be the one that helps a concurrent MT_LIST_ADDQ()
detect a late failure and try again. With this, the code survives at
least 30M connections.
Interestingly the exact same issue was addressed in 2.0-dev2 for MT_LIST_DEL
with commit 690d2ad4d ("BUG/MEDIUM: list: add missing store barriers when
updating elements and head").
This fix must be backported to 2.1 as MT_LIST_BEHEAD() is also used there.
It's only tagged as medium because it will only affect entry-level CPUs
like Cortex A53 (x86 are not affected), and requires load levels that are
very hard to achieve on such machines to trigger it. In practice it's
unlikely anyone will ever hit it.
Tim Duesterhus [Sat, 4 Jul 2020 09:53:26 +0000 (11:53 +0200)]
CLEANUP: Add static void hlua_deinit()
Compiling HAProxy with USE_LUA=1 and running a configuration check within
valgrind with a very simple configuration such as:
listen foo
bind *:8080
Will report quite a few possible leaks afterwards:
==24048== LEAK SUMMARY:
==24048== definitely lost: 0 bytes in 0 blocks
==24048== indirectly lost: 0 bytes in 0 blocks
==24048== possibly lost: 95,513 bytes in 1,209 blocks
==24048== still reachable: 329,960 bytes in 71 blocks
==24048== suppressed: 0 bytes in 0 blocks
Printing these possible leaks shows that all of them are caused by Lua.
Luckily Lua makes it *very* easy to free all used memory, so let's do
this on shutdown.
Afterwards this patch is applied the output looks much better:
==24199== LEAK SUMMARY:
==24199== definitely lost: 0 bytes in 0 blocks
==24199== indirectly lost: 0 bytes in 0 blocks
==24199== possibly lost: 0 bytes in 0 blocks
==24199== still reachable: 329,960 bytes in 71 blocks
==24199== suppressed: 0 bytes in 0 blocks
Tim Duesterhus [Sat, 4 Jul 2020 09:49:46 +0000 (11:49 +0200)]
BUG/MINOR: sample: Free str.area in smp_check_const_meth
Given the following example configuration:
listen foo
mode http
bind *:8080
http-request set-var(txn.leak) meth(GET)
server x example.com:80
Running a configuration check with valgrind reports:
==25992== 4 bytes in 1 blocks are definitely lost in loss record 1 of 344
==25992== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==25992== by 0x4E239D: my_strndup (tools.c:2261)
==25992== by 0x581E20: make_arg_list (arg.c:253)
==25992== by 0x4DE91D: sample_parse_expr (sample.c:890)
==25992== by 0x58E304: parse_store (vars.c:772)
==25992== by 0x566A3F: parse_http_req_cond (http_rules.c:95)
==25992== by 0x4A4CE6: cfg_parse_listen (cfgparse-listen.c:1339)
==25992== by 0x494C59: readcfgfile (cfgparse.c:2049)
==25992== by 0x545145: init (haproxy.c:2029)
==25992== by 0x421E42: main (haproxy.c:3175)
After this patch is applied the leak is gone as expected.
This is a fairly minor leak, but it can add up for many uses of the `bool()`
sample fetch. The bug most likely exists since the `bool()` sample fetch was
introduced in commit cc103299c75c530ab3637a1698306145bdc85552. The fix may
be backported to HAProxy 1.6+.
Tim Duesterhus [Sat, 4 Jul 2020 09:49:45 +0000 (11:49 +0200)]
BUG/MINOR: sample: Free str.area in smp_check_const_bool
Given the following example configuration:
listen foo
mode http
bind *:8080
http-request set-var(txn.leak) bool(1)
server x example.com:80
Running a configuration check with valgrind reports:
==24233== 2 bytes in 1 blocks are definitely lost in loss record 1 of 345
==24233== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==24233== by 0x4E238D: my_strndup (tools.c:2261)
==24233== by 0x581E10: make_arg_list (arg.c:253)
==24233== by 0x4DE90D: sample_parse_expr (sample.c:890)
==24233== by 0x58E2F4: parse_store (vars.c:772)
==24233== by 0x566A2F: parse_http_req_cond (http_rules.c:95)
==24233== by 0x4A4CE6: cfg_parse_listen (cfgparse-listen.c:1339)
==24233== by 0x494C59: readcfgfile (cfgparse.c:2049)
==24233== by 0x545135: init (haproxy.c:2029)
==24233== by 0x421E42: main (haproxy.c:3175)
After this patch is applied the leak is gone as expected.
This is a fairly minor leak, but it can add up for many uses of the `bool()`
sample fetch. The bug most likely exists since the `bool()` sample fetch was
introduced in commit cc103299c75c530ab3637a1698306145bdc85552. The fix may
be backported to HAProxy 1.6+.
Tim Duesterhus [Sat, 4 Jul 2020 09:49:44 +0000 (11:49 +0200)]
BUG/MINOR: haproxy: Free srule->expr during deinit
Given the following example configuration:
backend foo
mode http
use-server %[str(x)] if { always_true }
server x example.com:80
Running a configuration check with valgrind reports:
==19376== 170 (40 direct, 130 indirect) bytes in 1 blocks are definitely lost in loss record 281 of 347
==19376== at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19376== by 0x5091AC: add_sample_to_logformat_list (log.c:511)
==19376== by 0x50A5A6: parse_logformat_string (log.c:671)
==19376== by 0x4957F2: check_config_validity (cfgparse.c:2588)
==19376== by 0x54442D: init (haproxy.c:2129)
==19376== by 0x421E42: main (haproxy.c:3169)
After this patch is applied the leak is gone as expected.
This is a very minor leak that can only be observed if deinit() is called,
shortly before the OS will free all memory of the process anyway. No
backport needed.
Tim Duesterhus [Sat, 4 Jul 2020 09:49:43 +0000 (11:49 +0200)]
BUG/MINOR: haproxy: Free srule->file during deinit
Given the following example configuration:
backend foo
mode http
use-server x if { always_true }
server x example.com:80
Running a configuration check with valgrind reports:
==18650== 14 bytes in 1 blocks are definitely lost in loss record 3 of 345
==18650== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18650== by 0x649E489: strdup (strdup.c:42)
==18650== by 0x4A5438: cfg_parse_listen (cfgparse-listen.c:1548)
==18650== by 0x494C59: readcfgfile (cfgparse.c:2049)
==18650== by 0x5450B5: init (haproxy.c:2029)
==18650== by 0x421E42: main (haproxy.c:3168)
After this patch is applied the leak is gone as expected.
This is a very minor leak that can only be observed if deinit() is called,
shortly before the OS will free all memory of the process anyway. No
backport needed.
Tim Duesterhus [Sat, 4 Jul 2020 09:49:42 +0000 (11:49 +0200)]
BUG/MINOR: haproxy: Free proxy->unique_id_header during deinit
Given the following example configuration:
frontend foo
mode http
bind *:8080
unique-id-header x
Running a configuration check with valgrind reports:
==17621== 2 bytes in 1 blocks are definitely lost in loss record 1 of 341
==17621== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==17621== by 0x649E489: strdup (strdup.c:42)
==17621== by 0x4A87F1: cfg_parse_listen (cfgparse-listen.c:2747)
==17621== by 0x494C59: readcfgfile (cfgparse.c:2049)
==17621== by 0x545095: init (haproxy.c:2029)
==17621== by 0x421E42: main (haproxy.c:3167)
After this patch is applied the leak is gone as expected.
This is a very minor leak that can only be observed if deinit() is called,
shortly before the OS will free all memory of the process anyway. No
backport needed.
Tim Duesterhus [Sat, 4 Jul 2020 09:49:41 +0000 (11:49 +0200)]
BUG/MINOR: haproxy: Add missing free of server->(hostname|resolvers_id)
Given the following example configuration:
resolvers test
nameserver test 127.0.0.1:53
listen foo
bind *:8080
server foo example.com resolvers test
Running a configuration check within valgrind reports:
==21995== 5 bytes in 1 blocks are definitely lost in loss record 1 of 30
==21995== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==21995== by 0x5726489: strdup (strdup.c:42)
==21995== by 0x4B2CFB: parse_server (server.c:2163)
==21995== by 0x4680C1: cfg_parse_listen (cfgparse-listen.c:534)
==21995== by 0x459E33: readcfgfile (cfgparse.c:2167)
==21995== by 0x50778D: init (haproxy.c:2021)
==21995== by 0x418262: main (haproxy.c:3133)
==21995==
==21995== 12 bytes in 1 blocks are definitely lost in loss record 3 of 30
==21995== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==21995== by 0x5726489: strdup (strdup.c:42)
==21995== by 0x4AC666: srv_prepare_for_resolution (server.c:1606)
==21995== by 0x4B2EBD: parse_server (server.c:2081)
==21995== by 0x4680C1: cfg_parse_listen (cfgparse-listen.c:534)
==21995== by 0x459E33: readcfgfile (cfgparse.c:2167)
==21995== by 0x50778D: init (haproxy.c:2021)
==21995== by 0x418262: main (haproxy.c:3133)
with one more leak unrelated to `struct server`. After applying this
patch the leak is gone as expected.
This is a very minor leak that can only be observed if deinit() is called,
shortly before the OS will free all memory of the process anyway. No
backport needed.
Tim Duesterhus [Sat, 4 Jul 2020 09:49:40 +0000 (11:49 +0200)]
BUG/MINOR: haproxy: Free proxy->format_unique_id during deinit
Given the following example configuration:
frontend foo
mode http
bind *:8080
unique-id-format x
Running a configuration check with valgrind reports:
==30712== 42 (40 direct, 2 indirect) bytes in 1 blocks are definitely lost in loss record 18 of 39
==30712== at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30712== by 0x4ED7E9: add_to_logformat_list (log.c:462)
==30712== by 0x4EEE28: parse_logformat_string (log.c:720)
==30712== by 0x47B09A: check_config_validity (cfgparse.c:3046)
==30712== by 0x52881D: init (haproxy.c:2121)
==30712== by 0x41F382: main (haproxy.c:3126)
After this patch is applied the leak is gone as expected.
This is a very minor leak that can only be observed if deinit() is called,
shortly before the OS will free all memory of the process anyway. No
backport needed.
Running a configuration check within valgrind reports:
==1431== 32 bytes in 1 blocks are definitely lost in loss record 20 of 43
==1431== at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1431== by 0x4C39B5: sample_parse_expr (sample.c:982)
==1431== by 0x56B410: parse_acl_expr (acl.c:319)
==1431== by 0x56BA7F: parse_acl (acl.c:697)
==1431== by 0x48D225: cfg_parse_listen (cfgparse-listen.c:816)
==1431== by 0x4797C3: readcfgfile (cfgparse.c:2167)
==1431== by 0x52943D: init (haproxy.c:2021)
==1431== by 0x41F382: main (haproxy.c:3133)
After this patch is applied the leak is gone as expected.
This is a fairly minor leak that can only be observed if samples need to be
freed, which is not something that should occur during normal processing and
most likely only during shut down. Thus no backport should be needed.
Running a configuration check within valgrind reports:
==31371== 160 (48 direct, 112 indirect) bytes in 1 blocks are definitely lost in loss record 35 of 45
==31371== at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==31371== by 0x4C3832: sample_parse_expr (sample.c:876)
==31371== by 0x56B3E0: parse_acl_expr (acl.c:319)
==31371== by 0x56BA4F: parse_acl (acl.c:697)
==31371== by 0x48D225: cfg_parse_listen (cfgparse-listen.c:816)
==31371== by 0x4797C3: readcfgfile (cfgparse.c:2167)
==31371== by 0x5293ED: init (haproxy.c:2021)
==31371== by 0x41F382: main (haproxy.c:3126)
After this patch this leak is reduced. It will be fully removed in a
follow up patch:
==32503== 32 bytes in 1 blocks are definitely lost in loss record 20 of 43
==32503== at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==32503== by 0x4C39B5: sample_parse_expr (sample.c:982)
==32503== by 0x56B410: parse_acl_expr (acl.c:319)
==32503== by 0x56BA7F: parse_acl (acl.c:697)
==32503== by 0x48D225: cfg_parse_listen (cfgparse-listen.c:816)
==32503== by 0x4797C3: readcfgfile (cfgparse.c:2167)
==32503== by 0x52943D: init (haproxy.c:2021)
==32503== by 0x41F382: main (haproxy.c:3133)
This is a fairly minor leak that can only be observed if ACLs need to be
freed, which is not something that should occur during normal processing
and most likely only during shut down. Thus no backport should be needed.
Released version 2.3-dev0 with the following main changes :
- [RELEASE] Released version 2.3-dev0
- MINOR: version: back to development, update status message
Released version 2.2.0 with the following main changes :
- BUILD: mux-h2: fix typo breaking build when using DEBUG_LOCK
- CLEANUP: makefile: update the outdated list of DEBUG_xxx options
- BUILD: tools: make resolve_sym_name() return a const
- CLEANUP: auth: fix useless self-include of auth-t.h
- BUILD: tree-wide: cast arguments to tolower/toupper to unsigned char
- CLEANUP: assorted typo fixes in the code and comments
- WIP/MINOR: ssl: add sample fetches for keylog in frontend
- DOC: fix tune.ssl.keylog sample fetches array
- BUG/MINOR: ssl: check conn in keylog sample fetch
- DOC: configuration: various typo fixes
- MINOR: log: Remove unused case statement during the log-format string parsing
- BUG/MINOR: mux-h1: Fix the splicing in TUNNEL mode
- BUG/MINOR: mux-h1: Don't read data from a pipe if the mux is unable to receive
- BUG/MINOR: mux-h1: Disable splicing only if input data was processed
- BUG/MEDIUM: mux-h1: Disable splicing for the conn-stream if read0 is received
- MINOR: mux-h1: Improve traces about the splicing
- BUG/MINOR: backend: Remove CO_FL_SESS_IDLE if a client remains on the last server
- BUG/MEDIUM: connection: Don't consider new private connections as available
- BUG/MINOR: connection: See new connection as available only on reuse always
- DOC: configuration: remove obsolete mentions of H2 being converted to HTTP/1.x
- CLEANUP: ssl: remove unrelevant comment in smp_fetch_ssl_x_keylog()
- DOC: update INSTALL with new compiler versions
- DOC: minor update to coding style file
- MINOR: version: mention that it's an LTS release now
DOC: configuration: remove obsolete mentions of H2 being converted to HTTP/1.x
The first H2 implementation in version 1.8 used to turn HTTP/2 requests
to HTTP/1.1, causing many limitations. This is not true anymore and we
don't suffer from the lack of server-side H2 nor are we forced to close
mode anymore, so let's remove such obsolete mentions.
BUG/MINOR: connection: See new connection as available only on reuse always
When the multiplexer creation is delayed after the handshakes phase, the
connection is added in the available connection list if http-reuse never is not
configured for the backend. But it is a wrong statement. At this step, the
connection is not safe because it is a new connection. So it must be added in
the available connection list only if http-reuse always is used.
BUG/MEDIUM: connection: Don't consider new private connections as available
When a connection is created and the multiplexer is installed, if the connection
is marked as private, don't consider it as available, regardless the number of
available streams. This test is performed when the mux is installed when the
connection is created, in connect_server(), and when the mux is installed after
the handshakes stage.
BUG/MINOR: backend: Remove CO_FL_SESS_IDLE if a client remains on the last server
When a connection is picked from the session server list because the proxy or
the session are marked to use the last requested server, if it is idle, we must
marked it as used removing the CO_FL_SESS_IDLE flag and decrementing the session
idle_conns counter.
BUG/MEDIUM: mux-h1: Disable splicing for the conn-stream if read0 is received
The CS_FL_MAY_SPLICE flag must be unset for the conn-stream if a read0 is
received while reading on the kernel pipe. It is mandatory when some data was
also received. Otherwise, this flag prevent the call to the h1 rcv_buf()
callback. Thus the read0 will never be handled by the h1 multiplexer leading to
a freeze of the session until a timeout is reached.
BUG/MINOR: mux-h1: Disable splicing only if input data was processed
In h1_rcv_buf(), the splicing is systematically disabled if it was previously
enabled. When it happens, if the splicing is enabled it means the channel's
buffer was empty before calling h1_rcv_buf(). Thus, the only reason to disable
the splicing at this step is when some input data have just been processed.
BUG/MINOR: mux-h1: Don't read data from a pipe if the mux is unable to receive
In h1_rcv_pipe(), if the mux is unable to receive data, for instance because the
multiplexer is blocked on input waiting the other side (BUSY mode), no receive
must be performed.
BUG/MINOR: mux-h1: Fix the splicing in TUNNEL mode
In the commit 17ccd1a35 ("BUG/MEDIUM: connection: add a mux flag to indicate
splice usability"), The CS_FL_MAY_SPLICE flags was added to notify the upper
layer that the mux is able to use the splicing. But this was only done for the
payload in a message, in HTTP_MSG_DATA state. But the splicing is also possible
in TUNNEL mode, in HTTP_MSG_TUNNEL state. In addition, the splicing ability is
always disabled for chunked messages.
MINOR: log: Remove unused case statement during the log-format string parsing
Since the commit cd0d2ed6e ("MEDIUM: log-format: make the LF parser aware of
sample expressions' end"), the LF_STEXPR label in the last switch-case statement
at the end of the for loop in the parse_logformat_string() function cannot be
reached anymore.
WIP/MINOR: ssl: add sample fetches for keylog in frontend
OpenSSL 1.1.1 provides a callback registering function
SSL_CTX_set_keylog_callback, which allows one to receive a string
containing the keys to deciphers TLSv1.3.
Unfortunately it is not possible to store this data in binary form and
we can only get this information using the callback. Which means that we
need to store it until the connection is closed.
This patches add 2 pools, the first one, pool_head_ssl_keylog is used to
store a struct ssl_keylog which will be inserted as a ex_data in a SSL *.
The second one is pool_head_ssl_keylog_str which will be used to store
the hexadecimal strings.
To enable the capture of the keys, you need to set "tune.ssl.keylog on"
in your configuration.
BUILD: tree-wide: cast arguments to tolower/toupper to unsigned char
NetBSD apparently uses macros for tolower/toupper and complains about
the use of char for array subscripts. Let's properly cast all of them
to unsigned char where they are used.
BUILD: tools: make resolve_sym_name() return a const
Originally it was made to return a void* because some comparisons in the
code where it was used required a lot of casts. But now we don't need
that anymore. And having it non-const breaks the build on NetBSD 9 as
reported in issue #728.
So let's switch to const and adjust debug.c to accomodate this.
CLEANUP: makefile: update the outdated list of DEBUG_xxx options
A few options didn't exist anymore (FSM, HASH) and quite a few ones were
added since last update (MEM_STATS, DONT_SHARE_POOLS, NO_LOCKLESS_POOLS,
NO_LOCAL_POOLS, FAIL_ALLOC, STRICT_NOCRASH, HPACK.
BUILD: mux-h2: fix typo breaking build when using DEBUG_LOCK
A typo was accidently introduced in commit 48ce6a3 ("BUG/MEDIUM: muxes:
Make sure nobody stole the connection before using it."), a "&" was
placed in front of "OTHER_LOCK", which breaks DEBUG_LOCK. No backport
is needed.
Released version 2.2-dev12 with the following main changes :
- BUG/MINOR: mux_h2: don't lose the leaving trace in h2_io_cb()
- MINOR: cli: make "show sess" stop at the last known session
- CLEANUP: buffers: remove unused buffer_wq_lock lock
- BUG/MEDIUM: buffers: always allocate from the local cache first
- MINOR: connection: align toremove_{lock,connections} and cleanup into idle_conns
- CONTRIB: debug: add missing flags SI_FL_L7_RETRY & SI_FL_D_L7_RETRY
- BUG/MEDIUM: connections: Don't increase curr_used_conns for shared connections.
- BUG/MEDIUM: checks: Increment the server's curr_used_conns
- REORG: buffer: rename buffer.c to dynbuf.c
- REORG: includes: create tinfo.h for the thread_info struct
- CLEANUP: pool: only include the type files from types
- MINOR: pools: move the LRU cache heads to thread_info
- BUG/MINOR: debug: fix "show fd" null-deref when built with DEBUG_FD
- MINOR: stats: add 3 new output values for the per-server idle conn state
- MINOR: activity: add per-thread statistics on FD takeover
- BUG/MINOR: server: start cleaning idle connections from various points
- MEDIUM: server: improve estimate of the need for idle connections
- MINOR: stats: add the estimated need of concurrent connections per server
- BUG/MINOR: threads: Don't forget to init each thread toremove_lock.
- BUG/MEDIUM: lists: Lock the element while we check if it is in a list.
- Revert "BUG/MEDIUM: lists: Lock the element while we check if it is in a list."
- BUG/MINOR: haproxy: don't wake already stopping threads on exit
- BUG/MINOR: server: always count one idle slot for current thread
- MEDIUM: server: use the two thresholds for the connection release algorithm
- BUG/MINOR: http-rules: Fix ACLs parsing for http deny rules
- BUG/MINOR: sched: properly cover for a rare MT_LIST_ADDQ() race
- MINOR: mux-h1: avoid taking the toremove_lock in on dying tasks
- MINOR: mux-h2: avoid taking the toremove_lock in on dying tasks
- MINOR: mux-fcgi: avoid taking the toremove_lock in on dying tasks
- MINOR: pools: increase MAX_BASE_POOLS to 64
- DOC: ssl: add "allow-0rtt" and "ciphersuites" in crt-list
- BUG/MEDIUM: pattern: Add a trailing \0 to match strings only if possible
- BUG/MEDIUM: log-format: fix possible endless loop in parse_logformat_string()
- BUG/MINOR: proxy: fix dump_server_state()'s misuse of the trash
- BUG/MINOR: proxy: always initialize the trash in show servers state
- MINOR: cli/proxy: add a new "show servers conn" command
- MINOR: server: skip servers with no idle conns earlier
- BUG/MINOR: server: fix the connection release logic regarding nearly full conditions
- MEDIUM: server: add a new pool-low-conn server setting
- BUG/MEDIUM: backend: always search in the safe list after failing on the idle one
- MINOR: backend: don't always takeover from the same threads
- MINOR: sched: make sched->task_list_size atomic
- MEDIUM: sched: create a new TASK_KILLED task flag
- MEDIUM: sched: implement task_kill() to kill a task
- MEDIUM: mux-h1: use task_kill() during h1_takeover() instead of task_wakeup()
- MEDIUM: mux-h2: use task_kill() during h2_takeover() instead of task_wakeup()
- MEDIUM: mux-fcgi: use task_kill() during fcgi_takeover() instead of task_wakeup()
- MINOR: list: Add MT_LIST_DEL_SAFE_NOINIT() and MT_LIST_ADDQ_NOCHECK()
- CLEANUP: connections: rename the toremove_lock to takeover_lock
- MEDIUM: connections: Don't use a lock when moving connections to remove.
- DOC: configuration: add missing index entries for tune.pool-{low,high}-fd-ratio
- DOC: configuration: fix alphabetical ordering for tune.pool-{high,low}-fd-ratio
- MINOR: config: add a new tune.idle-pool.shared global setting.
- MINOR: 51d: silence a warning about null pointer dereference
- MINOR: debug: add a new "debug dev memstats" command
- MINOR: log-format: allow to preserve spacing in log format strings
- BUILD: debug: avoid build warnings with DEBUG_MEM_STATS
- BUG/MAJOR: sched: make sure task_kill() always queues the task
- BUG/MEDIUM: muxes: Make sure nobody stole the connection before using it.
- BUG/MEDIUM: cli/proxy: don't try to dump idle connection state if there's none
- BUILD: haproxy: fix build error when RLIMIT_AS is not set
- BUG/MAJOR: sched: make it work also when not building with DEBUG_STRICT
- MINOR: log: add time second fraction field to rfc5424 log timestamp.
- BUG/MINOR: log: missing timezone on iso dates.
- BUG/MEDIUM: server: don't kill all idle conns when there are not enough
- MINOR: sched: split tasklet_wakeup() into tasklet_wakeup_on()
- BUG/MEDIUM: connections: Set the tid for the old tasklet on takeover.
- BUG/MEDIUM: connections: Let the xprt layer know a takeover happened.
- BUG/MINOR: http_act: don't check capture id in backend (2)
- BUILD: makefile: disable threads by default on OpenBSD
- BUILD: peers: fix build warning with gcc 4.2.1
- CI: cirrus-ci: exclude slow reg-tests
Building on OpenBSD 6.7 with gcc-4.2.1 yields the following warnings
which suggest that the initialization is not taken as expected but
that the container member is reset with each initialization:
src/peers.c: In function 'peer_send_updatemsg':
src/peers.c:1000: warning: initialized field overwritten
src/peers.c:1000: warning: (near initialization for 'p.updt')
src/peers.c:1001: warning: initialized field overwritten
src/peers.c:1001: warning: (near initialization for 'p.updt')
src/peers.c:1002: warning: initialized field overwritten
src/peers.c:1002: warning: (near initialization for 'p.updt')
src/peers.c:1003: warning: initialized field overwritten
src/peers.c:1003: warning: (near initialization for 'p.updt')
src/peers.c:1004: warning: initialized field overwritten
src/peers.c:1004: warning: (near initialization for 'p.updt')
Fixing this is trivial, we just have to initialize one level at
a time.
BUILD: makefile: disable threads by default on OpenBSD
As reported by Ilya in issue #725, building with threads on OpenBSD
is broken with gcc:
include/haproxy/tinfo.h:30: error: thread-local storage not supported for this target
Better stay safe and disable it. Clang seems to support (or emulate)
thread-local, at least it builds. Those willing to experiment can
easily pass USE_THREAD=1.
Tim Duesterhus [Fri, 3 Jul 2020 11:43:42 +0000 (13:43 +0200)]
BUG/MINOR: http_act: don't check capture id in backend (2)
Please refer to commit 19a69b3740702ce5503a063e9dfbcea5b9187d27 for all the
details. This follow up commit fixes the `http-response capture` case, the
previous one only fixed the `http-request capture` one. The documentation was
already updated and the change to `check_http_res_capture` is identical to
the `check_http_req_capture` change.
BUG/MEDIUM: connections: Let the xprt layer know a takeover happened.
When we takeover a connection, let the xprt layer know. If it has its own
tasklet, and it is already scheduled, then it has to be destroyed, otherwise
it may run the new mux tasklet on the old thread.
Note that we only do this for the ssl xprt for now, because the only other
one that might wake the mux up is the handshake one, which is supposed to
disappear before idle connections exist.
BUG/MEDIUM: connections: Set the tid for the old tasklet on takeover.
In the various takeover() methods, make sure we schedule the old tasklet
on the old thread, as we don't want it to run on our own thread! This
was causing a very rare crash when building with DEBUG_STRICT, seeing
that either an FD's thread mask didn't match the thread ID in h1_io_cb(),
or that stream_int_notify() would try to queue a task with the wrong
tid_bit.
In order to reproduce this, it is necessary to maintain many connections
(typically 30k) at a high request rate flowing over H1+SSL between two
proxies, the second of which would randomly reject ~1% of the incoming
connection and randomly killing some idle ones using a very short client
timeout. The request rate must be adjusted so that the CPUs are nearly
saturated, but never reach 100%. It's easier to reproduce this by skipping
local connections and always picking from other threads. The issue
should happen in less than 20s otherwise it's necessary to restart to
reset the idle connections lists.
MINOR: sched: split tasklet_wakeup() into tasklet_wakeup_on()
tasklet_wakeup() only checks tl->tid to know whether the task is
programmed to run on the current thread or on a specific thread. We'll
have to ease this selection in a subsequent patch, preferably without
modifying tl->tid, so let's have a new tasklet_wakeup_on() function
to specify the thread number to run on. That the logic has not changed
at all.
BUG/MEDIUM: server: don't kill all idle conns when there are not enough
In srv_cleanup_idle_connections(), we compute how many idle connections
are in excess compared to the average need. But we may actually be missing
some, for example if a certain number were recently closed and the average
of used connections didn't change much since previous period. In this
case exceed_conn can become negative. There was no special case for this
in the code, and calculating the per-thread share of connections to kill
based on this value resulted in special value -1 to be passed to
srv_migrate_conns_to_remove(), which for this function means "kill all of
them", as used in srv_cleanup_connections() for example.
This causes large variations of idle connections counts on servers and
CPU spikes at the moment the cleanup task passes. These were quite more
visible with SSL as it costs CPU to close and re-establish these
connections, and it also takes time, reducing the reuse ratio, hence
increasing the amount of connections during reconnection.
In this patch we simply skip the killing loop when this condition is met.
The function timeofday_as_iso_us adds now the trailing local timezone offset.
Doing this the function could be use directly to generate rfc5424 logs.
It affects content of a ring if the ring's format is set to 'iso' and 'timed'.
Note: the default ring 'buf0' is of type 'timed'.
It is preferable NOT to backport this to stable releases unless bugs are
reported, because while the previous format is not correct and the new
one is correct, there is a small risk to cause inconsistencies in log
format to some users who would not expect such a change in a stable
cycle.
BUG/MAJOR: sched: make it work also when not building with DEBUG_STRICT
Sadly, the fix from commit 54d31170a ("BUG/MAJOR: sched: make sure
task_kill() always queues the task") broke the builds without DEBUG_STRICT
as, in order to be careful, it plcaed a BUG_ON() around the previously
failing condition to check for any new possible failure, but this BUG_ON
strips the condition when DEBUG_STRICT is not set. We don't want BUG_ON
to evaluate any condition either as some debugging code calls possibly
expensive ones (e.g. in htx_get_stline). Let's just drop the useless
BUG_ON().
BUILD: haproxy: fix build error when RLIMIT_AS is not set
As reported in issue #724, openbsd fails to build in haproxy.c
due to a faulty comma in the middle of a warning message. This code
is only compiled when RLIMIT_AS is not defined, which seems to be
rare these days.
This may be backported to older versions as the problem was likely
introduced when strict limits were added.
BUG/MEDIUM: cli/proxy: don't try to dump idle connection state if there's none
Commit 69f591e3b ("MINOR: cli/proxy: add a new "show servers conn" command")
added the ability to dump the idle connections state for a server, but we
must not do this if idle connections were not allocated, which happens if
the server is configured with pool-max-conn 0.
BUG/MEDIUM: muxes: Make sure nobody stole the connection before using it.
In the various timeout functions, make sure nobody stole the connection from
us before attempting to doing anything with it, there's a very small race
condition between the time we access the task context, and the time we
actually check it again with the lock, where it could have been free'd.
BUG/MAJOR: sched: make sure task_kill() always queues the task
task_kill() may fail to queue a task if this task has never ever run,
because its equivalent (tasklet->list) member has never been "emptied"
since it didn't pass through the LIST_DEL_INIT() that's performed by
run_tasks_from_lists(). This results in these tasks to never be freed.
It happens during the mux takeover since the target task usually is
the timeout task which, by definition, has never run yet.
This fixes commit eb8c2c69f ("MEDIUM: sched: implement task_kill() to
kill a task") which was introduced after 2.2-dev11 and doesn't need to
be backported.
Dragan Dosen [Tue, 23 Jun 2020 16:16:44 +0000 (18:16 +0200)]
MINOR: log-format: allow to preserve spacing in log format strings
Now it's possible to preserve spacing everywhere except in "log-format",
"log-format-sd" and "unique-id-format" directives, where spaces are
delimiters and are merged. That may be useful when the response payload
is specified as a log format string by "lf-file" or "lf-string", or even
for headers or anything else.
In order to merge spaces, a new option LOG_OPT_MERGE_SPACES is applied
exclusively on options passed to function parse_logformat_string().
This patch fixes an issue #701 ("http-request return log-format file
evaluation altering spacing of ASCII output/art").
MINOR: debug: add a new "debug dev memstats" command
Now when building with -DDEBUG_MEM_STATS, some malloc/calloc/strdup/realloc
stats are kept per file+line number and may be displayed and even reset on
the CLI using "debug dev memstats". This allows to easily track potential
leakers or abnormal usages.
MINOR: config: add a new tune.idle-pool.shared global setting.
Enables ('on') or disables ('off') sharing of idle connection pools between
threads for a same server. The default is to share them between threads in
order to minimize the number of persistent connections to a server, and to
optimize the connection reuse rate. But to help with debugging or when
suspecting a bug in HAProxy around connection reuse, it can be convenient to
forcefully disable this idle pool sharing between multiple threads, and force
this option to "off". The default is on.
This could have been nice to have during the idle connections debugging,
but it's not too late to add it!
DOC: configuration: fix alphabetical ordering for tune.pool-{high,low}-fd-ratio
In addition they were in the wrong alphabetical order in the doc. They
were added in 2.0 by commit 88698d966 ("MEDIUM: connections: Add a way
to control the number of idling connections.") so this must be backported
to 2.0.
DOC: configuration: add missing index entries for tune.pool-{low,high}-fd-ratio
These two keywords didn't have an entry in the index. They were added in
2.0 by commit 88698d966 ("MEDIUM: connections: Add a way to control the
number of idling connections.") so this must be backported to 2.0.
Olivier Houchard [Mon, 29 Jun 2020 18:14:28 +0000 (20:14 +0200)]
MINOR: list: Add MT_LIST_DEL_SAFE_NOINIT() and MT_LIST_ADDQ_NOCHECK()
Add two new macros, MT_LIST_DEL_SAFE_NOINIT makes sure we remove the
element from the list, without reinitializing its next and prev, and
MT_LIST_ADDQ_NOCHECK is similar to MT_LIST_ADDQ(), except it doesn't check
if the element is already in a list.
The goal is to be able to move an element from a list we're currently
parsing to another, keeping it locked in the meanwhile.
MEDIUM: mux-fcgi: use task_kill() during fcgi_takeover() instead of task_wakeup()
task_wakeup() passes the task through the global run queue under the
global RQ lock, which is expensive when dealing with large amounts of
fcgi_takeover() calls. Let's use the new task_kill() instead to kill the
task.
MEDIUM: mux-h2: use task_kill() during h2_takeover() instead of task_wakeup()
task_wakeup() passes the task through the global run queue under the
global RQ lock, which is expensive when dealing with large amounts of
h2_takeover() calls. Let's use the new task_kill() instead to kill the
task.
MEDIUM: mux-h1: use task_kill() during h1_takeover() instead of task_wakeup()
task_wakeup() passes the task through the global run queue under the
global RQ lock, which is expensive when dealing with large amounts of
h1_takeover() calls. Let's use the new task_kill() instead to kill the
task.
By doing so, a scenario involving approximately 130k takeover/s running on
16 threads gained almost 3% performance from 319k req/s to 328k.
Willy Tarreau [Tue, 30 Jun 2020 09:48:48 +0000 (11:48 +0200)]
MEDIUM: sched: implement task_kill() to kill a task
task_kill() may be used by any thread to kill any task with less overhead
than a regular wakeup. In order to achieve this, it bypasses the priority
tree and inserts the task directly into the shared tasklets list, cast as
a tasklet. The task_list_size is updated to make sure it is properly
decremented after execution of this task. The task will thus be picked by
process_runnable_tasks() after checking the tree and sent to the TL_URGENT
list, where it will be processed and killed.
If the task is bound to more than one thread, its first thread will be the
one notified.
If the task was already queued or running, nothing is done, only the flag
is added so that it gets killed before or after execution. Of course it's
the caller's responsibility to make sur any resources allocated by this
task were already cleaned up or taken over.
Willy Tarreau [Tue, 30 Jun 2020 09:48:48 +0000 (11:48 +0200)]
MEDIUM: sched: create a new TASK_KILLED task flag
This flag, when set, will be used to indicate that the task must die.
At the moment this may only be placed by the task itself or by the
scheduler when placing it into the TL_NORMAL queue.
MINOR: backend: don't always takeover from the same threads
The next thread walking algorithm in commit 566df309c ("MEDIUM:
connections: Attempt to get idle connections from other threads.")
proved to be sufficient for most cases, but it still has some rough
edges when threads are unevenly loaded. If one thread wakes up with
10 streams to process in a burst, it will mainly take over connections
from the next one until it doesn't have anymore.
This patch implements a rotating index that is stored into the server
list and that any thread taking over a connection is responsible for
updating. This way it starts mostly random and avoids always picking
from the same place. This results in a smoother distribution overall
and a slightly lower takeover rate.
BUG/MEDIUM: backend: always search in the safe list after failing on the idle one
There's a tricky behavior that was lost when the idle connections were
made sharable between thread in commit 566df309c ("MEDIUM: connections:
Attempt to get idle connections from other threads."), it is the ability
to retry from the safe list when looking for any type of idle connection
and not finding one in the idle list.
It is already important when dealing with long-lived connections since
they ultimately all become safe, but that case is already covered by
the fact that safe conns not being used end up closing and are not
looked up anymore since connect_server() sees there are none.
But it's even more important when using server-side connections which
periodically close, because the new connections may spend half of their
time in safe state and the other half in the idle state, and failing
to grab one such connection from the right list results in establishing
a new connection.
This patch makes sure that a failure to find an idle connection results
in a new attempt at finding one from the safe list if available. In order
to avoid locking twice, connections are attempted alternatively from the
idle then safe list when picking from siblings. Tests have shown a ~2%
performance increase by avoiding to lock twice.
A typical test with 10000 connections over 16 threads with 210 servers
having a 1 millisecond response time and closing every 5 requests shows
a degrading performance starting at 120k req/s down to 60-90k and an
average reuse rate of 44%. After the fix, the reuse rate raises to 79%
and the performance becomes stable at 254k req/s. Similarly the previous
test with full keep-alive has now increased from 96% reuse rate to 99%
and from 352k to 375k req/s.
MEDIUM: server: add a new pool-low-conn server setting
The problem with the way idle connections currently work is that it's
easy for a thread to steal all of its siblings' connections, then release
them, then it's done by another one, etc. This happens even more easily
due to scheduling latencies, or merged events inside the same pool loop,
which, when dealing with a fast server responding in sub-millisecond
delays, can really result in one thread being fully at work at a time.
In such a case, we perform a huge amount of takeover() which consumes
CPU and requires quite some locking, sometimes resulting in lower
performance than expected.
In order to fight against this problem, this patch introduces a new server
setting "pool-low-conn", whose purpose is to dictate when it is allowed to
steal connections from a sibling. As long as the number of idle connections
remains at least as high as this value, it is permitted to take over another
connection. When the idle connection count becomes lower, a thread may only
use its own connections or create a new one. By proceeding like this even
with a low number (typically 2*nbthreads), we quickly end up in a situation
where all active threads have a few connections. It then becomes possible
to connect to a server without bothering other threads the vast majority
of the time, while still being able to use these connections when the
number of available FDs becomes low.
We also use this threshold instead of global.nbthread in the connection
release logic, allowing to keep more extra connections if needed.
A test performed with 10000 concurrent HTTP/1 connections, 16 threads
and 210 servers with 1 millisecond of server response time showed the
following numbers:
haproxy 2.1.7: 185000 requests per second
haproxy 2.2: 314000 requests per second
haproxy 2.2 lowconn 32: 352000 requests per second
The takeover rate goes down from 300k/s to 13k/s. The difference is
further amplified as the response time shrinks.
BUG/MINOR: server: fix the connection release logic regarding nearly full conditions
There was a logic bug in commit ddfe0743d ("MEDIUM: server: use the two
thresholds for the connection release algorithm"): instead of keeping
only our first idle connection when FDs become scarce, the condition was
inverted resulting in enforcing this constraint unless FDs are scarce.
This results in less idle connections than permitted to be kept under
normal condition.
MINOR: server: skip servers with no idle conns earlier
In conn_backend_get() we can avoid locking other servers when trying
to steal their connections when we know for sure they will not have
one, so let's do it to lower the contention on the lock.
MINOR: cli/proxy: add a new "show servers conn" command
This command reuses the existing "show servers state" to also dump the
state of active and idle connections. The main use is to serve as a
debugging tool to troubleshot connection reuse issues.
BUG/MINOR: proxy: always initialize the trash in show servers state
Actually the cleanup in commit 6ff8143f7 ("BUG/MINOR: proxy: fix
dump_server_state()'s misuse of the trash") allowed to spot that the
trash is never reset when dumping a servers state. I couldn't manage
to make it dump garbage even with large setups but didn't find either
where it's cleared between successive calls while other handlers do
explicitly invoke chunk_reset(), so it seems to happen a bit by luck.
Let's use chunk_printf() here for each turn, it makes things clearer.
This could be backported along with previous patch, especially if any
user reports occasional garbage appearing in the show servers output.
projects / thirdparty / haproxy.git / log
