Idea + question regarding the build targets
Hi all,
I'm currently re-reading my notes for the upcoming release and found
something I noted not that long ago regarding the TARGET variable of
the makefile. The list of operating systems hasn't changed in a while
and in parallel we've added a bunch of build options that do not solely
depend on the kernel anymore, but more on features related to the whole
environment (pcre, openssl, lua, systemd, zlib, and so on).
I thought that now that the targets definition is very simple in the
makefile, we could simply add distro names and versions and adjust the
set of default USE_xxx variables based on this. We could thus have
rhel{8,7,6,5}, debian{9,8,7,6}, ubuntu{18,16}, fedora{31,30,29} and so
on, just like we already have for solaris/openbsd/freebsd for example,
except that we generally only reference one version there. It would
become easier to add new variants even to stable distros to ease the
build for new users, who would just have to issue "make TARGET=debian9"
for example, without having to figure if PCRE2 has to be specified or
OPENSSL or any such thing.
This this is fairly minor and basically just requires a few lines to
be added to the makefile and to the doc, so I'm fine with including
this to the final release.
So my questions are :
- does anybody think it's a bad idea ?
- and among those who support the idea (if any), do you use
particular options for a given distro (pcre2, lua version, systemd)
- should we consider that using these pre-set distros enables more
features by default given that packages are readily available ?
(openssl seems obvious, zlib and pcre{1,2} probably, lua maybe)
Note that it is also something we can add as backports after the release.
It's just that if we want to teach people to start using a distro name
over a kernel version, we'd rather propose a few entries first.
Thanks for any hint,
Willy
Re: slow healthchecks after dev6+ with added commit "6ec902a MINOR: threads: serialize threads initialization"
Hi Pieter, On Tue, Jun 11, 2019 at 10:46:50PM +0200, PiBa-NL wrote: > Seems i kept you busy for another day.. But the result is there, it looks > 100% fixed to me :). Excellent, thanks for the fast feedback! We're getting there :-) I'm starting to really like our new development model, as for the first time we have the time to address non-critical issues before the release without being distracted by new bugs introduced every week. It starts to pay off. Cheers, Willy
Re: slow healthchecks after dev6+ with added commit "6ec902a MINOR: threads: serialize threads initialization"
Hi Willy,
Op 11-6-2019 om 11:37 schreef Willy Tarreau:
On Tue, Jun 11, 2019 at 09:06:46AM +0200, Willy Tarreau wrote:
I'd like you to give it a try in your environment to confirm whether or
not it does improve things. If so, I'll clean it up and merge it. I'm
also interested in any reproducer you could have, given that the made up
test case I did above doesn't even show anything alarming.
No need to waste your time anymore, I now found how to reproduce it with
this config :
global
stats socket /tmp/sock1 mode 666 level admin
nbthread 64
backend stopme
timeout server 1s
option tcp-check
tcp-check send "debug dev exit\n"
server cli unix@/tmp/sock1 check
The I run it in loops bound to different CPU counts :
$ time for i in {1..20}; do
taskset -c 0,1,2,3 ./haproxy -db -f slow-init.cfg >/dev/null 2>&1
done
With a single CPU, it can take up to 10 seconds to run the loop on
commits e186161 and e4d7c9d while it takes 0.18 second with the patch.
With 4 CPUs like above, it takes 1.5s with e186161, 2.3s with e4d7c9d
and 0.16 second with the patch.
The tests I had run consisted in starting hundreds of thousands of
listeners to amplify the impact of the start time, but in the end
it was diluting the extra time in an already very long time. Running
it in loops like above is quite close to what regtests do and explains
why I couldn't spot the difference (e.g. a few hundreds of ms at worst
among tens of seconds).
Thus I'm merging the patch now (cleaned up already and tested as well
without threads).
Let's hope it's the last time :-)
Thanks,
Willy
Seems i kept you busy for another day.. But the result is there, it
looks 100% fixed to me :).
Running without nbthread, and as such using 16 threads of the VM i'm
using, i now get this:
2.0-dev7-ca3551f 2019/06/11
** h1 0.732 WAIT4 pid=80796 status=0x0002 (user 0.055515 sys 0.039653)
** h2 0.846 WAIT4 pid=80799 status=0x0002 (user 0.039039 sys 0.039039)
# top TEST ./test/tls_health_checks-org.vtc passed (0.848)
Also with repeating the testcase 1000 times while running 10 of them in
parallel only 1 of them failed with a timeout:
S1 0.280 syslog|<134>Jun 11 22:24:48 haproxy[88306]:
::1:63856 [11/Jun/2019:22:24:48.074] fe1/1: Timeout during SSL handshake
I think together with the really short timeouts in the testcase itself
this is an excellent result.
I'm considering this one fully fixed, thanks again.
Regards,
PiBa-NL (Pieter)
Re: [ANNOUNCE] haproxy-2.0-dev7
Am 11.06.2019 um 22:01 schrieb Willy Tarreau:
> On Tue, Jun 11, 2019 at 09:42:13PM +0200, Aleksandar Lazic wrote:
>> Sorry to say that but there is no dev7 and in
>> http://www.haproxy.org/download/2.0/src/devel/
>> also not.
>
> Ah silly me! And who forgot to run "pubish-release" in your opinion ? :-)
The cat which hasn't passed the keyboard ;-)
> It's OK now. Thanks for reporting this!
Perfect.
Build log: https://gitlab.com/aleks001/haproxy20-centos/-/jobs/229375977
Image with tls 1.3: https://hub.docker.com/r/me2digital/haproxy20-centos
```
docker run --rm --entrypoint /usr/local/sbin/haproxy [MASKED]/haproxy20-centos
-vv
HA-Proxy version 2.0-dev7 2019/06/11 - https://haproxy.org/
Build options :
TARGET = linux2628
CPU = generic
CC = gcc
CFLAGS = -O2 -g -fno-strict-aliasing -Wdeclaration-after-statement -fwrapv
-Wno-unused-label -Wno-sign-compare -Wno-unused-parameter
-Wno-old-style-declaration -Wno-ignored-qualifiers -Wno-clobbered
-Wno-missing-field-initializers -Wtype-limits
OPTIONS = USE_PCRE=1 USE_PCRE_JIT=1 USE_THREAD=1 USE_PTHREAD_PSHARED=1
USE_REGPARM=1 USE_LINUX_SPLICE=1 USE_GETADDRINFO=1 USE_OPENSSL=1 USE_LUA=1
USE_SLZ=1 USE_TFO=1 USE_NS=1
Feature list : +EPOLL -KQUEUE -MY_EPOLL -MY_SPLICE +NETFILTER +PCRE +PCRE_JIT
-PCRE2 -PCRE2_JIT +POLL -PRIVATE_CACHE +THREAD +PTHREAD_PSHARED +REGPARM
-STATIC_PCRE -STATIC_PCRE2 +TPROXY +LINUX_TPROXY +LINUX_SPLICE +LIBCRYPT
+CRYPT_H -VSYSCALL +GETADDRINFO +OPENSSL +LUA +FUTEX +ACCEPT4 -MY_ACCEPT4 -ZLIB
+SLZ +CPU_AFFINITY +TFO +NS +DL +RT -DEVICEATLAS -51DEGREES -WURFL -SYSTEMD
-OBSOLETE_LINKER +PRCTL +THREAD_DUMP -EVPORTS
Default settings :
bufsize = 16384, maxrewrite = 1024, maxpollevents = 200
Built with multi-threading support (MAX_THREADS=64, default=1).
Built with OpenSSL version : OpenSSL 1.1.1c 28 May 2019
Running on OpenSSL version : OpenSSL 1.1.1c 28 May 2019
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
OpenSSL library supports : TLSv1.0 TLSv1.1 TLSv1.2 TLSv1.3
Built with Lua version : Lua 5.3.5
Built with network namespace support.
Built with transparent proxy support using: IP_TRANSPARENT IPV6_TRANSPARENT
IP_FREEBIND
Built with libslz for stateless compression.
Compression algorithms supported : identity("identity"), deflate("deflate"),
raw-deflate("deflate"), gzip("gzip")
Built with PCRE version : 8.32 2012-11-30
Running on PCRE version : 8.32 2012-11-30
PCRE library supports JIT : yes
Encrypted password support via crypt(3): yes
Built with the Prometheus exporter as a service
Available polling systems :
epoll : pref=300, test result OK
poll : pref=200, test result OK
select : pref=150, test result OK
Total: 3 (3 usable), will use epoll.
Available multiplexer protocols :
(protocols marked as cannot be specified using 'proto' keyword)
h2 : mode=HTXside=FE|BE mux=H2
h2 : mode=HTTP side=FEmux=H2
: mode=HTXside=FE|BE mux=H1
: mode=TCP|HTTP side=FE|BE mux=PASS
Available services :
prometheus-exporter
Available filters :
[SPOE] spoe
[COMP] compression
[CACHE] cache
[TRACE] trace
```
> Willy
>
Re: [ANNOUNCE] haproxy-2.0-dev7
On Tue, Jun 11, 2019 at 10:02:48PM +0200, Tim Düsterhus wrote: > Should I file an issue (referring to this thread) to keep track of the > whole issue and possible solutions? Oh yes please! > In Travis we have a clearly defined environment, so that kind of test > should work there as well. Good point! Tomorrow I'll talk about this one with Fred and we could decide to disable it for the release, then later think how we can add categorize and use such delicate tests. Thanks, Willy
Re: [ANNOUNCE] haproxy-2.0-dev7
Willy,
Am 11.06.19 um 21:58 schrieb Willy Tarreau:
>> So clearly there is no *seamless* reload happening here which is the
>> very thing the test is testing. The question is: Is the issue with the
>> 'abns' socket or is the issue with the 'fd@${testme}' socket? Can
>> fd-sockets be seamlessly passed?
>
> That's an excellent question. I tend to think we cannot pass them by
> definition since the fd is supposed to be passed by the caller in
> this case! So very likely the issue stems from the fact that we're
> relying on passing an fd in a situation where it has no chance of
> being passed due to the way it's configured.
>
> I don't know if we can run a vtest client against an abns address, in
> which case a solution could consist in using exclusively abns sockets
> for the tests. Another solution could consist in not using the client
> at all and relying on health checks instead to send the request. But
> in all cases that's racy.
Should I file an issue (referring to this thread) to keep track of the
whole issue and possible solutions?
>> In any case this is a bad test, because it is inherently racy.
>
> Yes I agree. Typically the type of thing I tend to be cautious about
> in regression testing, I know that when we try to go too far in this
> direction we tend to spend more time and energy fixing problems
> invented from scratch in the test scenarios instead of addressing
> real issues. There's a mid-point to find, which is always difficult.
>
> Note, we could also imagine having a few "unit" tests that require a
> very specific environment and which would be compatible with testing
> by hand (e.g. binding to a fixed port, etc). I see how this could be
> useful.
In Travis we have a clearly defined environment, so that kind of test
should work there as well.
Best regards
Tim Düsterhus
Re: [ANNOUNCE] haproxy-2.0-dev7
On Tue, Jun 11, 2019 at 09:42:13PM +0200, Aleksandar Lazic wrote: > Sorry to say that but there is no dev7 and in > http://www.haproxy.org/download/2.0/src/devel/ > also not. Ah silly me! And who forgot to run "pubish-release" in your opinion ? :-) It's OK now. Thanks for reporting this! Willy
Re: [ANNOUNCE] haproxy-2.0-dev7
On Tue, Jun 11, 2019 at 09:27:37PM +0200, Tim Düsterhus wrote:
> Reading the log I believe this might actually be a real problem within
> HAProxy. Note the log lines starting at 370 in the attached file:
>
> line 370: The old worker starts stopping its proxies (due to having
> received the SIGUSR1 from the re-executed master)
> line 401: The client connects
> line 406: The client starts waiting for a response
> line 408: The old worker stopped all proxies
> line 409: The new worker starts
> line 410: The client receives a connection reset
> line 411: The old worker exits
>
> So clearly there is no *seamless* reload happening here which is the
> very thing the test is testing. The question is: Is the issue with the
> 'abns' socket or is the issue with the 'fd@${testme}' socket? Can
> fd-sockets be seamlessly passed?
That's an excellent question. I tend to think we cannot pass them by
definition since the fd is supposed to be passed by the caller in
this case! So very likely the issue stems from the fact that we're
relying on passing an fd in a situation where it has no chance of
being passed due to the way it's configured.
I don't know if we can run a vtest client against an abns address, in
which case a solution could consist in using exclusively abns sockets
for the tests. Another solution could consist in not using the client
at all and relying on health checks instead to send the request. But
in all cases that's racy.
> In any case this is a bad test, because it is inherently racy.
Yes I agree. Typically the type of thing I tend to be cautious about
in regression testing, I know that when we try to go too far in this
direction we tend to spend more time and energy fixing problems
invented from scratch in the test scenarios instead of addressing
real issues. There's a mid-point to find, which is always difficult.
Note, we could also imagine having a few "unit" tests that require a
very specific environment and which would be compatible with testing
by hand (e.g. binding to a fixed port, etc). I see how this could be
useful.
Willy
Re: [ANNOUNCE] haproxy-2.0-dev7
Am 11.06.2019 um 20:02 schrieb Willy Tarreau: > Hi, > > HAProxy 2.0-dev7 was released on 2019/06/11. It added 34 new commits > after version 2.0-dev6. [snipp] > Please find the usual URLs below : >Site index : http://www.haproxy.org/ >Discourse: http://discourse.haproxy.org/ >Slack channel: https://slack.haproxy.org/ >Issue tracker: https://github.com/haproxy/haproxy/issues >Sources : http://www.haproxy.org/download/2.0/src/ Sorry to say that but there is no dev7 and in http://www.haproxy.org/download/2.0/src/devel/ also not. >Git repository : http://git.haproxy.org/git/haproxy.git/ >Git Web browsing : http://git.haproxy.org/?p=haproxy.git >Changelog: http://www.haproxy.org/download/2.0/src/CHANGELOG >Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/ > > Willy Regards Aleks
Re: [ANNOUNCE] haproxy-2.0-dev7
Willy,
Am 11.06.19 um 20:53 schrieb Willy Tarreau:
> Hi Tim,
>
> On Tue, Jun 11, 2019 at 08:14:41PM +0200, Tim Düsterhus wrote:
>> Willy,
>>
>> Am 11.06.19 um 20:02 schrieb Willy Tarreau:
>>> I noticed that no regtest failed on Travis since commit 7b3a79f6 which
>>
>> The abns_socket.vtc one is incredibly flaky though. I've noticed two
>> "failed" tests for Travis in in the list of commits today where a single
>> configuration failed to succeed because of this test. I've pushed the
>> button to re-run the failing configurations and they immediately
>> succeeded. We really must find a solution for that one.
>
> Ah that's interesting indeed. I remember seeing it fail very often in
> the past and was almost happy not to see it anymore. Now I understand,
> you're behind the curtains with your crank to restart it :-)
Only today! There's one failed job due to that test from 7 days before,
though: https://travis-ci.com/haproxy/haproxy/jobs/205334815#L529. I've
attached the relevant part of the log for posterity.
I guess if one pushes the "restart" button it will succeed without issue :/
> Yes we definitely need to address this. Either the test is bogus (which
> is always possible) or it reveals a real problem that either we need to
> address or to document if it's a technical limitation, all of which are
> unknown to me at this point.
>
Reading the log I believe this might actually be a real problem within
HAProxy. Note the log lines starting at 370 in the attached file:
line 370: The old worker starts stopping its proxies (due to having
received the SIGUSR1 from the re-executed master)
line 401: The client connects
line 406: The client starts waiting for a response
line 408: The old worker stopped all proxies
line 409: The new worker starts
line 410: The client receives a connection reset
line 411: The old worker exits
So clearly there is no *seamless* reload happening here which is the
very thing the test is testing. The question is: Is the issue with the
'abns' socket or is the issue with the 'fd@${testme}' socket? Can
fd-sockets be seamlessly passed?
In any case this is a bad test, because it is inherently racy.
Best regards
Tim Düsterhus
Test case: reg-tests/seamless-reload/abns_socket.vtc
*top 0.0 TEST reg-tests/seamless-reload/abns_socket.vtc starting
top 0.0 extmacro def pwd=/home/travis/build/haproxy/haproxy
top 0.0 extmacro def no-htx=
top 0.0 extmacro def localhost=127.0.0.1
top 0.0 extmacro def bad_backend=127.0.0.1 46119
top 0.0 extmacro def bad_ip=192.0.2.255
top 0.0 macro def testdir=/home/travis/build/haproxy/haproxy/reg-tests/seamless-reload
top 0.0 macro def tmpdir=/tmp/haregtests-2019-06-04_14-58-10.evaaZM/vtc.5888.1e49b2c2
** top 0.0 === varnishtest "Seamless reload issue with abns sockets"
*top 0.0 VTEST Seamless reload issue with abns sockets
** top 0.0 === feature ignore_unknown_macro
** top 0.0 === haproxy h1 -W -conf {
h10.0 macro def h1_cli_sock=127.0.0.1 35461
h10.0 macro def h1_cli_addr=127.0.0.1
h10.0 macro def h1_cli_port=35461
h10.0 setenv(cli, 4)
h10.0 macro def h1_testme_sock=127.0.0.1 43423
h10.0 macro def h1_testme_addr=127.0.0.1
h10.0 macro def h1_testme_port=43423
h10.0 setenv(testme, 5)
** h10.0 haproxy_start
h10.0 opt_worker 1 opt_daemon 0 opt_check_mode 0
h10.0 argv|exec "/home/travis/build/haproxy/haproxy/haproxy" -d -W -f "/tmp/haregtests-2019-06-04_14-58-10.evaaZM/vtc.5888.1e49b2c2/h1/cfg" -p "/tmp/haregtests-2019-06-04_14-58-10.evaaZM/vtc.5888.1e49b2c2/h1/pid"
h10.0 conf|global
h10.0 conf|\tstats socket "/tmp/haregtests-2019-06-04_14-58-10.evaaZM/vtc.5888.1e49b2c2/h1/stats.sock" level admin mode 600
h10.0 conf|stats socket "fd@${cli}" level admin
h10.0 conf|
h10.0 conf| global
h10.0 conf|stats socket "/tmp/haregtests-2019-06-04_14-58-10.evaaZM/vtc.5888.1e49b2c2/h1/stats" level admin expose-fd listeners
h10.0 conf|
h10.0 conf| defaults
h10.0 conf|mode http
h10.0 conf| option http-use-htx
h10.0 conf|log global
h10.0 conf|option httplog
h10.0 conf|timeout connect 15ms
h10.0 conf|timeout client 20ms
h10.0 conf|timeout server 20ms
h10.0 conf|
h10.0 conf| listen testme
h10.0 conf|bind "fd@${testme}"
h10.0 conf|server test_abns_server abns@wpproc1 send-proxy-v2
h10.0 conf|
h10.0 conf| frontend test_abns
h10.0 conf|bind abns@wpproc1 accept-proxy
h10.0 conf|http-request deny deny_status 200
h10.0 XXX 7 @637
*** h10.0 PID: 6050
h10.0 macro def h1_pid=6050
h10.0 macro def h1_name=/tmp/haregtests-2019-06-04_14-58-10.evaaZM/vtc.5888.1e49b2c2/h1
*** h10.0 wait-pid-file
*** h1
Re: [ANNOUNCE] haproxy-2.0-dev7
Hi Tim, On Tue, Jun 11, 2019 at 08:14:41PM +0200, Tim Düsterhus wrote: > Willy, > > Am 11.06.19 um 20:02 schrieb Willy Tarreau: > > I noticed that no regtest failed on Travis since commit 7b3a79f6 which > > The abns_socket.vtc one is incredibly flaky though. I've noticed two > "failed" tests for Travis in in the list of commits today where a single > configuration failed to succeed because of this test. I've pushed the > button to re-run the failing configurations and they immediately > succeeded. We really must find a solution for that one. Ah that's interesting indeed. I remember seeing it fail very often in the past and was almost happy not to see it anymore. Now I understand, you're behind the curtains with your crank to restart it :-) Yes we definitely need to address this. Either the test is bogus (which is always possible) or it reveals a real problem that either we need to address or to document if it's a technical limitation, all of which are unknown to me at this point. Willy
Re: [ANNOUNCE] haproxy-2.0-dev7
Willy, Am 11.06.19 um 20:02 schrieb Willy Tarreau: > I noticed that no regtest failed on Travis since commit 7b3a79f6 which The abns_socket.vtc one is incredibly flaky though. I've noticed two "failed" tests for Travis in in the list of commits today where a single configuration failed to succeed because of this test. I've pushed the button to re-run the failing configurations and they immediately succeeded. We really must find a solution for that one. Best regards Tim Düsterhus
[ANNOUNCE] haproxy-2.0-dev7
Hi, HAProxy 2.0-dev7 was released on 2019/06/11. It added 34 new commits after version 2.0-dev6. We're getting pretty good! I really was hesitating about marking it final already or not, but given that we've addressed a few last minute non-trivial issues and that there are still enough code cleanups and doc updates left to keep anyone busy, I thought I'd rather slightly postpone the final release to focus on maximal stability and cleanliness than perfect schedule. In short, there was an issue with the H1 to H2 upgrade making the performance irregular, another issue affecting the end of compression in HTX, a case where H1 could erroneously report being connected on the backend side before the end of the handshake (possibly the one causing me some irregular performance in H2 to H1 tests), the unsafe thread initialization issues, and a more delicate fix for HTX to address situations where buffer defragmentation was sometimes abused resulting in low performance. >From now on I estimate that the code is the code we'll release, more or less the fixes for any bug which will be reported in between. Let's focus solely on tidying the remaining FIXMEs, the doc, and possibly exporting a few more info to the CLI/stats to help with debugging (e.g. a few H1/H2 counters are terribly missing). I noticed that no regtest failed on Travis since commit 7b3a79f6 which concluded the changes for the stacking of the connection layers that begun after the start of the periodic failures which appeared between dev3 and dev4 mainly. Thus I would not completely rule out the possibility that some deeply burried bug in the old connection code was responsible for a part of them. If this is the case, good riddance! Let's set on anything between Friday and Tuesday for the final release, the former if no more bugs are reported, the latter if some are fixed late. Please, really, no more dev for 2.0, if you have anything to submit which doesn't belong to the exceptions above, this will go into -next (which I just rebased now) but I would appreciate it if you could delay your submissions so that we stay focused on 2.0. Please find the usual URLs below : Site index : http://www.haproxy.org/ Discourse: http://discourse.haproxy.org/ Slack channel: https://slack.haproxy.org/ Issue tracker: https://github.com/haproxy/haproxy/issues Sources : http://www.haproxy.org/download/2.0/src/ Git repository : http://git.haproxy.org/git/haproxy.git/ Git Web browsing : http://git.haproxy.org/?p=haproxy.git Changelog: http://www.haproxy.org/download/2.0/src/CHANGELOG Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/ Willy --- Complete changelog : Christopher Faulet (5): BUG/MINOR: cache/htx: Fix the counting of data already sent by the cache applet BUG/MEDIUM: compression/htx: Fix the adding of the last data block MINOR: flt_trace: Don't scrash the original offset during the random forwarding MAJOR: htx: Rework how free rooms are tracked in an HTX message MINOR: htx: Add the function htx_move_blk_before() Daniel Corbett (4): MINOR: contrib/spoa_server: Upgrade SPOP to 2.0 BUG/MEDIUM: contrib/spoa_server: Set FIN flag on agent frames MINOR: contrib/spoa_server: Add random IP score DOC/MINOR: contrib/spoa_server: Fix typo in README Frédéric Lécaille (6): MINOR peers: data structure simplifications for server names dictionary cache. DOC: peers: Update for dictionary cache entries for peers protocol. MINOR: dict: Store the length of the dictionary entries. MINOR: peers: A bit of optimization when encoding cached server names. MINOR: peers: Optimization for dictionary cache lookup. BUG/MINOR: dict: race condition fix when inserting dictionary entries. Olivier Houchard (6): MINOR: chunks: Make sure trash_size is only set once. BUG/MEDIUM: H1: When upgrading, make sure we don't free the buffer too early. BUG/MEDIUM: stream_interface: Make sure we call si_cs_process() if CS_FL_EOI. Revert "BUG/MEDIUM: H1: When upgrading, make sure we don't free the buffer too early." BUG/MEDIUM: h1: Don't try to subscribe if we had a connection error. BUG/MEDIUM: h1: Don't consider we're connected if the handshake isn't done. Willy Tarreau (13): BUG/MEDIUM: mux-h2: make sure the connection timeout is always set MINOR: tools: add new bitmap manipulation functions MINOR: logs: use the new bitmap functions instead of fd_sets for encoding maps Revert "MINOR: chunks: Make sure trash_size is only set once." MINOR: threads: serialize threads initialization MEDIUM: tools: improve time format error detection MINOR: threads: avoid clearing harmless twice in thread_release() MEDIUM: threads: add thread_sync_release() to synchronize steps BUG/MEDIUM: init/threads: prevent initialized threads from starting before others
Re: [PATCH] BUG/MEDIUM: contrib/spoa_server: Set FIN flag on agent frames
Hi Daniel, On Tue, Jun 11, 2019 at 08:49:17AM -0700, Daniel Corbett wrote: > Hello Willy, > > > Apologies for the delay. > > Thank you for taking the time to review. (...) Thanks for the updated patch set, that was just in time! I had already tagged 2.0-dev7. I just deleted the tag, merged your patches and tagged again :-) Cheers, Willy
Re: How to get overall stats in haproxy?
On Tue, 11 Jun 2019 at 10:03, wrote: > > Hi > > I'm using multiple processes haproxy, how can I get overall stats in haproxy? > > I have to access every process's stats page, and get the stats of that > process and count. > > Is there any convenient way to get overall stats? > > Thanks > > >From the command line, you can use https://github.com/unixsurfer/haproxytool and for pushing stats to Graphite you can use https://github.com/unixsurfer/haproxystats Alternatively, you can switch to threads where stats are aggregated by haproxy Cheers, Pavlos
Re: [PATCH] BUG/MEDIUM: contrib/spoa_server: Set FIN flag on agent frames
Hello Willy,
Apologies for the delay.
Thank you for taking the time to review.
> On Jun 2, 2019, at 10:11 PM, Willy Tarreau wrote:
>
> Hi Daniel,
>
> On Thu, May 30, 2019 at 12:57:10AM -0400, Daniel Corbett wrote:
>> Hello,
>>
>>
>> When communicating over SPOP the AGENT-HELLO, AGENT-DISCONNECT,
>> and ACK frames must have the FIN flag set.
>>
>> This patch also upgrades the SPOP_VERSION to 2.0 and adds the
>> "ip_score" transaction variable to the python & lua scripts.
>>
>> Thanks to Christopher for giving me the answer on how to solve this issue.
>
> Are all these changes related ? It seems to me these are distinct
> features, or maybe even a bug fix plus an extra feature, in which
> case these should be separate patches.
Something like that :)
They are all changes to make the spoa_server work out of the box as currently
it's not working with 2.0-dev.
I have gone ahead and separated the patches.
>
> I'm having a few quick questions below.
>
>> From e0e7992b3b2af3ac0e898d48d6ce5f7e9416b568 Mon Sep 17 00:00:00 2001
>> From: Daniel Corbett
>> Date: Wed, 29 May 2019 17:44:05 -0400
>> Subject: [PATCH] BUG/MEDIUM: contrib/spoa_server: Set FIN flag on agent
>> frames
>>
>> When communicating over SPOP the AGENT-HELLO, AGENT-DISCONNECT,
>> and ACK frames must have the FIN flag set.
>>
>> This patch also upgrades the SPOP_VERSION to 2.0 and adds the
>> "ip_score" transaction variable to the python & lua scripts.
> (...)
>> diff --git a/contrib/spoa_server/ps_lua.lua b/contrib/spoa_server/ps_lua.lua
>> index 26620459..aaf373c6 100644
>> --- a/contrib/spoa_server/ps_lua.lua
>> +++ b/contrib/spoa_server/ps_lua.lua
>> @@ -14,4 +14,5 @@ spoa.register_message("check-client-ip", function(args)
>> spoa.set_var_ipv6("ipv6", spoa.scope.txn, "1::f")
>> spoa.set_var_str("str", spoa.scope.txn, "1::f")
>> spoa.set_var_bin("bin", spoa.scope.txn, "1::f")
>> +spoa.set_var_int32("ip_score", spoa.scope.txn, 77)
>
> Be careful, your editor seems to have mixed spaces and tabs here, and
> a few other times in the same patch.
>
Sorry about that. I believe it was mainly due to python indention errors that
I was dealing with :(
>> diff --git a/contrib/spoa_server/spoa-server.conf
>> b/contrib/spoa_server/spoa-server.conf
>> index 13bd126a..87bbb721 100644
>> --- a/contrib/spoa_server/spoa-server.conf
>> +++ b/contrib/spoa_server/spoa-server.conf
>> @@ -1,5 +1,6 @@
>> global
>> debug
>> +nbthread 1
>
> What is the reason for forcing nbthread to 1 here ? Did you face a
> particular bug revealed by threads ? Given that many people use example
> configs as starting point for theirs, I'd rather avoid doing this, or
> this will last 10 years just like "option httpclose" or even the funny
> "ssl-engine rdrand" which in addition to being wrong was so much copied
> that google now auto-completes it when searching "ssl-engine haproxy"!
>
Yes I have hit a bug where the SPOA results are blank between requests if
threading is enabled. I found the quick fix to be setting nbthread to 1 --
however, I agree with you that we must be careful with these settings as I
realize they can persist for long after their intended need.
Unfortunately, I'm not really sure where to start fixing this issue.
>> diff --git a/contrib/spoa_server/spoa.c b/contrib/spoa_server/spoa.c
>> index a958f222..8cb40c54 100644
>> --- a/contrib/spoa_server/spoa.c
>> +++ b/contrib/spoa_server/spoa.c
>> @@ -679,13 +679,16 @@ error:
>> * the number of written bytes otherwise. */
>> static void prepare_agentack(struct worker *w)
>> {
>> +unsigned int flags = SPOE_FRM_FL_FIN;
>> +
>> w->ack_len = 0;
>>
>> /* Frame type */
>> w->ack[w->ack_len++] = SPOE_FRM_T_AGENT_ACK;
>>
>> -/* No flags for now */
>> -memset(w->ack + w->ack_len, 0, 4); /* No flags */
>> +/* Set flags */
>> +flags = htonl(flags);
>
> In general it's not a good idea to have the same variable hold two
> different endian versions of a same value. You can be certain that
> at one point something will be added above or below and will use the
> wrong one. Better simply do "flags = htonl(SPOE_FRM_FL_FIN)" or even
> better, preset flags to zero at the beginning of the function, and
> then add this flag this way : "flags |= htonl(SPOE_FRM_FL_FIN)".
>
>> +memcpy(w->ack + w->ack_len, (char *), 4);
>
> You don't need the "(char*)" cast here, "" is correct.
>
> The same applies to the two or three other locations. Thanks!
> Willy
>
Thanks for the info. Truthfully, I copied most of this from
contrib/spoa_example/spoa.c
I've fixed with your recommendations.
Thanks,
-- Daniel
0001-MINOR-contrib-spoa_server-Upgrade-SPOP-to-2.0.patch
Description: Binary data
0002-BUG-MEDIUM-contrib-spoa_server-Set-FIN-flag-on-agent.patch
Description: Binary data
0003-MINOR-contrib-spoa_server-Add-random-IP-score.patch
Description: Binary data
0004-DOC-MINOR-contrib-spoa_server-Fix-typo-in-README.patch
Description:
Re: SD-termination cause
Hi Maksim, On Wed, Jun 05, 2019 at 09:59:29AM +0200, Willy Tarreau wrote: > I'm still trying to fix this mess, it's not trivial at all and may > even require an extra -dev. So expect some updates later. Just a quick note to let you know that Olivier just fixed a directly related issue in 2.0-dev and I'm fairly confident it fixes your case as well. In short, the low-level mux-h1 believed the connection was completed before the handshakes were finished. Thus, it signaled to the upper layers that they could start to work too early. For me it was causing some "SH--" fields instead of "sC--", and that's fixed now. Feel free to re-run a test. Willy
Re: slow healthchecks after dev6+ with added commit "6ec902a MINOR: threads: serialize threads initialization"
On Tue, Jun 11, 2019 at 09:06:46AM +0200, Willy Tarreau wrote:
> I'd like you to give it a try in your environment to confirm whether or
> not it does improve things. If so, I'll clean it up and merge it. I'm
> also interested in any reproducer you could have, given that the made up
> test case I did above doesn't even show anything alarming.
No need to waste your time anymore, I now found how to reproduce it with
this config :
global
stats socket /tmp/sock1 mode 666 level admin
nbthread 64
backend stopme
timeout server 1s
option tcp-check
tcp-check send "debug dev exit\n"
server cli unix@/tmp/sock1 check
The I run it in loops bound to different CPU counts :
$ time for i in {1..20}; do
taskset -c 0,1,2,3 ./haproxy -db -f slow-init.cfg >/dev/null 2>&1
done
With a single CPU, it can take up to 10 seconds to run the loop on
commits e186161 and e4d7c9d while it takes 0.18 second with the patch.
With 4 CPUs like above, it takes 1.5s with e186161, 2.3s with e4d7c9d
and 0.16 second with the patch.
The tests I had run consisted in starting hundreds of thousands of
listeners to amplify the impact of the start time, but in the end
it was diluting the extra time in an already very long time. Running
it in loops like above is quite close to what regtests do and explains
why I couldn't spot the difference (e.g. a few hundreds of ms at worst
among tens of seconds).
Thus I'm merging the patch now (cleaned up already and tested as well
without threads).
Let's hope it's the last time :-)
Thanks,
Willy
How to get overall stats in haproxy?
Hi I'm using multiple processes haproxy, how can I get overall stats in haproxy? I have to access every process's stats page, and get the stats of that process and count. Is there any convenient way to get overall stats? Thanks
Re: Haproxy 1.9.8 - 100% CPU
Hi Marco, On Tue, Jun 11, 2019 at 09:10:19AM +0200, Marco Corte wrote: > Hello! > > It did not happen for weeks, but today I found again haproxy using a full > CPU core. > Haproxy v1.9.8 on Ubuntu 18.04. Ah, bad :-( > Actually there was a misalignment in a "peer" stick table configuration > between the two peers, but I do not know if this can cause the behaviour. > > If anyone is interested, I have "show sess all" and "show fd" outputs. I'm indeed interested. We've addressed scary issues very recently in 2.0 and I'm interested in comparing if they could match this case or not. If so this will tell us we'll have to be prepared to backport the fixes, but I'd rather wait a bit before doing so as they are tricky :-/ Thanks, Willy
Re: cygwin compilation error
Hi Bob, On Mon, Jun 10, 2019 at 09:54:29PM +, Zakharychev, Bob wrote: > FWIW: after a bit of experimentation and a quick dumb behavior comparison > program I found that FD_SET is compiled incorrectly by GCC 8.3.0 under Cygwin > with optimization level of -O2 and up. I compared native FD_SET/FD_ISSET with > hap_fd_set/hap_fd_isset and while the latter always functions correctly, the > former broke with -O2 and beyond. Further playing with optimizations enabled > by -O2 I was able to narrow the offender down to -ftree-pre, which enables > partial redundancy elimination (PRE) on trees. When tree PRE is enabled, GCC > generates wrong code for inlined FD_SET in map initialization loop which > results in most, and eventually all, bits of the map set when they shouldn't, > which explains those superfluous escapes that caused reg-tests failures. When > compiled with -fno-tree-pre, most reg-tests now pass. Still, since there is > already native implementation of bit address lookup and it seems to work > correctly with all optimizations enabled, maybe it's better just to switch to > it. Very interesting analysis, thank you for sharing it. In the mean time, as I mentioned in the ticket, I got rid of FD_SET/FD_ISSET(). The relevant commit is : commit 1bfd6020ce5f54120efd87906fd3675515ce4b8e Author: Willy Tarreau Date: Fri Jun 7 11:10:07 2019 +0200 MINOR: logs: use the new bitmap functions instead of fd_sets for encoding maps The fd_sets we've been using in the log encoding functions are not portable and were shown to break at least under Cygwin. This patch gets rid of them in favor of the new bitmap functions. (...) It's in latest 2.0-dev, was merged just after dev6. It should work just fine in your situation. Thanks! Willy
Re: Increase in sockets in TIME_WAIT with 1.9.x
On Mon, Jun 10, 2019 at 04:01:27PM -0500, Dave Chiluk wrote: > We are in the process of evaluating upgrading to 1.9.8 from 1.8.17, > and we are seeing a roughly 70% increase in sockets in TIME_WAIT on > our haproxy servers with a mostly idle server cluster > $ sudo netstat | grep 'TIME_WAIT' | wc -l Be careful, TIME_WAIT on the frontend is neither important nor representative of anything, only the backend counts. > Looking at the source/destination of this it seems likely that this > comes from healthchecks. We also see a corresponding load increase on > the backend applications serving the healthchecks. It's very possible and problematic at the same time. > Checking the git logs for healthcheck was unfruitful. Any clue what > might be going on? Normally we make lots of efforts to close health-check responses with a TCP RST (by disabling lingering before closing). I don't see why it wouldn't be done here. What OS are you running on and what do your health checks look like in the configuration ? Thanks, Willy
Re: [PATCH] BUG/MINOR: checks: Include UNIX server paths in arguments
Hi,
I think we could take this one, however I'm having some comments :
On Mon, Jun 10, 2019 at 04:13:19PM -0500, linuxdaemon wrote:
> --- a/src/checks.c
> +++ b/src/checks.c
> @@ -1891,8 +1891,23 @@ static int prepare_external_check(struct check *check)
> goto err;
> }
>
> - addr_to_str(>addr, buf, sizeof(buf));
> - check->argv[3] = strdup(buf);
> + // Make sure to send the unix socket address for the server to the
> external check command
> + if (s->addr.ss_family == AF_INET || s->addr.ss_family == AF_INET6) {
> + addr_to_str(>addr, buf, sizeof(buf));
> + check->argv[3] = strdup(buf);
> + }
> + else if (s->addr.ss_family == AF_UNIX) {
> + const struct sockaddr_un *un;
> +
> + un = (struct sockaddr_un *)>addr;
> + check->argv[3] = strdup(un->sun_path);
This will still not work for abstract namespace sockets, resulting in an
empty string. Maybe we should think about a way to encode such addresses
and document it so that external checks could use it.
Also, just thinking loud, what above differenciates the argument format
allowing an external check command to detect that this is a unix socket ?
I think we should at least use a prefix that's not allowed in AF_INET
nor AF_INET6 to make it unambigous. Otherwise if your UNIX socket's
address is the text representation of the equivalent IP listening
address (which is not uncommon) such as "192.168.1.35:8080", you will
create confusion for the external check process.
Maybe we should prefix unix sockets paths with "unix:" then we can
prefix abstract sockets with "abns:" ? Or maybe use "unix@" and "abns@"
just like in the haproxy config, that's less work to maintain homogenous
configurations and parsers outside.
Just my two cents,
Willy
Haproxy 1.9.8 - 100% CPU
Hello! It did not happen for weeks, but today I found again haproxy using a full CPU core. Haproxy v1.9.8 on Ubuntu 18.04. Actually there was a misalignment in a "peer" stick table configuration between the two peers, but I do not know if this can cause the behaviour. If anyone is interested, I have "show sess all" and "show fd" outputs. Thank you .marcoc $ sudo socat /var/run/haproxy.socket - <<< "show info" Name: HAProxy Version: 1.9.8-1ppa1~bionic Release_date: 2019/05/16 Nbthread: 1 Nbproc: 1 Process_num: 1 Pid: 7692 Uptime: 0d 16h00m46s Uptime_sec: 57646 Memmax_MB: 0 PoolAlloc_MB: 4 PoolUsed_MB: 4 PoolFailed: 0 Ulimit-n: 50240 Maxsock: 50240 Maxconn: 2 Hard_maxconn: 2 CurrConns: 12 CumConns: 22598 CumReq: 42267 MaxSslConns: 0 CurrSslConns: 15 CumSslConns: 59263 Maxpipes: 5000 PipesUsed: 0 PipesFree: 0 ConnRate: 0 ConnRateLimit: 0 MaxConnRate: 62 SessRate: 0 SessRateLimit: 0 MaxSessRate: 62 SslRate: 0 SslRateLimit: 0 MaxSslRate: 62 SslFrontendKeyRate: 0 SslFrontendMaxKeyRate: 40 SslFrontendSessionReuse_pct: 0 SslBackendKeyRate: 0 SslBackendMaxKeyRate: 6 SslCacheLookups: 6193 SslCacheMisses: 93 CompressBpsIn: 0 CompressBpsOut: 0 CompressBpsRateLim: 0 ZlibMemUsage: 0 MaxZlibMemUsage: 0 Tasks: 467 Run_queue: 3 Idle_pct: 54 node: pmli01 Stopping: 0 Jobs: 76 Unstoppable Jobs: 0 Listeners: 62 ActivePeers: 1 ConnectedPeers: 1 DroppedLogs: 0 BusyPolling: 0
Re: slow healthchecks after dev6+ with added commit "6ec902a MINOR: threads: serialize threads initialization"
Hit enter too fast, with the patch now.
On Tue, Jun 11, 2019 at 09:06:46AM +0200, Willy Tarreau wrote:
> Hi again Pieter,
>
> On Tue, Jun 11, 2019 at 04:24:47AM +0200, Willy Tarreau wrote:
> > I'm
> > going to have a look at this this morning. I now see how to make things
> > worse to observe the changes, I suspect that forcing a high nbthread and
> > binding all of them to a single CPU should reveal the issue much better.
>
> So I cannot reproduce your cases but by cheating I could make a very
> slight difference : I have started 50 processes in parallel, all on
> CPU #0, and all having 64 threads. That's a total of 3200 threads on
> a single CPU. Doing this with the TLS health check regtest, I see that
> before the patches it tool 14.2 seconds and after it took 14.7. However
> by modifying the startup code with the attached patch, it goes down to
> 11.3 seconds.
>
> I'd like you to give it a try in your environment to confirm whether or
> not it does improve things. If so, I'll clean it up and merge it. I'm
> also interested in any reproducer you could have, given that the made up
> test case I did above doesn't even show anything alarming.
>
> Thank you!
> Willy
diff --git a/src/haproxy.c b/src/haproxy.c
index a8898b78d..ca7cb77d5 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2556,6 +2556,10 @@ static void run_poll_loop()
}
}
+static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER;
+static pthread_cond_t init_cond = PTHREAD_COND_INITIALIZER;
+static int waiters = 0;
+
static void *run_thread_poll_loop(void *data)
{
struct per_thread_alloc_fct *ptaf;
@@ -2577,7 +2581,11 @@ static void *run_thread_poll_loop(void *data)
* after reallocating them locally. This will also ensure there is
* no race on file descriptors allocation.
*/
- thread_isolate();
+
+ pthread_mutex_lock(_mutex);
+ /* first one must set the number of waiters */
+ if (!waiters)
+ waiters = global.nbthread;
tv_update_date(-1,-1);
@@ -2608,14 +2616,20 @@ static void *run_thread_poll_loop(void *data)
* we want all threads to have already allocated their local fd tables
* before doing so.
*/
- thread_sync_release();
- thread_isolate();
- if (tid == 0)
+ waiters--;
+ /* the last one is responsible for starting the listeners */
+ if (waiters == 0)
protocol_enable_all();
- /* done initializing this thread, don't start before others are done */
- thread_sync_release();
+ pthread_cond_broadcast(_cond);
+ pthread_mutex_unlock(_mutex);
+
+ /* now wait for other threads to finish starting */
+ pthread_mutex_lock(_mutex);
+ while (waiters)
+ pthread_cond_wait(_cond, _mutex);
+ pthread_mutex_unlock(_mutex);
run_poll_loop();
Re: slow healthchecks after dev6+ with added commit "6ec902a MINOR: threads: serialize threads initialization"
Hi again Pieter, On Tue, Jun 11, 2019 at 04:24:47AM +0200, Willy Tarreau wrote: > I'm > going to have a look at this this morning. I now see how to make things > worse to observe the changes, I suspect that forcing a high nbthread and > binding all of them to a single CPU should reveal the issue much better. So I cannot reproduce your cases but by cheating I could make a very slight difference : I have started 50 processes in parallel, all on CPU #0, and all having 64 threads. That's a total of 3200 threads on a single CPU. Doing this with the TLS health check regtest, I see that before the patches it tool 14.2 seconds and after it took 14.7. However by modifying the startup code with the attached patch, it goes down to 11.3 seconds. I'd like you to give it a try in your environment to confirm whether or not it does improve things. If so, I'll clean it up and merge it. I'm also interested in any reproducer you could have, given that the made up test case I did above doesn't even show anything alarming. Thank you! Willy
