Re: [RFC][PATCHES] seamless reload

[Willy Tarreau]

Hi William,

On Sat, May 27, 2017 at 12:08:38AM +0200, William Lallemand wrote:
> The attached patches provide the "expose-fd listeners" stats socket option and
> remove the "no-unused-socket" global option.
> 
> It behaves exactly has Willy explain above minus the master process :-)
> 
> ps: Master Worker patches are coming soon!

Cool, thanks!

I'm applying them right now.
Willy

Re: [RFC][PATCHES] seamless reload

[William Lallemand]

Hi guys,

On Fri, May 12, 2017 at 04:26:01PM +0200, Willy Tarreau wrote:
> In fact William is currently working on the master-worker model to get rid
> of the systemd-wrapper and found some corner cases between this and your
> patchset. Nothing particularly difficult, just the fact that he'll need
> to pass the path to the previous socket to the new processes during reloads.
> 
> During this investigation it was found that we'd need to be able to say
> that a process possibly has no stats socket and that the next one will not
> be able to retrieve the FDs. Such information cannot be passed from the
> command line since it's a consequence of the config parsing. Thus we thought
> it would make sense to have a per-socket option to say whether or not it
> would be usable for offering the listening file descriptors, just like we
> currently have an administrative level on them (I even seem to remember
> that Olivier first asked if we wouldn't need to do this). And suddenly a
> few benefits appear when doing this :
>   - security freaks not willing to expose FDs over the socket would simply
> not enable them ;
> 
>   - we could restrict the number of processes susceptible of exposing the
> FDs simply by playing with the "process" directive on the socket ; that
> could also save some system-wide FDs ;
> 
>   - the master process could reliably find the socket's path in the conf
> (the first one with this new directive enabled), even if it's changed
> between reloads ;
> 
>   - in the default case (no specific option) we wouldn't change the existing
> behaviour so it would not make existing reloads worse.
> 

The attached patches provide the "expose-fd listeners" stats socket option and
remove the "no-unused-socket" global option.

It behaves exactly has Willy explain above minus the master process :-)

ps: Master Worker patches are coming soon!

-- 
William Lallemand

>From 5567d977f722e862c6ba56d65118e094ac28735c Mon Sep 17 00:00:00 2001
From: William Lallemand 
Date: Wed, 24 May 2017 00:57:40 +0200
Subject: [PATCH 1/3] cli: add ACCESS_LVL_MASK to store the access level

The current level variable use only 2 bits for storing the 3 access
level (user, oper and admin).

This patch add a bitmask which allows to use the remaining bits for
other usage.
---
 include/types/global.h |  2 ++
 src/cli.c  | 32 ++--
 src/stats.c|  2 +-
 src/stick_table.c  |  4 ++--
 4 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/include/types/global.h b/include/types/global.h
index 57b969d..cd5fda3 100644
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -69,6 +69,8 @@
 #define ACCESS_LVL_USER 1
 #define ACCESS_LVL_OPER 2
 #define ACCESS_LVL_ADMIN3
+#define ACCESS_LVL_MASK 0x3
+
 
 /* SSL server verify mode */
 enum {
diff --git a/src/cli.c b/src/cli.c
index 55baee3..cdbaf2b 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -217,7 +217,8 @@ static int stats_parse_global(char **args, int section_type, struct proxy *curpx
 		}
 
 		bind_conf = bind_conf_alloc(global.stats_fe, file, line, args[2], xprt_get(XPRT_RAW));
-		bind_conf->level = ACCESS_LVL_OPER; /* default access level */
+		bind_conf->level &= ~ACCESS_LVL_MASK;
+		bind_conf->level |= ACCESS_LVL_OPER; /* default access level */
 
 		if (!str2listener(args[2], global.stats_fe, bind_conf, file, line, err)) {
 			memprintf(err, "parsing [%s:%d] : '%s %s' : %s\n",
@@ -383,7 +384,7 @@ int cli_has_level(struct appctx *appctx, int level)
 	struct stream_interface *si = appctx->owner;
 	struct stream *s = si_strm(si);
 
-	if (strm_li(s)->bind_conf->level < level) {
+	if ((strm_li(s)->bind_conf->level & ACCESS_LVL_MASK) < level) {
 		appctx->ctx.cli.msg = stats_permission_denied_msg;
 		appctx->st0 = CLI_ST_PRINT;
 		return 0;
@@ -790,12 +791,12 @@ static int cli_io_handler_show_cli_sock(struct appctx *appctx)
 		} else
 			continue;
 
-		if (bind_conf->level == ACCESS_LVL_USER)
-			chunk_appendf(, "user ");
-		else if (bind_conf->level == ACCESS_LVL_OPER)
-			chunk_appendf(, "operator ");
-		else if (bind_conf->level == ACCESS_LVL_ADMIN)
+		if ((bind_conf->level & ACCESS_LVL_MASK) == ACCESS_LVL_ADMIN)
 			chunk_appendf(, "admin ");
+		else if ((bind_conf->level & ACCESS_LVL_MASK) == ACCESS_LVL_OPER)
+			chunk_appendf(, "operator ");
+		else if ((bind_conf->level & ACCESS_LVL_MASK) == ACCESS_LVL_USER)
+			chunk_appendf(, "user ");
 		else
 			chunk_appendf(, "  ");
 
@@ -1000,13 +1001,16 @@ static int bind_parse_level(char **args, int cur_arg, struct proxy *px, struct b
 		return ERR_ALERT | ERR_FATAL;
 	}
 
-	if (!strcmp(args[cur_arg+1], "user"))
-		conf->level = ACCESS_LVL_USER;
-	else if (!strcmp(args[cur_arg+1], "operator"))
-		conf->level = ACCESS_LVL_OPER;
-	else if (!strcmp(args[cur_arg+1], "admin"))
-		conf->level = ACCESS_LVL_ADMIN;
-	else {
+	if (!strcmp(args[cur_arg+1], "user")) {
+	

Re: [RFC][PATCHES] seamless reload

[Willy Tarreau]

Hi Pavlos, Olivier,

On Mon, May 08, 2017 at 02:34:05PM +0200, Olivier Houchard wrote:
> Hi Pavlos,
> 
> On Sun, May 07, 2017 at 12:05:28AM +0200, Pavlos Parissis wrote:
> [...]
> > Ignore ignore what I wrote, I am an idiot I am an idiot as I forgot the most
> > important bit of the test, to enable the seamless reload by suppling the
> > HAPROXY_STATS_SOCKET environment variable:-(
> > 
> > I added to the systemd overwrite file:
> > [Service]
> > Environment=CONFIG="/etc/lb_engine/haproxy.cfg"
> > "HAPROXY_STATS_SOCKET=/run/lb_engine/process-1.sock"
> > 
> > and wrk2 reports ZERO errors where with HAPEE reports ~49.
> > 
> > I am terrible sorry for this stupid mistake.
> > 
> > But, this mistake revealed something interesting. The fact that with the 
> > latest
> > code we have more errors during reload.
> > 
> > @Olivier, great work dude. I am waiting for this to be back-ported to 
> > HAPEE-1.7r1.
> > 
> > Once again I am sorry for my mistake,
> > Pavlos
> > 
> 
> Thanks a lot for testing !
> This is interesting indeed. My patch may make it worse when not passing
> fds via the unix socket, as all processes now keep all sockets opened, even
> the one they're not using, maybe it make the window between the last
> accept and the close bigger.

That's very interesting indeed. In fact it's the window between the last
accept and the *last* close, due to processes holding the socket while
not being willing to do accept anything on it.

> If that is so, then the global option "no-unused-socket" should provide
> a comparable error rate.

In fact William is currently working on the master-worker model to get rid
of the systemd-wrapper and found some corner cases between this and your
patchset. Nothing particularly difficult, just the fact that he'll need
to pass the path to the previous socket to the new processes during reloads.

During this investigation it was found that we'd need to be able to say
that a process possibly has no stats socket and that the next one will not
be able to retrieve the FDs. Such information cannot be passed from the
command line since it's a consequence of the config parsing. Thus we thought
it would make sense to have a per-socket option to say whether or not it
would be usable for offering the listening file descriptors, just like we
currently have an administrative level on them (I even seem to remember
that Olivier first asked if we wouldn't need to do this). And suddenly a
few benefits appear when doing this :
  - security freaks not willing to expose FDs over the socket would simply
not enable them ;

  - we could restrict the number of processes susceptible of exposing the
FDs simply by playing with the "process" directive on the socket ; that
could also save some system-wide FDs ;

  - the master process could reliably find the socket's path in the conf
(the first one with this new directive enabled), even if it's changed
between reloads ;

  - in the default case (no specific option) we wouldn't change the existing
behaviour so it would not make existing reloads worse.

Pavlos, regarding the backport to your beloved version, that's planned, but
as you can see, while the main technical issues have already been sorted out,
there will still be a few small integration-specific changes to come, which
is why for now it's still on hold until all these details are sorted out
once for all.

Best regards,
Willy

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

Hi Pavlos,

On Sun, May 07, 2017 at 12:05:28AM +0200, Pavlos Parissis wrote:
[...]
> Ignore ignore what I wrote, I am an idiot I am an idiot as I forgot the most
> important bit of the test, to enable the seamless reload by suppling the
> HAPROXY_STATS_SOCKET environment variable:-(
> 
> I added to the systemd overwrite file:
> [Service]
> Environment=CONFIG="/etc/lb_engine/haproxy.cfg"
> "HAPROXY_STATS_SOCKET=/run/lb_engine/process-1.sock"
> 
> and wrk2 reports ZERO errors where with HAPEE reports ~49.
> 
> I am terrible sorry for this stupid mistake.
> 
> But, this mistake revealed something interesting. The fact that with the 
> latest
> code we have more errors during reload.
> 
> @Olivier, great work dude. I am waiting for this to be back-ported to 
> HAPEE-1.7r1.
> 
> Once again I am sorry for my mistake,
> Pavlos
> 

Thanks a lot for testing !
This is interesting indeed. My patch may make it worse when not passing
fds via the unix socket, as all processes now keep all sockets opened, even
the one they're not using, maybe it make the window between the last
accept and the close bigger.
If that is so, then the global option "no-unused-socket" should provide
a comparable error rate.

Regards,

Olivier

Re: [RFC][PATCHES] seamless reload

[Pavlos Parissis]

On 06/05/2017 11:15 μμ, Pavlos Parissis wrote:
> On 04/05/2017 01:16 μμ, Olivier Houchard wrote:
>> On Thu, May 04, 2017 at 10:03:07AM +, Pierre Cheynier wrote:
>>> Hi Olivier,
>>>
>>> Many thanks for that ! As you know, we are very interested on this topic.
>>> We'll test your patches soon for sure.
>>>
>>> Pierre
>>
>> Hi Pierre :)
>>
>> Thanks ! I'm very interested in knowing how well it works for you.
>> Maybe we can talk about that around a beer sometime.
>>
>> Olivier
>>
> 
> Hi,
> 
> I finally managed to find time to perform some testing.
> 
> Fristly, let me explain environment.
> 
> Server and generator are on different servers (bare medal) with the same spec,
> network interrupts are pinned to all CPUs and irqbalancer daemon is disabled.
> Both nodes have 10GbE network interfaces.
> 
> I compared HAPEE with HAProxy using the following versions:
> 
> ### HAProxy
> The git SHA isn't mentioned in the output because I created the tarball
> with:
> 
> git archive --format=tar --prefix="haproxy-1.8.0/" HEAD | gzip -9 >
> haproxy-1.8.0.tar.gz
> 
> as I had to build the rpm using a tar ball, but I used the latest haproxy
> at f494977bc1a361c26f8cc0516366ef2662ac9502 commit.
> 
> /usr/sbin/haproxy -vv
> HA-Proxy version 1.8-dev1 2017/04/03
> Copyright 2000-2017 Willy Tarreau 
> 
> Build options :
>   TARGET  = linux2628
>   CPU = generic
>   CC  = gcc
>   CFLAGS  = -DMAX_HOSTNAME_LEN=42
>   OPTIONS = USE_LINUX_TPROXY=1 USE_CPU_AFFINITY=1 USE_REGPARM=1 USE_OPENSSL=1
> USE_PCRE=1 USE_PCRE_JIT=1
> 
> Default settings :
>   maxconn = 2000, bufsize = 16384, maxrewrite = 1024, maxpollevents = 200
> 
> Built with OpenSSL version : OpenSSL 1.0.1e-fips 11 Feb 2013
> Running on OpenSSL version : OpenSSL 1.0.1e-fips 11 Feb 2013
> OpenSSL library supports TLS extensions : yes
> OpenSSL library supports SNI : yes
> Built with transparent proxy support using: IP_TRANSPARENT IPV6_TRANSPARENT
> IP_FREEBIND
> Built with network namespace support.
> Built without compression support (neither USE_ZLIB nor USE_SLZ are set).
> Compression algorithms supported : identity("identity")
> Encrypted password support via crypt(3): yes
> Built with PCRE version : 8.32 2012-11-30
> Running on PCRE version : 8.32 2012-11-30
> PCRE library supports JIT : yes
> 
> 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 filters :
> [SPOE] spoe
> [COMP] compression
> [TRACE] trace
> 
> ### HAPEE version
> /opt/hapee-1.7/sbin/hapee-lb -vv
> HA-Proxy version 1.7.0-1.0.0-163.180 2017/04/10
> Copyright 2000-2016 Willy Tarreau 
> 
> Build options :
>   TARGET  = linux2628
>   CPU = generic
>   CC  = gcc
>   CFLAGS  = -O2 -g -fno-strict-aliasing -Wdeclaration-after-statement
> -DMAX_SESS_STKCTR=10 -DSTKTABLE_EXTRA_DATA_TYPES=10
>   OPTIONS = USE_MODULES=1 USE_LINUX_SPLICE=1 USE_LINUX_TPROXY=1 USE_SLZ=1
> USE_CPU_AFFINITY=1 USE_REGPARM=1 USE_OPENSSL=1 USE_LUA=1 USE_PCRE= 
> USE_PCRE_JIT=1
> USE_NS=1
> 
> Default settings :
>   maxconn = 2000, bufsize = 16384, maxrewrite = 1024, maxpollevents = 200
> 
> Encrypted password support via crypt(3): yes
> Built with libslz for stateless compression.
> Compression algorithms supported : identity("identity"), deflate("deflate"),
> raw-deflate("deflate"), gzip("gzip")
> Built with OpenSSL version : OpenSSL 1.0.1e-fips 11 Feb 2013
> Running on OpenSSL version : OpenSSL 1.0.1e-fips 11 Feb 2013
> OpenSSL library supports TLS extensions : yes
> OpenSSL library supports SNI : yes
> OpenSSL library supports prefer-server-ciphers : yes
> Built with PCRE version : 8.32 2012-11-30
> Running on PCRE version : 8.32 2012-11-30
> PCRE library supports JIT : yes
> Built with Lua version : Lua 5.3.3
> Built with transparent proxy support using: IP_TRANSPARENT IPV6_TRANSPARENT
> IP_FREEBIND
> Built with network namespace support
> 
> 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 filters :
> [COMP] compression
> [TRACE] trace
> [SPOE] spoe
> 
> 
> The configuration is the same and it is attached. As you can use I use nbproc 
> >1
> and each process is pinned to different CPU. We have 12 real CPUs as Intel 
> hyper
> threading is disabled, but we only use 10 CPUs for haproxy, the remaining two 
> CPUs
> are left for other daemons to use.
> 
> I experimented with wrk2 and httpress stress tools and decided to use wrk2 for
> these tests. I didn't want to use the inject and other tools provided by 
> haproxy
> as I believe using different clients provides higher chances to spot problems.
> 
> In my tests I see that wrk2 reports higher read errors with HAProxy (3890) 
> than
> HAPEE (36). I don't know the meaning of 

Re: [RFC][PATCHES] seamless reload

[Pavlos Parissis]

On 04/05/2017 01:16 μμ, Olivier Houchard wrote:
> On Thu, May 04, 2017 at 10:03:07AM +, Pierre Cheynier wrote:
>> Hi Olivier,
>>
>> Many thanks for that ! As you know, we are very interested on this topic.
>> We'll test your patches soon for sure.
>>
>> Pierre
> 
> Hi Pierre :)
> 
> Thanks ! I'm very interested in knowing how well it works for you.
> Maybe we can talk about that around a beer sometime.
> 
> Olivier
> 

Hi,

I finally managed to find time to perform some testing.

Fristly, let me explain environment.

Server and generator are on different servers (bare medal) with the same spec,
network interrupts are pinned to all CPUs and irqbalancer daemon is disabled.
Both nodes have 10GbE network interfaces.

I compared HAPEE with HAProxy using the following versions:

### HAProxy
The git SHA isn't mentioned in the output because I created the tarball
with:

git archive --format=tar --prefix="haproxy-1.8.0/" HEAD | gzip -9 >
haproxy-1.8.0.tar.gz

as I had to build the rpm using a tar ball, but I used the latest haproxy
at f494977bc1a361c26f8cc0516366ef2662ac9502 commit.

/usr/sbin/haproxy -vv
HA-Proxy version 1.8-dev1 2017/04/03
Copyright 2000-2017 Willy Tarreau 

Build options :
  TARGET  = linux2628
  CPU = generic
  CC  = gcc
  CFLAGS  = -DMAX_HOSTNAME_LEN=42
  OPTIONS = USE_LINUX_TPROXY=1 USE_CPU_AFFINITY=1 USE_REGPARM=1 USE_OPENSSL=1
USE_PCRE=1 USE_PCRE_JIT=1

Default settings :
  maxconn = 2000, bufsize = 16384, maxrewrite = 1024, maxpollevents = 200

Built with OpenSSL version : OpenSSL 1.0.1e-fips 11 Feb 2013
Running on OpenSSL version : OpenSSL 1.0.1e-fips 11 Feb 2013
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
Built with transparent proxy support using: IP_TRANSPARENT IPV6_TRANSPARENT
IP_FREEBIND
Built with network namespace support.
Built without compression support (neither USE_ZLIB nor USE_SLZ are set).
Compression algorithms supported : identity("identity")
Encrypted password support via crypt(3): yes
Built with PCRE version : 8.32 2012-11-30
Running on PCRE version : 8.32 2012-11-30
PCRE library supports JIT : yes

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 filters :
[SPOE] spoe
[COMP] compression
[TRACE] trace

### HAPEE version
/opt/hapee-1.7/sbin/hapee-lb -vv
HA-Proxy version 1.7.0-1.0.0-163.180 2017/04/10
Copyright 2000-2016 Willy Tarreau 

Build options :
  TARGET  = linux2628
  CPU = generic
  CC  = gcc
  CFLAGS  = -O2 -g -fno-strict-aliasing -Wdeclaration-after-statement
-DMAX_SESS_STKCTR=10 -DSTKTABLE_EXTRA_DATA_TYPES=10
  OPTIONS = USE_MODULES=1 USE_LINUX_SPLICE=1 USE_LINUX_TPROXY=1 USE_SLZ=1
USE_CPU_AFFINITY=1 USE_REGPARM=1 USE_OPENSSL=1 USE_LUA=1 USE_PCRE= 
USE_PCRE_JIT=1
USE_NS=1

Default settings :
  maxconn = 2000, bufsize = 16384, maxrewrite = 1024, maxpollevents = 200

Encrypted password support via crypt(3): yes
Built with libslz for stateless compression.
Compression algorithms supported : identity("identity"), deflate("deflate"),
raw-deflate("deflate"), gzip("gzip")
Built with OpenSSL version : OpenSSL 1.0.1e-fips 11 Feb 2013
Running on OpenSSL version : OpenSSL 1.0.1e-fips 11 Feb 2013
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
OpenSSL library supports prefer-server-ciphers : yes
Built with PCRE version : 8.32 2012-11-30
Running on PCRE version : 8.32 2012-11-30
PCRE library supports JIT : yes
Built with Lua version : Lua 5.3.3
Built with transparent proxy support using: IP_TRANSPARENT IPV6_TRANSPARENT
IP_FREEBIND
Built with network namespace support

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 filters :
[COMP] compression
[TRACE] trace
[SPOE] spoe


The configuration is the same and it is attached. As you can use I use nbproc >1
and each process is pinned to different CPU. We have 12 real CPUs as Intel hyper
threading is disabled, but we only use 10 CPUs for haproxy, the remaining two 
CPUs
are left for other daemons to use.

I experimented with wrk2 and httpress stress tools and decided to use wrk2 for
these tests. I didn't want to use the inject and other tools provided by haproxy
as I believe using different clients provides higher chances to spot problems.

In my tests I see that wrk2 reports higher read errors with HAProxy (3890) than
HAPEE (36). I don't know the meaning of the read error and it could be some
stupidity in the code of wrk2. I am saying this because two years ago we spent
four weeks stress testing HAPEE and found out that all open source http stress
tool sucks and some of the errors they report are client errors rather server.
But, in this case wrk2 was always reporting 

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

On Thu, May 04, 2017 at 10:03:07AM +, Pierre Cheynier wrote:
> Hi Olivier,
> 
> Many thanks for that ! As you know, we are very interested on this topic.
> We'll test your patches soon for sure.
> 
> Pierre

Hi Pierre :)

Thanks ! I'm very interested in knowing how well it works for you.
Maybe we can talk about that around a beer sometime.

Olivier

RE: [RFC][PATCHES] seamless reload

[Pierre Cheynier]

Hi Olivier,

Many thanks for that ! As you know, we are very interested on this topic.
We'll test your patches soon for sure.

Pierre

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

On Wed, Apr 19, 2017 at 09:58:27AM +0200, Pavlos Parissis wrote:
> On 13/04/2017 06:18 μμ, Olivier Houchard wrote:
> > On Thu, Apr 13, 2017 at 06:00:59PM +0200, Conrad Hoffmann wrote:
> >> On 04/13/2017 05:10 PM, Olivier Houchard wrote:
> >>> On Thu, Apr 13, 2017 at 04:59:26PM +0200, Conrad Hoffmann wrote:
>  Sure, here it is ;P
> 
>  I now get a segfault (on reload):
> 
>  *** Error in `/usr/sbin/haproxy': corrupted double-linked list:
>  0x05b511e0 ***
> 
>  Here is the backtrace, retrieved from the core file:
> 
>  (gdb) bt
>  #0  0x7f4c92801067 in __GI_raise (sig=sig@entry=6) at
>  ../nptl/sysdeps/unix/sysv/linux/raise.c:56
>  #1  0x7f4c92802448 in __GI_abort () at abort.c:89
>  #2  0x7f4c9283f1b4 in __libc_message (do_abort=do_abort@entry=1,
>  fmt=fmt@entry=0x7f4c92934210 "*** Error in `%s': %s: 0x%s ***\n") at
>  ../sysdeps/posix/libc_fatal.c:175
>  #3  0x7f4c9284498e in malloc_printerr (action=1, str=0x7f4c929302ec
>  "corrupted double-linked list", ptr=) at malloc.c:4996
>  #4  0x7f4c92845923 in _int_free (av=0x7f4c92b71620 ,
>  p=, have_lock=0) at malloc.c:3996
>  #5  0x00485850 in tcp_find_compatible_fd (l=0xaaed20) at
>  src/proto_tcp.c:812
>  #6  tcp_bind_listener (listener=0xaaed20, errmsg=0x7ffccc774e10 "",
>  errlen=100) at src/proto_tcp.c:878
>  #7  0x00493ce1 in start_proxies (verbose=0) at src/proxy.c:793
>  #8  0x004091ec in main (argc=21, argv=0x7ffccc775168) at
>  src/haproxy.c:1942
> >>>
> >>> Ok, yet another stupid mistake, hopefully the attached patch fixes this :)
> >>>
> >>> Thanks !
> >>>
> >>> Olivier
> >>
> >>
> >> It does indeed! Not only does it work now, the result is impressive! Not a
> >> single dropped request even when aggressively reloading under substantial 
> >> load!
> >>
> >> You certainly gave me an unexpected early easter present here :)
> >>
> >> I will now head out, but I am planning on installing a 1.8 version with
> >> your patches on our canary pool (which receives a small amount of
> >> production traffic to test changes) after the holidays. I will happily test
> >> anything else that might be helpful for you. I will also set up a proper
> >> load test inside our data center then, but I expect no surprises there (my
> >> current tests were done over a VPN link, somewhat limiting the achievable
> >> throughput).
> >>
> >> Once more, thank you so much! This will greatly simplify much of our
> >> operations. If there is anything else we can help test, let me know :)
> > 
> > Pfew, at least :) Thanks a lot for your patience, and doing all that 
> > testing !
> > 
> > Olivier
> > 
> 
> 
> Joining this again a bit late, do you still want me to test it?
> I would like to know if there are any performance impact, but I see that
> Conrad Hoffmann has done all the hard work on this. So, we can conclude that 
> we
> don't expect any performance impact.
> 
> Once again thanks a lot for your hard work on this.
> @Conrad Hoffmann, thanks a lot for testing this and checking if there is any
> performance impact.
> 


Hi Pavlos,

More testing is always appreciated :)
I don't expect any performance impact, but that wouldn't be the first time
I say that and am proven wrong, though as you said with all the testing
Conrad did, I'm fairly confident it should be OK.

Thanks !

Olivier

Re: [RFC][PATCHES] seamless reload

[Pavlos Parissis]

On 13/04/2017 06:18 μμ, Olivier Houchard wrote:
> On Thu, Apr 13, 2017 at 06:00:59PM +0200, Conrad Hoffmann wrote:
>> On 04/13/2017 05:10 PM, Olivier Houchard wrote:
>>> On Thu, Apr 13, 2017 at 04:59:26PM +0200, Conrad Hoffmann wrote:
 Sure, here it is ;P

 I now get a segfault (on reload):

 *** Error in `/usr/sbin/haproxy': corrupted double-linked list:
 0x05b511e0 ***

 Here is the backtrace, retrieved from the core file:

 (gdb) bt
 #0  0x7f4c92801067 in __GI_raise (sig=sig@entry=6) at
 ../nptl/sysdeps/unix/sysv/linux/raise.c:56
 #1  0x7f4c92802448 in __GI_abort () at abort.c:89
 #2  0x7f4c9283f1b4 in __libc_message (do_abort=do_abort@entry=1,
 fmt=fmt@entry=0x7f4c92934210 "*** Error in `%s': %s: 0x%s ***\n") at
 ../sysdeps/posix/libc_fatal.c:175
 #3  0x7f4c9284498e in malloc_printerr (action=1, str=0x7f4c929302ec
 "corrupted double-linked list", ptr=) at malloc.c:4996
 #4  0x7f4c92845923 in _int_free (av=0x7f4c92b71620 ,
 p=, have_lock=0) at malloc.c:3996
 #5  0x00485850 in tcp_find_compatible_fd (l=0xaaed20) at
 src/proto_tcp.c:812
 #6  tcp_bind_listener (listener=0xaaed20, errmsg=0x7ffccc774e10 "",
 errlen=100) at src/proto_tcp.c:878
 #7  0x00493ce1 in start_proxies (verbose=0) at src/proxy.c:793
 #8  0x004091ec in main (argc=21, argv=0x7ffccc775168) at
 src/haproxy.c:1942
>>>
>>> Ok, yet another stupid mistake, hopefully the attached patch fixes this :)
>>>
>>> Thanks !
>>>
>>> Olivier
>>
>>
>> It does indeed! Not only does it work now, the result is impressive! Not a
>> single dropped request even when aggressively reloading under substantial 
>> load!
>>
>> You certainly gave me an unexpected early easter present here :)
>>
>> I will now head out, but I am planning on installing a 1.8 version with
>> your patches on our canary pool (which receives a small amount of
>> production traffic to test changes) after the holidays. I will happily test
>> anything else that might be helpful for you. I will also set up a proper
>> load test inside our data center then, but I expect no surprises there (my
>> current tests were done over a VPN link, somewhat limiting the achievable
>> throughput).
>>
>> Once more, thank you so much! This will greatly simplify much of our
>> operations. If there is anything else we can help test, let me know :)
> 
> Pfew, at least :) Thanks a lot for your patience, and doing all that testing !
> 
> Olivier
> 


Joining this again a bit late, do you still want me to test it?
I would like to know if there are any performance impact, but I see that
Conrad Hoffmann has done all the hard work on this. So, we can conclude that we
don't expect any performance impact.

Once again thanks a lot for your hard work on this.
@Conrad Hoffmann, thanks a lot for testing this and checking if there is any
performance impact.

Cheers,
Pavlos



signature.asc
Description: OpenPGP digital signature

Re: [RFC][PATCHES] seamless reload

[Willy Tarreau]

On Thu, Apr 13, 2017 at 06:18:59PM +0200, Olivier Houchard wrote:
> > Once more, thank you so much! This will greatly simplify much of our
> > operations. If there is anything else we can help test, let me know :)
> 
> Pfew, at least :) Thanks a lot for your patience, and doing all that testing !

Yep thanks for the testing and feedback, that's much appreciated. I was
also impressed by the efficiency of this change. We've been talking about
it for years, having had a few attempts at it in the past and this one
was the good one.

I've merged the patches already.

Great job, thanks guys!
Willy

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

On Thu, Apr 13, 2017 at 06:00:59PM +0200, Conrad Hoffmann wrote:
> On 04/13/2017 05:10 PM, Olivier Houchard wrote:
> > On Thu, Apr 13, 2017 at 04:59:26PM +0200, Conrad Hoffmann wrote:
> >> Sure, here it is ;P
> >>
> >> I now get a segfault (on reload):
> >>
> >> *** Error in `/usr/sbin/haproxy': corrupted double-linked list:
> >> 0x05b511e0 ***
> >>
> >> Here is the backtrace, retrieved from the core file:
> >>
> >> (gdb) bt
> >> #0  0x7f4c92801067 in __GI_raise (sig=sig@entry=6) at
> >> ../nptl/sysdeps/unix/sysv/linux/raise.c:56
> >> #1  0x7f4c92802448 in __GI_abort () at abort.c:89
> >> #2  0x7f4c9283f1b4 in __libc_message (do_abort=do_abort@entry=1,
> >> fmt=fmt@entry=0x7f4c92934210 "*** Error in `%s': %s: 0x%s ***\n") at
> >> ../sysdeps/posix/libc_fatal.c:175
> >> #3  0x7f4c9284498e in malloc_printerr (action=1, str=0x7f4c929302ec
> >> "corrupted double-linked list", ptr=) at malloc.c:4996
> >> #4  0x7f4c92845923 in _int_free (av=0x7f4c92b71620 ,
> >> p=, have_lock=0) at malloc.c:3996
> >> #5  0x00485850 in tcp_find_compatible_fd (l=0xaaed20) at
> >> src/proto_tcp.c:812
> >> #6  tcp_bind_listener (listener=0xaaed20, errmsg=0x7ffccc774e10 "",
> >> errlen=100) at src/proto_tcp.c:878
> >> #7  0x00493ce1 in start_proxies (verbose=0) at src/proxy.c:793
> >> #8  0x004091ec in main (argc=21, argv=0x7ffccc775168) at
> >> src/haproxy.c:1942
> > 
> > Ok, yet another stupid mistake, hopefully the attached patch fixes this :)
> > 
> > Thanks !
> > 
> > Olivier
> 
> 
> It does indeed! Not only does it work now, the result is impressive! Not a
> single dropped request even when aggressively reloading under substantial 
> load!
> 
> You certainly gave me an unexpected early easter present here :)
> 
> I will now head out, but I am planning on installing a 1.8 version with
> your patches on our canary pool (which receives a small amount of
> production traffic to test changes) after the holidays. I will happily test
> anything else that might be helpful for you. I will also set up a proper
> load test inside our data center then, but I expect no surprises there (my
> current tests were done over a VPN link, somewhat limiting the achievable
> throughput).
> 
> Once more, thank you so much! This will greatly simplify much of our
> operations. If there is anything else we can help test, let me know :)

Pfew, at least :) Thanks a lot for your patience, and doing all that testing !

Olivier

Re: [RFC][PATCHES] seamless reload

[Conrad Hoffmann]

On 04/13/2017 05:10 PM, Olivier Houchard wrote:
> On Thu, Apr 13, 2017 at 04:59:26PM +0200, Conrad Hoffmann wrote:
>> Sure, here it is ;P
>>
>> I now get a segfault (on reload):
>>
>> *** Error in `/usr/sbin/haproxy': corrupted double-linked list:
>> 0x05b511e0 ***
>>
>> Here is the backtrace, retrieved from the core file:
>>
>> (gdb) bt
>> #0  0x7f4c92801067 in __GI_raise (sig=sig@entry=6) at
>> ../nptl/sysdeps/unix/sysv/linux/raise.c:56
>> #1  0x7f4c92802448 in __GI_abort () at abort.c:89
>> #2  0x7f4c9283f1b4 in __libc_message (do_abort=do_abort@entry=1,
>> fmt=fmt@entry=0x7f4c92934210 "*** Error in `%s': %s: 0x%s ***\n") at
>> ../sysdeps/posix/libc_fatal.c:175
>> #3  0x7f4c9284498e in malloc_printerr (action=1, str=0x7f4c929302ec
>> "corrupted double-linked list", ptr=) at malloc.c:4996
>> #4  0x7f4c92845923 in _int_free (av=0x7f4c92b71620 ,
>> p=, have_lock=0) at malloc.c:3996
>> #5  0x00485850 in tcp_find_compatible_fd (l=0xaaed20) at
>> src/proto_tcp.c:812
>> #6  tcp_bind_listener (listener=0xaaed20, errmsg=0x7ffccc774e10 "",
>> errlen=100) at src/proto_tcp.c:878
>> #7  0x00493ce1 in start_proxies (verbose=0) at src/proxy.c:793
>> #8  0x004091ec in main (argc=21, argv=0x7ffccc775168) at
>> src/haproxy.c:1942
> 
> Ok, yet another stupid mistake, hopefully the attached patch fixes this :)
> 
> Thanks !
> 
> Olivier


It does indeed! Not only does it work now, the result is impressive! Not a
single dropped request even when aggressively reloading under substantial load!

You certainly gave me an unexpected early easter present here :)

I will now head out, but I am planning on installing a 1.8 version with
your patches on our canary pool (which receives a small amount of
production traffic to test changes) after the holidays. I will happily test
anything else that might be helpful for you. I will also set up a proper
load test inside our data center then, but I expect no surprises there (my
current tests were done over a VPN link, somewhat limiting the achievable
throughput).

Once more, thank you so much! This will greatly simplify much of our
operations. If there is anything else we can help test, let me know :)

Cheers,
Conrad
-- 
Conrad Hoffmann
Traffic Engineer

SoundCloud Ltd. | Rheinsberger Str. 76/77, 10115 Berlin, Germany

Managing Director: Alexander Ljung | Incorporated in England & Wales
with Company No. 6343600 | Local Branch Office | AG Charlottenburg |
HRB 110657B

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

On Thu, Apr 13, 2017 at 04:59:26PM +0200, Conrad Hoffmann wrote:
> Sure, here it is ;P
> 
> I now get a segfault (on reload):
> 
> *** Error in `/usr/sbin/haproxy': corrupted double-linked list:
> 0x05b511e0 ***
> 
> Here is the backtrace, retrieved from the core file:
> 
> (gdb) bt
> #0  0x7f4c92801067 in __GI_raise (sig=sig@entry=6) at
> ../nptl/sysdeps/unix/sysv/linux/raise.c:56
> #1  0x7f4c92802448 in __GI_abort () at abort.c:89
> #2  0x7f4c9283f1b4 in __libc_message (do_abort=do_abort@entry=1,
> fmt=fmt@entry=0x7f4c92934210 "*** Error in `%s': %s: 0x%s ***\n") at
> ../sysdeps/posix/libc_fatal.c:175
> #3  0x7f4c9284498e in malloc_printerr (action=1, str=0x7f4c929302ec
> "corrupted double-linked list", ptr=) at malloc.c:4996
> #4  0x7f4c92845923 in _int_free (av=0x7f4c92b71620 ,
> p=, have_lock=0) at malloc.c:3996
> #5  0x00485850 in tcp_find_compatible_fd (l=0xaaed20) at
> src/proto_tcp.c:812
> #6  tcp_bind_listener (listener=0xaaed20, errmsg=0x7ffccc774e10 "",
> errlen=100) at src/proto_tcp.c:878
> #7  0x00493ce1 in start_proxies (verbose=0) at src/proxy.c:793
> #8  0x004091ec in main (argc=21, argv=0x7ffccc775168) at
> src/haproxy.c:1942

Ok, yet another stupid mistake, hopefully the attached patch fixes this :)

Thanks !

Olivier
>From 7c7fe0c00129d60617cba786cbec7bbdd9ce08f8 Mon Sep 17 00:00:00 2001
From: Olivier Houchard 
Date: Thu, 13 Apr 2017 17:06:53 +0200
Subject: [PATCH 12/12] BUG/MINOR: Properly remove the xfer_sock from the
 linked list.

Doubly linked list are hard to get right.
---
 src/proto_tcp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index f558f00..57d6fc1 100644
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -806,7 +806,7 @@ static int tcp_find_compatible_fd(struct listener *l)
if (xfer_sock->prev)
xfer_sock->prev->next = xfer_sock->next;
if (xfer_sock->next)
-   xfer_sock->next->prev = xfer_sock->next->prev;
+   xfer_sock->next->prev = xfer_sock->prev;
free(xfer_sock->iface);
free(xfer_sock->namespace);
free(xfer_sock);
-- 
2.9.3

Re: [RFC][PATCHES] seamless reload

[Conrad Hoffmann]

On 04/13/2017 03:50 PM, Olivier Houchard wrote:
> On Thu, Apr 13, 2017 at 03:06:47PM +0200, Conrad Hoffmann wrote:
>>
>>
>> On 04/13/2017 02:28 PM, Olivier Houchard wrote:
>>> On Thu, Apr 13, 2017 at 12:59:38PM +0200, Conrad Hoffmann wrote:
 On 04/13/2017 11:31 AM, Olivier Houchard wrote:
> On Thu, Apr 13, 2017 at 11:17:45AM +0200, Conrad Hoffmann wrote:
>> Hi Olivier,
>>
>> On 04/12/2017 06:09 PM, Olivier Houchard wrote:
>>> On Wed, Apr 12, 2017 at 05:50:54PM +0200, Olivier Houchard wrote:
 On Wed, Apr 12, 2017 at 05:30:17PM +0200, Conrad Hoffmann wrote:
> Hi again,
>
> so I tried to get this to work, but didn't manage yet. I also don't 
> quite
> understand how this is supposed to work. The first haproxy process is
> started _without_ the -x option, is that correct? Where does that 
> instance
> ever create the socket for transfer to later instances?
>
> I have it working now insofar that on reload, subsequent instances are
> spawned with the -x option, but they'll just complain that they can't 
> get
> anything from the unix socket (because, for all I can tell, it's not
> there?). I also can't see the relevant code path where this socket 
> gets
> created, but I didn't have time to read all of it yet.
>
> Am I doing something wrong? Did anyone get this to work with the
> systemd-wrapper so far?
>
> Also, but this might be a coincidence, my test setup takes a huge
> performance penalty just by applying your patches (without any 
> reloading
> whatsoever). Did this happen to anybody else? I'll send some numbers 
> and
> more details tomorrow.
>

 Ok I can confirm the performance issues, I'm investigating.

>>>
>>> Found it, I was messing with SO_LINGER when I shouldn't have been.
>>
>> 
>>
>> thanks a lot, I can confirm that the performance regression seems to be 
>> gone!
>>
>> I am still having the other (conceptual) problem, though. Sorry if this 
>> is
>> just me holding it wrong or something, it's been a while since I dug
>> through the internals of haproxy.
>>
>> So, as I mentioned before, we use nbproc (12) and the systemd-wrapper,
>> which in turn starts haproxy in daemon mode, giving us a process tree 
>> like
>> this (path and file names shortened for brevity):
>>
>> \_ /u/s/haproxy-systemd-wrapper -f ./hap.cfg -p /v/r/hap.pid
>> \_ /u/s/haproxy-master
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>>
>> Now, in our config file, we have something like this:
>>
>> # expose admin socket for each process
>>   stats socket ${STATS_ADDR}   level admin process 1
>>   stats socket ${STATS_ADDR}-2 level admin process 2
>>   stats socket ${STATS_ADDR}-3 level admin process 3
>>   stats socket ${STATS_ADDR}-4 level admin process 4
>>   stats socket ${STATS_ADDR}-5 level admin process 5
>>   stats socket ${STATS_ADDR}-6 level admin process 6
>>   stats socket ${STATS_ADDR}-7 level admin process 7
>>   stats socket ${STATS_ADDR}-8 level admin process 8
>>   stats socket ${STATS_ADDR}-9 level admin process 9
>>   stats socket ${STATS_ADDR}-10 level admin process 10
>>   stats socket ${STATS_ADDR}-11 level admin process 11
>>   stats socket ${STATS_ADDR}-12 level admin process 12
>>
>> Basically, we have a dedicate admin socket for each ("real") process, as 
>> we
>> need to be able to talk to each process individually. So I was wondering:
>> which admin socket should I pass as HAPROXY_STATS_SOCKET? I initially
>> thought it would have to be a special stats socket in the haproxy-master
>> process (which we currently don't have), but as far as I can tell from 
>> the
>> output of `lsof` the haproxy-master process doesn't even hold any FDs
>> anymore. Will this setup currently work with your patches at all? Do I 
>> need
>> to add a stats socket to the master process? Or would this require a list
>> of stats sockets to be passed, similar 

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

On Thu, Apr 13, 2017 at 03:06:47PM +0200, Conrad Hoffmann wrote:
> 
> 
> On 04/13/2017 02:28 PM, Olivier Houchard wrote:
> > On Thu, Apr 13, 2017 at 12:59:38PM +0200, Conrad Hoffmann wrote:
> >> On 04/13/2017 11:31 AM, Olivier Houchard wrote:
> >>> On Thu, Apr 13, 2017 at 11:17:45AM +0200, Conrad Hoffmann wrote:
>  Hi Olivier,
> 
>  On 04/12/2017 06:09 PM, Olivier Houchard wrote:
> > On Wed, Apr 12, 2017 at 05:50:54PM +0200, Olivier Houchard wrote:
> >> On Wed, Apr 12, 2017 at 05:30:17PM +0200, Conrad Hoffmann wrote:
> >>> Hi again,
> >>>
> >>> so I tried to get this to work, but didn't manage yet. I also don't 
> >>> quite
> >>> understand how this is supposed to work. The first haproxy process is
> >>> started _without_ the -x option, is that correct? Where does that 
> >>> instance
> >>> ever create the socket for transfer to later instances?
> >>>
> >>> I have it working now insofar that on reload, subsequent instances are
> >>> spawned with the -x option, but they'll just complain that they can't 
> >>> get
> >>> anything from the unix socket (because, for all I can tell, it's not
> >>> there?). I also can't see the relevant code path where this socket 
> >>> gets
> >>> created, but I didn't have time to read all of it yet.
> >>>
> >>> Am I doing something wrong? Did anyone get this to work with the
> >>> systemd-wrapper so far?
> >>>
> >>> Also, but this might be a coincidence, my test setup takes a huge
> >>> performance penalty just by applying your patches (without any 
> >>> reloading
> >>> whatsoever). Did this happen to anybody else? I'll send some numbers 
> >>> and
> >>> more details tomorrow.
> >>>
> >>
> >> Ok I can confirm the performance issues, I'm investigating.
> >>
> >
> > Found it, I was messing with SO_LINGER when I shouldn't have been.
> 
>  
> 
>  thanks a lot, I can confirm that the performance regression seems to be 
>  gone!
> 
>  I am still having the other (conceptual) problem, though. Sorry if this 
>  is
>  just me holding it wrong or something, it's been a while since I dug
>  through the internals of haproxy.
> 
>  So, as I mentioned before, we use nbproc (12) and the systemd-wrapper,
>  which in turn starts haproxy in daemon mode, giving us a process tree 
>  like
>  this (path and file names shortened for brevity):
> 
>  \_ /u/s/haproxy-systemd-wrapper -f ./hap.cfg -p /v/r/hap.pid
>  \_ /u/s/haproxy-master
>  \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>  \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>  \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>  \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>  \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>  \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>  \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>  \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>  \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>  \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>  \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>  \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> 
>  Now, in our config file, we have something like this:
> 
>  # expose admin socket for each process
>    stats socket ${STATS_ADDR}   level admin process 1
>    stats socket ${STATS_ADDR}-2 level admin process 2
>    stats socket ${STATS_ADDR}-3 level admin process 3
>    stats socket ${STATS_ADDR}-4 level admin process 4
>    stats socket ${STATS_ADDR}-5 level admin process 5
>    stats socket ${STATS_ADDR}-6 level admin process 6
>    stats socket ${STATS_ADDR}-7 level admin process 7
>    stats socket ${STATS_ADDR}-8 level admin process 8
>    stats socket ${STATS_ADDR}-9 level admin process 9
>    stats socket ${STATS_ADDR}-10 level admin process 10
>    stats socket ${STATS_ADDR}-11 level admin process 11
>    stats socket ${STATS_ADDR}-12 level admin process 12
> 
>  Basically, we have a dedicate admin socket for each ("real") process, as 
>  we
>  need to be able to talk to each process individually. So I was wondering:
>  which admin socket should I pass as HAPROXY_STATS_SOCKET? I initially
>  thought it would have to be a special stats socket in the haproxy-master
>  process (which we currently don't have), but as far as I can tell from 
>  the
>  output of `lsof` the haproxy-master process doesn't even hold any FDs
>  anymore. Will this setup currently work with your patches at all? Do I 
>  need
>  to add a stats socket to the master process? Or would this require a list
>  of stats sockets to be passed, similar to the list of PIDs that gets 
>  passed
>  

Re: [RFC][PATCHES] seamless reload

[Conrad Hoffmann]

On 04/13/2017 02:28 PM, Olivier Houchard wrote:
> On Thu, Apr 13, 2017 at 12:59:38PM +0200, Conrad Hoffmann wrote:
>> On 04/13/2017 11:31 AM, Olivier Houchard wrote:
>>> On Thu, Apr 13, 2017 at 11:17:45AM +0200, Conrad Hoffmann wrote:
 Hi Olivier,

 On 04/12/2017 06:09 PM, Olivier Houchard wrote:
> On Wed, Apr 12, 2017 at 05:50:54PM +0200, Olivier Houchard wrote:
>> On Wed, Apr 12, 2017 at 05:30:17PM +0200, Conrad Hoffmann wrote:
>>> Hi again,
>>>
>>> so I tried to get this to work, but didn't manage yet. I also don't 
>>> quite
>>> understand how this is supposed to work. The first haproxy process is
>>> started _without_ the -x option, is that correct? Where does that 
>>> instance
>>> ever create the socket for transfer to later instances?
>>>
>>> I have it working now insofar that on reload, subsequent instances are
>>> spawned with the -x option, but they'll just complain that they can't 
>>> get
>>> anything from the unix socket (because, for all I can tell, it's not
>>> there?). I also can't see the relevant code path where this socket gets
>>> created, but I didn't have time to read all of it yet.
>>>
>>> Am I doing something wrong? Did anyone get this to work with the
>>> systemd-wrapper so far?
>>>
>>> Also, but this might be a coincidence, my test setup takes a huge
>>> performance penalty just by applying your patches (without any reloading
>>> whatsoever). Did this happen to anybody else? I'll send some numbers and
>>> more details tomorrow.
>>>
>>
>> Ok I can confirm the performance issues, I'm investigating.
>>
>
> Found it, I was messing with SO_LINGER when I shouldn't have been.

 

 thanks a lot, I can confirm that the performance regression seems to be 
 gone!

 I am still having the other (conceptual) problem, though. Sorry if this is
 just me holding it wrong or something, it's been a while since I dug
 through the internals of haproxy.

 So, as I mentioned before, we use nbproc (12) and the systemd-wrapper,
 which in turn starts haproxy in daemon mode, giving us a process tree like
 this (path and file names shortened for brevity):

 \_ /u/s/haproxy-systemd-wrapper -f ./hap.cfg -p /v/r/hap.pid
 \_ /u/s/haproxy-master
 \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
 \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
 \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
 \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
 \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
 \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
 \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
 \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
 \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
 \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
 \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
 \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds

 Now, in our config file, we have something like this:

 # expose admin socket for each process
   stats socket ${STATS_ADDR}   level admin process 1
   stats socket ${STATS_ADDR}-2 level admin process 2
   stats socket ${STATS_ADDR}-3 level admin process 3
   stats socket ${STATS_ADDR}-4 level admin process 4
   stats socket ${STATS_ADDR}-5 level admin process 5
   stats socket ${STATS_ADDR}-6 level admin process 6
   stats socket ${STATS_ADDR}-7 level admin process 7
   stats socket ${STATS_ADDR}-8 level admin process 8
   stats socket ${STATS_ADDR}-9 level admin process 9
   stats socket ${STATS_ADDR}-10 level admin process 10
   stats socket ${STATS_ADDR}-11 level admin process 11
   stats socket ${STATS_ADDR}-12 level admin process 12

 Basically, we have a dedicate admin socket for each ("real") process, as we
 need to be able to talk to each process individually. So I was wondering:
 which admin socket should I pass as HAPROXY_STATS_SOCKET? I initially
 thought it would have to be a special stats socket in the haproxy-master
 process (which we currently don't have), but as far as I can tell from the
 output of `lsof` the haproxy-master process doesn't even hold any FDs
 anymore. Will this setup currently work with your patches at all? Do I need
 to add a stats socket to the master process? Or would this require a list
 of stats sockets to be passed, similar to the list of PIDs that gets passed
 to new haproxy instances, so that each process can talk to the one from
 which it is taking over the socket(s)? In case I need a stats socket for
 the master process, what would be the directive to create it?

>>>
>>> Hi Conrad,
>>>
>>> Any of those sockets will do. Each process are made to keep all the 
>>> listening 

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

On Thu, Apr 13, 2017 at 12:59:38PM +0200, Conrad Hoffmann wrote:
> On 04/13/2017 11:31 AM, Olivier Houchard wrote:
> > On Thu, Apr 13, 2017 at 11:17:45AM +0200, Conrad Hoffmann wrote:
> >> Hi Olivier,
> >>
> >> On 04/12/2017 06:09 PM, Olivier Houchard wrote:
> >>> On Wed, Apr 12, 2017 at 05:50:54PM +0200, Olivier Houchard wrote:
>  On Wed, Apr 12, 2017 at 05:30:17PM +0200, Conrad Hoffmann wrote:
> > Hi again,
> >
> > so I tried to get this to work, but didn't manage yet. I also don't 
> > quite
> > understand how this is supposed to work. The first haproxy process is
> > started _without_ the -x option, is that correct? Where does that 
> > instance
> > ever create the socket for transfer to later instances?
> >
> > I have it working now insofar that on reload, subsequent instances are
> > spawned with the -x option, but they'll just complain that they can't 
> > get
> > anything from the unix socket (because, for all I can tell, it's not
> > there?). I also can't see the relevant code path where this socket gets
> > created, but I didn't have time to read all of it yet.
> >
> > Am I doing something wrong? Did anyone get this to work with the
> > systemd-wrapper so far?
> >
> > Also, but this might be a coincidence, my test setup takes a huge
> > performance penalty just by applying your patches (without any reloading
> > whatsoever). Did this happen to anybody else? I'll send some numbers and
> > more details tomorrow.
> >
> 
>  Ok I can confirm the performance issues, I'm investigating.
> 
> >>>
> >>> Found it, I was messing with SO_LINGER when I shouldn't have been.
> >>
> >> 
> >>
> >> thanks a lot, I can confirm that the performance regression seems to be 
> >> gone!
> >>
> >> I am still having the other (conceptual) problem, though. Sorry if this is
> >> just me holding it wrong or something, it's been a while since I dug
> >> through the internals of haproxy.
> >>
> >> So, as I mentioned before, we use nbproc (12) and the systemd-wrapper,
> >> which in turn starts haproxy in daemon mode, giving us a process tree like
> >> this (path and file names shortened for brevity):
> >>
> >> \_ /u/s/haproxy-systemd-wrapper -f ./hap.cfg -p /v/r/hap.pid
> >> \_ /u/s/haproxy-master
> >> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> >> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> >> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> >> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> >> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> >> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> >> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> >> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> >> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> >> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> >> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> >> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> >>
> >> Now, in our config file, we have something like this:
> >>
> >> # expose admin socket for each process
> >>   stats socket ${STATS_ADDR}   level admin process 1
> >>   stats socket ${STATS_ADDR}-2 level admin process 2
> >>   stats socket ${STATS_ADDR}-3 level admin process 3
> >>   stats socket ${STATS_ADDR}-4 level admin process 4
> >>   stats socket ${STATS_ADDR}-5 level admin process 5
> >>   stats socket ${STATS_ADDR}-6 level admin process 6
> >>   stats socket ${STATS_ADDR}-7 level admin process 7
> >>   stats socket ${STATS_ADDR}-8 level admin process 8
> >>   stats socket ${STATS_ADDR}-9 level admin process 9
> >>   stats socket ${STATS_ADDR}-10 level admin process 10
> >>   stats socket ${STATS_ADDR}-11 level admin process 11
> >>   stats socket ${STATS_ADDR}-12 level admin process 12
> >>
> >> Basically, we have a dedicate admin socket for each ("real") process, as we
> >> need to be able to talk to each process individually. So I was wondering:
> >> which admin socket should I pass as HAPROXY_STATS_SOCKET? I initially
> >> thought it would have to be a special stats socket in the haproxy-master
> >> process (which we currently don't have), but as far as I can tell from the
> >> output of `lsof` the haproxy-master process doesn't even hold any FDs
> >> anymore. Will this setup currently work with your patches at all? Do I need
> >> to add a stats socket to the master process? Or would this require a list
> >> of stats sockets to be passed, similar to the list of PIDs that gets passed
> >> to new haproxy instances, so that each process can talk to the one from
> >> which it is taking over the socket(s)? In case I need a stats socket for
> >> the master process, what would be the directive to create it?
> >>
> > 
> > Hi Conrad,
> > 
> > Any of those sockets will do. Each process are made to keep all the 
> > listening sockets opened, even if the proxy is not bound to that 

Re: [RFC][PATCHES] seamless reload

[Conrad Hoffmann]

On 04/13/2017 11:31 AM, Olivier Houchard wrote:
> On Thu, Apr 13, 2017 at 11:17:45AM +0200, Conrad Hoffmann wrote:
>> Hi Olivier,
>>
>> On 04/12/2017 06:09 PM, Olivier Houchard wrote:
>>> On Wed, Apr 12, 2017 at 05:50:54PM +0200, Olivier Houchard wrote:
 On Wed, Apr 12, 2017 at 05:30:17PM +0200, Conrad Hoffmann wrote:
> Hi again,
>
> so I tried to get this to work, but didn't manage yet. I also don't quite
> understand how this is supposed to work. The first haproxy process is
> started _without_ the -x option, is that correct? Where does that instance
> ever create the socket for transfer to later instances?
>
> I have it working now insofar that on reload, subsequent instances are
> spawned with the -x option, but they'll just complain that they can't get
> anything from the unix socket (because, for all I can tell, it's not
> there?). I also can't see the relevant code path where this socket gets
> created, but I didn't have time to read all of it yet.
>
> Am I doing something wrong? Did anyone get this to work with the
> systemd-wrapper so far?
>
> Also, but this might be a coincidence, my test setup takes a huge
> performance penalty just by applying your patches (without any reloading
> whatsoever). Did this happen to anybody else? I'll send some numbers and
> more details tomorrow.
>

 Ok I can confirm the performance issues, I'm investigating.

>>>
>>> Found it, I was messing with SO_LINGER when I shouldn't have been.
>>
>> 
>>
>> thanks a lot, I can confirm that the performance regression seems to be gone!
>>
>> I am still having the other (conceptual) problem, though. Sorry if this is
>> just me holding it wrong or something, it's been a while since I dug
>> through the internals of haproxy.
>>
>> So, as I mentioned before, we use nbproc (12) and the systemd-wrapper,
>> which in turn starts haproxy in daemon mode, giving us a process tree like
>> this (path and file names shortened for brevity):
>>
>> \_ /u/s/haproxy-systemd-wrapper -f ./hap.cfg -p /v/r/hap.pid
>> \_ /u/s/haproxy-master
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
>>
>> Now, in our config file, we have something like this:
>>
>> # expose admin socket for each process
>>   stats socket ${STATS_ADDR}   level admin process 1
>>   stats socket ${STATS_ADDR}-2 level admin process 2
>>   stats socket ${STATS_ADDR}-3 level admin process 3
>>   stats socket ${STATS_ADDR}-4 level admin process 4
>>   stats socket ${STATS_ADDR}-5 level admin process 5
>>   stats socket ${STATS_ADDR}-6 level admin process 6
>>   stats socket ${STATS_ADDR}-7 level admin process 7
>>   stats socket ${STATS_ADDR}-8 level admin process 8
>>   stats socket ${STATS_ADDR}-9 level admin process 9
>>   stats socket ${STATS_ADDR}-10 level admin process 10
>>   stats socket ${STATS_ADDR}-11 level admin process 11
>>   stats socket ${STATS_ADDR}-12 level admin process 12
>>
>> Basically, we have a dedicate admin socket for each ("real") process, as we
>> need to be able to talk to each process individually. So I was wondering:
>> which admin socket should I pass as HAPROXY_STATS_SOCKET? I initially
>> thought it would have to be a special stats socket in the haproxy-master
>> process (which we currently don't have), but as far as I can tell from the
>> output of `lsof` the haproxy-master process doesn't even hold any FDs
>> anymore. Will this setup currently work with your patches at all? Do I need
>> to add a stats socket to the master process? Or would this require a list
>> of stats sockets to be passed, similar to the list of PIDs that gets passed
>> to new haproxy instances, so that each process can talk to the one from
>> which it is taking over the socket(s)? In case I need a stats socket for
>> the master process, what would be the directive to create it?
>>
> 
> Hi Conrad,
> 
> Any of those sockets will do. Each process are made to keep all the 
> listening sockets opened, even if the proxy is not bound to that specific
> process, justly so that it can be transferred via the unix socket.
> 
> Regards,
> 
> Olivier


Thanks, I am finally starting to understand, but I think there still might
be a problem. I didn't see that initially, but when I use one of the
processes existing admin sockets it 

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

On Thu, Apr 13, 2017 at 11:17:45AM +0200, Conrad Hoffmann wrote:
> Hi Olivier,
> 
> On 04/12/2017 06:09 PM, Olivier Houchard wrote:
> > On Wed, Apr 12, 2017 at 05:50:54PM +0200, Olivier Houchard wrote:
> >> On Wed, Apr 12, 2017 at 05:30:17PM +0200, Conrad Hoffmann wrote:
> >>> Hi again,
> >>>
> >>> so I tried to get this to work, but didn't manage yet. I also don't quite
> >>> understand how this is supposed to work. The first haproxy process is
> >>> started _without_ the -x option, is that correct? Where does that instance
> >>> ever create the socket for transfer to later instances?
> >>>
> >>> I have it working now insofar that on reload, subsequent instances are
> >>> spawned with the -x option, but they'll just complain that they can't get
> >>> anything from the unix socket (because, for all I can tell, it's not
> >>> there?). I also can't see the relevant code path where this socket gets
> >>> created, but I didn't have time to read all of it yet.
> >>>
> >>> Am I doing something wrong? Did anyone get this to work with the
> >>> systemd-wrapper so far?
> >>>
> >>> Also, but this might be a coincidence, my test setup takes a huge
> >>> performance penalty just by applying your patches (without any reloading
> >>> whatsoever). Did this happen to anybody else? I'll send some numbers and
> >>> more details tomorrow.
> >>>
> >>
> >> Ok I can confirm the performance issues, I'm investigating.
> >>
> > 
> > Found it, I was messing with SO_LINGER when I shouldn't have been.
> 
> 
> 
> thanks a lot, I can confirm that the performance regression seems to be gone!
> 
> I am still having the other (conceptual) problem, though. Sorry if this is
> just me holding it wrong or something, it's been a while since I dug
> through the internals of haproxy.
> 
> So, as I mentioned before, we use nbproc (12) and the systemd-wrapper,
> which in turn starts haproxy in daemon mode, giving us a process tree like
> this (path and file names shortened for brevity):
> 
> \_ /u/s/haproxy-systemd-wrapper -f ./hap.cfg -p /v/r/hap.pid
> \_ /u/s/haproxy-master
> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> \_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
> 
> Now, in our config file, we have something like this:
> 
> # expose admin socket for each process
>   stats socket ${STATS_ADDR}   level admin process 1
>   stats socket ${STATS_ADDR}-2 level admin process 2
>   stats socket ${STATS_ADDR}-3 level admin process 3
>   stats socket ${STATS_ADDR}-4 level admin process 4
>   stats socket ${STATS_ADDR}-5 level admin process 5
>   stats socket ${STATS_ADDR}-6 level admin process 6
>   stats socket ${STATS_ADDR}-7 level admin process 7
>   stats socket ${STATS_ADDR}-8 level admin process 8
>   stats socket ${STATS_ADDR}-9 level admin process 9
>   stats socket ${STATS_ADDR}-10 level admin process 10
>   stats socket ${STATS_ADDR}-11 level admin process 11
>   stats socket ${STATS_ADDR}-12 level admin process 12
> 
> Basically, we have a dedicate admin socket for each ("real") process, as we
> need to be able to talk to each process individually. So I was wondering:
> which admin socket should I pass as HAPROXY_STATS_SOCKET? I initially
> thought it would have to be a special stats socket in the haproxy-master
> process (which we currently don't have), but as far as I can tell from the
> output of `lsof` the haproxy-master process doesn't even hold any FDs
> anymore. Will this setup currently work with your patches at all? Do I need
> to add a stats socket to the master process? Or would this require a list
> of stats sockets to be passed, similar to the list of PIDs that gets passed
> to new haproxy instances, so that each process can talk to the one from
> which it is taking over the socket(s)? In case I need a stats socket for
> the master process, what would be the directive to create it?
> 

Hi Conrad,

Any of those sockets will do. Each process are made to keep all the 
listening sockets opened, even if the proxy is not bound to that specific
process, justly so that it can be transferred via the unix socket.

Regards,

Olivier

Re: [RFC][PATCHES] seamless reload

[Conrad Hoffmann]

Hi Olivier,

On 04/12/2017 06:09 PM, Olivier Houchard wrote:
> On Wed, Apr 12, 2017 at 05:50:54PM +0200, Olivier Houchard wrote:
>> On Wed, Apr 12, 2017 at 05:30:17PM +0200, Conrad Hoffmann wrote:
>>> Hi again,
>>>
>>> so I tried to get this to work, but didn't manage yet. I also don't quite
>>> understand how this is supposed to work. The first haproxy process is
>>> started _without_ the -x option, is that correct? Where does that instance
>>> ever create the socket for transfer to later instances?
>>>
>>> I have it working now insofar that on reload, subsequent instances are
>>> spawned with the -x option, but they'll just complain that they can't get
>>> anything from the unix socket (because, for all I can tell, it's not
>>> there?). I also can't see the relevant code path where this socket gets
>>> created, but I didn't have time to read all of it yet.
>>>
>>> Am I doing something wrong? Did anyone get this to work with the
>>> systemd-wrapper so far?
>>>
>>> Also, but this might be a coincidence, my test setup takes a huge
>>> performance penalty just by applying your patches (without any reloading
>>> whatsoever). Did this happen to anybody else? I'll send some numbers and
>>> more details tomorrow.
>>>
>>
>> Ok I can confirm the performance issues, I'm investigating.
>>
> 
> Found it, I was messing with SO_LINGER when I shouldn't have been.



thanks a lot, I can confirm that the performance regression seems to be gone!

I am still having the other (conceptual) problem, though. Sorry if this is
just me holding it wrong or something, it's been a while since I dug
through the internals of haproxy.

So, as I mentioned before, we use nbproc (12) and the systemd-wrapper,
which in turn starts haproxy in daemon mode, giving us a process tree like
this (path and file names shortened for brevity):

\_ /u/s/haproxy-systemd-wrapper -f ./hap.cfg -p /v/r/hap.pid
\_ /u/s/haproxy-master
\_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
\_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
\_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
\_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
\_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
\_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
\_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
\_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
\_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
\_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
\_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds
\_ /u/s/haproxy -f ./hap.cfg -p /v/r/hap.pid -Ds

Now, in our config file, we have something like this:

# expose admin socket for each process
  stats socket ${STATS_ADDR}   level admin process 1
  stats socket ${STATS_ADDR}-2 level admin process 2
  stats socket ${STATS_ADDR}-3 level admin process 3
  stats socket ${STATS_ADDR}-4 level admin process 4
  stats socket ${STATS_ADDR}-5 level admin process 5
  stats socket ${STATS_ADDR}-6 level admin process 6
  stats socket ${STATS_ADDR}-7 level admin process 7
  stats socket ${STATS_ADDR}-8 level admin process 8
  stats socket ${STATS_ADDR}-9 level admin process 9
  stats socket ${STATS_ADDR}-10 level admin process 10
  stats socket ${STATS_ADDR}-11 level admin process 11
  stats socket ${STATS_ADDR}-12 level admin process 12

Basically, we have a dedicate admin socket for each ("real") process, as we
need to be able to talk to each process individually. So I was wondering:
which admin socket should I pass as HAPROXY_STATS_SOCKET? I initially
thought it would have to be a special stats socket in the haproxy-master
process (which we currently don't have), but as far as I can tell from the
output of `lsof` the haproxy-master process doesn't even hold any FDs
anymore. Will this setup currently work with your patches at all? Do I need
to add a stats socket to the master process? Or would this require a list
of stats sockets to be passed, similar to the list of PIDs that gets passed
to new haproxy instances, so that each process can talk to the one from
which it is taking over the socket(s)? In case I need a stats socket for
the master process, what would be the directive to create it?

Thanks a lot,
Conrad
-- 
Conrad Hoffmann
Traffic Engineer

SoundCloud Ltd. | Rheinsberger Str. 76/77, 10115 Berlin, Germany

Managing Director: Alexander Ljung | Incorporated in England & Wales
with Company No. 6343600 | Local Branch Office | AG Charlottenburg |
HRB 110657B

Re: [RFC][PATCHES] seamless reload

[Willy Tarreau]

On Wed, Apr 12, 2017 at 07:41:43PM +0200, Olivier Houchard wrote:
> + if (default_tcp_maxseg == -1) {
> + default_tcp_maxseg = -2;
> + fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
> + if (fd < 0)
> + Warning("Failed to create a temporary socket!\n");
> + else {
> + if (getsockopt(fd, IPPROTO_TCP, TCP_MAXSEG, 
> _tcp_maxseg,
> + _len) == -1)
> + Warning("Failed to get the default value of 
> TCP_MAXSEG\n");

Olivier, you're missing a close(fd) here, it'll leak this fd.

> + }
> + }
> + if (default_tcp6_maxseg == -1) {
> + default_tcp6_maxseg = -2;
> + fd = socket(AF_INET6, SOCK_STREAM, IPPROTO_TCP);
> + if (fd >= 0) {
> + if (getsockopt(fd, IPPROTO_TCP, TCP_MAXSEG, 
> _tcp6_maxseg,
> + _len) == -1)
> + Warning("Failed ot get the default value of 
> TCP_MAXSEG for IPv6\n");
> + close(fd);
> + }
> + }
> +#endif
> +
>  
(...)

> + getsockopt(fd, IPPROTO_TCP, TCP_MAXSEG, , );
> + if (tmpmaxseg != defaultmss &&
> + setsockopt(fd, IPPROTO_TCP, TCP_MAXSEG,
> + , sizeof(defaultmss)) == -1) {

Please fix the alignment for the argument here, it's a bit confusing :-)

Otherwise looks good. I think it was a good idea to create temporary
sockets to retrieve some default settings. That may be something we
could generalize to guess other parameters or capabilities if needed
in the future. For example we could use this to detect whether or not
IPv6 is supported and emit errors only once instead of every bind line.

Another use case is to always know the MSS applied to a listening socket
in order to automatically adjust the SSL maxrecord instead of assuming
1460 by default. Over time we might find other use cases.

Cheers,
Willy

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

On Wed, Apr 12, 2017 at 11:19:37AM -0700, Steven Davidovitz wrote:
> I had a problem testing it on Mac OS X, because cmsghdr is aligned to 4
> bytes. I changed the CMSG_ALIGN(sizeof(struct cmsghdr)) call to CMSG_LEN(0)
> to fix it.
> 

Oh right, I'll change that.
Thanks a lot !

Olivier

Re: [RFC][PATCHES] seamless reload

[Steven Davidovitz]

I had a problem testing it on Mac OS X, because cmsghdr is aligned to 4
bytes. I changed the CMSG_ALIGN(sizeof(struct cmsghdr)) call to CMSG_LEN(0)
to fix it.

On Wed, Apr 12, 2017 at 10:41 AM, Olivier Houchard 
wrote:

> Yet another patch, on top of the previous ones.
> This one tries to get the default value of TCP_MAXSEG by creating a
> temporary
> TCP socket, so that one can remove the "mss" entry from its configuration
> file,
>  and reset the mss value for any transferred socket from the old process.
>
> Olivier
>

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

Yet another patch, on top of the previous ones.
This one tries to get the default value of TCP_MAXSEG by creating a temporary 
TCP socket, so that one can remove the "mss" entry from its configuration file,
 and reset the mss value for any transferred socket from the old process.

Olivier
>From 7dc2432f3a7c4a9e9531adafa4524a199e394f90 Mon Sep 17 00:00:00 2001
From: Olivier Houchard 
Date: Wed, 12 Apr 2017 19:32:15 +0200
Subject: [PATCH 10/10] MINOR: tcp: Attempt to reset TCP_MAXSEG when reusing a
 socket.

Guess the default value for TCP_MAXSEG by binding a temporary TCP socket and
getting it, and use that in case the we're reusing a socket from the old
process, and its TCP_MAXSEG is different. That way one can reset the
TCP_MAXSEG value to the default one when reusing sockets.
---
 src/proto_tcp.c | 55 ---
 1 file changed, 52 insertions(+), 3 deletions(-)

diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index ea6b8f7..8050f3e 100644
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -110,6 +110,12 @@ static struct protocol proto_tcpv6 = {
.nb_listeners = 0,
 };
 
+/* Default TCP parameters, got by opening a temporary TCP socket. */
+#ifdef TCP_MAXSEG
+static int default_tcp_maxseg = -1;
+static int default_tcp6_maxseg = -1;
+#endif
+
 /* Binds ipv4/ipv6 address  to socket , unless  is set, in 
which
  * case we try to bind .  is a 2-bit field consisting of :
  *  - 0 : ignore remote address (may even be a NULL pointer)
@@ -829,6 +835,35 @@ int tcp_bind_listener(struct listener *listener, char 
*errmsg, int errlen)
int ext, ready;
socklen_t ready_len;
const char *msg = NULL;
+#ifdef TCP_MAXSEG
+
+   /* Create a temporary TCP socket to get default parameters we can't
+* guess.
+* */
+   ready_len = sizeof(default_tcp_maxseg);
+   if (default_tcp_maxseg == -1) {
+   default_tcp_maxseg = -2;
+   fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+   if (fd < 0)
+   Warning("Failed to create a temporary socket!\n");
+   else {
+   if (getsockopt(fd, IPPROTO_TCP, TCP_MAXSEG, 
_tcp_maxseg,
+   _len) == -1)
+   Warning("Failed to get the default value of 
TCP_MAXSEG\n");
+   }
+   }
+   if (default_tcp6_maxseg == -1) {
+   default_tcp6_maxseg = -2;
+   fd = socket(AF_INET6, SOCK_STREAM, IPPROTO_TCP);
+   if (fd >= 0) {
+   if (getsockopt(fd, IPPROTO_TCP, TCP_MAXSEG, 
_tcp6_maxseg,
+   _len) == -1)
+   Warning("Failed ot get the default value of 
TCP_MAXSEG for IPv6\n");
+   close(fd);
+   }
+   }
+#endif
+
 
/* ensure we never return garbage */
if (errlen)
@@ -960,10 +995,24 @@ int tcp_bind_listener(struct listener *listener, char 
*errmsg, int errlen)
msg = "cannot set MSS";
err |= ERR_WARN;
}
+   } else if (ext) {
+   int tmpmaxseg = -1;
+   int defaultmss;
+   socklen_t len = sizeof(tmpmaxseg);
+
+   if (listener->addr.ss_family == AF_INET)
+   defaultmss = default_tcp_maxseg;
+   else
+   defaultmss = default_tcp6_maxseg;
+
+   getsockopt(fd, IPPROTO_TCP, TCP_MAXSEG, , );
+   if (tmpmaxseg != defaultmss &&
+   setsockopt(fd, IPPROTO_TCP, TCP_MAXSEG,
+   , sizeof(defaultmss)) == -1) {
+   msg = "cannot set MSS";
+   err |= ERR_WARN;
+   }
}
-   /* XXX: No else, the way to get the default MSS will vary from system
-* to system.
-*/
 #endif
 #if defined(TCP_USER_TIMEOUT)
if (listener->tcp_ut) {
-- 
2.9.3

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

On Wed, Apr 12, 2017 at 05:50:54PM +0200, Olivier Houchard wrote:
> On Wed, Apr 12, 2017 at 05:30:17PM +0200, Conrad Hoffmann wrote:
> > Hi again,
> > 
> > so I tried to get this to work, but didn't manage yet. I also don't quite
> > understand how this is supposed to work. The first haproxy process is
> > started _without_ the -x option, is that correct? Where does that instance
> > ever create the socket for transfer to later instances?
> > 
> > I have it working now insofar that on reload, subsequent instances are
> > spawned with the -x option, but they'll just complain that they can't get
> > anything from the unix socket (because, for all I can tell, it's not
> > there?). I also can't see the relevant code path where this socket gets
> > created, but I didn't have time to read all of it yet.
> > 
> > Am I doing something wrong? Did anyone get this to work with the
> > systemd-wrapper so far?
> > 
> > Also, but this might be a coincidence, my test setup takes a huge
> > performance penalty just by applying your patches (without any reloading
> > whatsoever). Did this happen to anybody else? I'll send some numbers and
> > more details tomorrow.
> > 
> 
> Ok I can confirm the performance issues, I'm investigating.
> 

Found it, I was messing with SO_LINGER when I shouldn't have been.

Can you try the new version of
0003-MINOR-tcp-When-binding-socket-attempt-to-reuse-one-f.patch
or replace, in src/proto_tcp.c :
if (getsockopt(fd, SOL_SOCKET, SO_LINGER, , ) == 0 &&
(tmplinger.l_onoff == 0 || tmplinger.l_linger == 0)) {
/* Attempt to activate SO_LINGER, not sure what a sane
 * value is, using the default BSD value of 120s.
 */
tmplinger.l_onoff = 1;
tmplinger.l_linger = 120;
setsockopt(fd, SOL_SOCKET, SO_LINGER, ,
sizeof(tmplinger));
}


by :
if (getsockopt(fd, SOL_SOCKET, SO_LINGER, , ) == 0 &&
(tmplinger.l_onoff == 1 || tmplinger.l_linger == 0)) {
tmplinger.l_onoff = 0;
tmplinger.l_linger = 0;
setsockopt(fd, SOL_SOCKET, SO_LINGER, ,
sizeof(tmplinger));
}
}


Thanks !

Olivier
>From 1bf2b9c550285b434c865adeb175277ce00c842b Mon Sep 17 00:00:00 2001
From: Olivier Houchard 
Date: Wed, 5 Apr 2017 22:39:56 +0200
Subject: [PATCH 3/9] MINOR: tcp: When binding socket, attempt to reuse one
 from the old proc.

Try to reuse any socket from the old process, provided by the "-x" flag,
before binding a new one, assuming it is compatible.
"Compatible" here means same address and port, same namspace if any,
same interface if any, and that the following flags are the same :
LI_O_FOREIGN, LI_O_V6ONLY and LI_O_V4V6.
Also change tcp_bind_listener() to always enable/disable socket options,
instead of just doing so if it is in the configuration file, as the option
may have been removed, ie TCP_FASTOPEN may have been set in the old process,
and removed from the new configuration, so we have to disable it.
---
 src/proto_tcp.c | 119 +++-
 1 file changed, 117 insertions(+), 2 deletions(-)

diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index 5e12b99..ea6b8f7 100644
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -731,6 +731,83 @@ int tcp_connect_probe(struct connection *conn)
return 0;
 }
 
+/* XXX: Should probably be elsewhere */
+static int compare_sockaddr(struct sockaddr_storage *a, struct 
sockaddr_storage *b)
+{
+   if (a->ss_family != b->ss_family) {
+   return (-1);
+   }
+   switch (a->ss_family) {
+   case AF_INET:
+   {
+   struct sockaddr_in *a4 = (void *)a, *b4 = (void *)b;
+   if (a4->sin_port != b4->sin_port)
+   return (-1);
+   return (memcmp(>sin_addr, >sin_addr,
+   sizeof(a4->sin_addr)));
+   }
+   case AF_INET6:
+   {
+   struct sockaddr_in6 *a6 = (void *)a, *b6 = (void *)b;
+   if (a6->sin6_port != b6->sin6_port)
+   return (-1);
+   return (memcmp(>sin6_addr, >sin6_addr,
+   sizeof(a6->sin6_addr)));
+   }
+   default:
+   return (-1);
+   }
+
+}
+
+#define LI_MANDATORY_FLAGS (LI_O_FOREIGN | LI_O_V6ONLY | LI_O_V4V6)
+/* When binding the listeners, check if a socket has been sent to us by the
+ * previous process that we could reuse, instead of creating a new one.
+ */
+static int tcp_find_compatible_fd(struct listener *l)
+{
+   struct xfer_sock_list *xfer_sock = xfer_sock_list;
+   int ret = -1;
+
+   while (xfer_sock) {
+   

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

On Wed, Apr 12, 2017 at 05:30:17PM +0200, Conrad Hoffmann wrote:
> Hi again,
> 
> so I tried to get this to work, but didn't manage yet. I also don't quite
> understand how this is supposed to work. The first haproxy process is
> started _without_ the -x option, is that correct? Where does that instance
> ever create the socket for transfer to later instances?
> 
> I have it working now insofar that on reload, subsequent instances are
> spawned with the -x option, but they'll just complain that they can't get
> anything from the unix socket (because, for all I can tell, it's not
> there?). I also can't see the relevant code path where this socket gets
> created, but I didn't have time to read all of it yet.
> 
> Am I doing something wrong? Did anyone get this to work with the
> systemd-wrapper so far?
> 
> Also, but this might be a coincidence, my test setup takes a huge
> performance penalty just by applying your patches (without any reloading
> whatsoever). Did this happen to anybody else? I'll send some numbers and
> more details tomorrow.
> 

Ok I can confirm the performance issues, I'm investigating.

Olivier

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

On Wed, Apr 12, 2017 at 05:30:17PM +0200, Conrad Hoffmann wrote:
> Hi again,
> 
> so I tried to get this to work, but didn't manage yet. I also don't quite
> understand how this is supposed to work. The first haproxy process is
> started _without_ the -x option, is that correct? Where does that instance
> ever create the socket for transfer to later instances?
> 
> I have it working now insofar that on reload, subsequent instances are
> spawned with the -x option, but they'll just complain that they can't get
> anything from the unix socket (because, for all I can tell, it's not
> there?). I also can't see the relevant code path where this socket gets
> created, but I didn't have time to read all of it yet.
> 
> Am I doing something wrong? Did anyone get this to work with the
> systemd-wrapper so far?
> 

You're right, the first one runs without -x. The socket used is just any
stats socket.

> Also, but this might be a coincidence, my test setup takes a huge
> performance penalty just by applying your patches (without any reloading
> whatsoever). Did this happen to anybody else? I'll send some numbers and
> more details tomorrow.
> 

Ah this is news to me, I did not expect a performances impact, can you share
your configuration file ?

Thanks !

Olivier

Re: [RFC][PATCHES] seamless reload

[Conrad Hoffmann]

Hi again,

so I tried to get this to work, but didn't manage yet. I also don't quite
understand how this is supposed to work. The first haproxy process is
started _without_ the -x option, is that correct? Where does that instance
ever create the socket for transfer to later instances?

I have it working now insofar that on reload, subsequent instances are
spawned with the -x option, but they'll just complain that they can't get
anything from the unix socket (because, for all I can tell, it's not
there?). I also can't see the relevant code path where this socket gets
created, but I didn't have time to read all of it yet.

Am I doing something wrong? Did anyone get this to work with the
systemd-wrapper so far?

Also, but this might be a coincidence, my test setup takes a huge
performance penalty just by applying your patches (without any reloading
whatsoever). Did this happen to anybody else? I'll send some numbers and
more details tomorrow.

Thanks a lot,
Conrad

On 04/12/2017 03:47 PM, Conrad Hoffmann wrote:
> On 04/12/2017 03:37 PM, Olivier Houchard wrote:
>> On Wed, Apr 12, 2017 at 03:16:31PM +0200, Conrad Hoffmann wrote:
>>> Hi Olivier,
>>>
>>> I was very eager to try out your patch set, thanks a lot! However, after
>>> applying all of them (including the last three), it seems that using
>>> environment variables in the config is broken (i.e. ${VARNAME} does not get
>>> replaced with the value of the environment variable anymore). I am using
>>> the systemd-wrapper, but not systemd.
>>>
>>> Just from looking at the patches I couldn't make out what exactly might be
>>> the problem. I guess it could be either the wrapper not passing on its
>>> environment properly or haproxy not using it properly anymore. If you have
>>> any immediate ideas, let me know. I'll try to fully debug this when I can
>>> spare the time.
>>>
>>> Thanks a lot,
>>> Conrad
>>>
>>
>> Hi Conrad,
>>
>> You're right, that was just me being an idiot :)
>> Please replace 
>> 0007-MINOR-systemd-wrapper-add-support-for-passing-the-x-.patch
>> with the one attached, or just replace in src/haproxy-systemd-wrapper.c :
>>
>> argv = calloc(4 + main_argc + nb_pid + 1 +
>>  stats_socket != NULL ? 2 : 0, sizeof(char *));
>> by :
>> argv = calloc(4 + main_argc + nb_pid + 1 +
>>  (stats_socket != NULL ? 2 : 0), sizeof(char *));
>>
>> Regards,
>>
>> Olivier
> 
> 
> Works indeed, hard to spot :)
> 
> Thanks a lot, I'll now get back to testing the actual reloads!
> 
> Conrad
> 

-- 
Conrad Hoffmann
Traffic Engineer

SoundCloud Ltd. | Rheinsberger Str. 76/77, 10115 Berlin, Germany

Managing Director: Alexander Ljung | Incorporated in England & Wales
with Company No. 6343600 | Local Branch Office | AG Charlottenburg |
HRB 110657B

Re: [RFC][PATCHES] seamless reload

[Conrad Hoffmann]

On 04/12/2017 03:37 PM, Olivier Houchard wrote:
> On Wed, Apr 12, 2017 at 03:16:31PM +0200, Conrad Hoffmann wrote:
>> Hi Olivier,
>>
>> I was very eager to try out your patch set, thanks a lot! However, after
>> applying all of them (including the last three), it seems that using
>> environment variables in the config is broken (i.e. ${VARNAME} does not get
>> replaced with the value of the environment variable anymore). I am using
>> the systemd-wrapper, but not systemd.
>>
>> Just from looking at the patches I couldn't make out what exactly might be
>> the problem. I guess it could be either the wrapper not passing on its
>> environment properly or haproxy not using it properly anymore. If you have
>> any immediate ideas, let me know. I'll try to fully debug this when I can
>> spare the time.
>>
>> Thanks a lot,
>> Conrad
>>
> 
> Hi Conrad,
> 
> You're right, that was just me being an idiot :)
> Please replace 0007-MINOR-systemd-wrapper-add-support-for-passing-the-x-.patch
> with the one attached, or just replace in src/haproxy-systemd-wrapper.c :
> 
> argv = calloc(4 + main_argc + nb_pid + 1 +
>   stats_socket != NULL ? 2 : 0, sizeof(char *));
> by :
> argv = calloc(4 + main_argc + nb_pid + 1 +
>   (stats_socket != NULL ? 2 : 0), sizeof(char *));
> 
> Regards,
> 
> Olivier


Works indeed, hard to spot :)

Thanks a lot, I'll now get back to testing the actual reloads!

Conrad
-- 
Conrad Hoffmann
Traffic Engineer

SoundCloud Ltd. | Rheinsberger Str. 76/77, 10115 Berlin, Germany

Managing Director: Alexander Ljung | Incorporated in England & Wales
with Company No. 6343600 | Local Branch Office | AG Charlottenburg |
HRB 110657B

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

On Wed, Apr 12, 2017 at 03:16:31PM +0200, Conrad Hoffmann wrote:
> Hi Olivier,
> 
> I was very eager to try out your patch set, thanks a lot! However, after
> applying all of them (including the last three), it seems that using
> environment variables in the config is broken (i.e. ${VARNAME} does not get
> replaced with the value of the environment variable anymore). I am using
> the systemd-wrapper, but not systemd.
> 
> Just from looking at the patches I couldn't make out what exactly might be
> the problem. I guess it could be either the wrapper not passing on its
> environment properly or haproxy not using it properly anymore. If you have
> any immediate ideas, let me know. I'll try to fully debug this when I can
> spare the time.
> 
> Thanks a lot,
> Conrad
> 

Hi Conrad,

You're right, that was just me being an idiot :)
Please replace 0007-MINOR-systemd-wrapper-add-support-for-passing-the-x-.patch
with the one attached, or just replace in src/haproxy-systemd-wrapper.c :

argv = calloc(4 + main_argc + nb_pid + 1 +
stats_socket != NULL ? 2 : 0, sizeof(char *));
by :
argv = calloc(4 + main_argc + nb_pid + 1 +
(stats_socket != NULL ? 2 : 0), sizeof(char *));

Regards,

Olivier
>From 526dca943b9cc89732c54bc43a6ce36e17b67890 Mon Sep 17 00:00:00 2001
From: Olivier Houchard 
Date: Sun, 9 Apr 2017 16:28:10 +0200
Subject: [PATCH 7/9] MINOR: systemd wrapper: add support for passing the -x
 option.

Make the systemd wrapper chech if HAPROXY_STATS_SOCKET if set.
If set, it will use it as an argument to the "-x" option, which makes
haproxy asks for any listening socket, on the stats socket, in order
to achieve reloads with no new connection lost.
---
 contrib/systemd/haproxy.service.in |  2 ++
 src/haproxy-systemd-wrapper.c  | 10 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/contrib/systemd/haproxy.service.in 
b/contrib/systemd/haproxy.service.in
index dca81a2..05bb716 100644
--- a/contrib/systemd/haproxy.service.in
+++ b/contrib/systemd/haproxy.service.in
@@ -3,6 +3,8 @@ Description=HAProxy Load Balancer
 After=network.target
 
 [Service]
+# You can point the environment variable HAPROXY_STATS_SOCKET to a stats
+# socket if you want seamless reloads.
 Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
 ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q
 ExecStart=@SBINDIR@/haproxy-systemd-wrapper -f $CONFIG -p $PIDFILE
diff --git a/src/haproxy-systemd-wrapper.c b/src/haproxy-systemd-wrapper.c
index f6a9c85..457f5bd 100644
--- a/src/haproxy-systemd-wrapper.c
+++ b/src/haproxy-systemd-wrapper.c
@@ -92,11 +92,15 @@ static void spawn_haproxy(char **pid_strv, int nb_pid)
pid = fork();
if (!pid) {
char **argv;
+   char *stats_socket = NULL;
int i;
int argno = 0;
 
/* 3 for "haproxy -Ds -sf" */
-   argv = calloc(4 + main_argc + nb_pid + 1, sizeof(char *));
+   if (nb_pid > 0)
+   stats_socket = getenv("HAPROXY_STATS_SOCKET");
+   argv = calloc(4 + main_argc + nb_pid + 1 +
+   (stats_socket != NULL ? 2 : 0), sizeof(char *));
if (!argv) {
fprintf(stderr, SD_NOTICE "haproxy-systemd-wrapper: 
failed to calloc(), please try again later.\n");
exit(1);
@@ -121,6 +125,10 @@ static void spawn_haproxy(char **pid_strv, int nb_pid)
argv[argno++] = "-sf";
for (i = 0; i < nb_pid; ++i)
argv[argno++] = pid_strv[i];
+   if (stats_socket != NULL) {
+   argv[argno++] = "-x";
+   argv[argno++] = stats_socket;
+   }
}
argv[argno] = NULL;
 
-- 
2.9.3

Re: [RFC][PATCHES] seamless reload

[Conrad Hoffmann]

Hi Olivier,

I was very eager to try out your patch set, thanks a lot! However, after
applying all of them (including the last three), it seems that using
environment variables in the config is broken (i.e. ${VARNAME} does not get
replaced with the value of the environment variable anymore). I am using
the systemd-wrapper, but not systemd.

Just from looking at the patches I couldn't make out what exactly might be
the problem. I guess it could be either the wrapper not passing on its
environment properly or haproxy not using it properly anymore. If you have
any immediate ideas, let me know. I'll try to fully debug this when I can
spare the time.

Thanks a lot,
Conrad

On 04/10/2017 08:09 PM, Olivier Houchard wrote:
> 
> Hi,
> 
> On top of those patches, here a 3 more patches.
> The first one makes the systemd wrapper check for a HAPROXY_STATS_SOCKET
> environment variable. If set, it will use that as an argument to -x, when
> reloading the process.
> The second one sends listening unix sockets, as well as IPv4/v6 sockets. 
> I see no reason not to, and that means we no longer have to wait until
> the old process close the socket before being able to accept new connections
> on it.
> The third one adds a new global optoin, nosockettransfer, if set, we assume
> we will never try to transfer listening sockets through the stats socket,
> and close any socket nout bound to our process, to save a few file
> descriptors.
> 
> Regards,
> 
> Olivier
> 

-- 
Conrad Hoffmann
Traffic Engineer

SoundCloud Ltd. | Rheinsberger Str. 76/77, 10115 Berlin, Germany

Managing Director: Alexander Ljung | Incorporated in England & Wales
with Company No. 6343600 | Local Branch Office | AG Charlottenburg |
HRB 110657B

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

On Tue, Apr 11, 2017 at 08:16:48PM +0200, Willy Tarreau wrote:
> Hi guys,
> 
> On Mon, Apr 10, 2017 at 11:08:56PM +0200, Pavlos Parissis wrote:
> > IMHO: a better name would be 'stats nounsedsockets', as it is referring to a
> > generic functionality of UNIX stats socket, rather to a very specific 
> > functionality.
> 
> Just two things on this :
>   - it's not directly related to the stats socket but it's a global
> behaviour of the process so it should not be under "stats" ;
> 
>   - please guys, don't stick words without a '-' as a delimitor
> anymore, we've made this mistake in the past with "forceclose",
> "httpclose" or "dontlognull" or whatever and some of them are
> really hard to read. That's why we now split the words using
> dashes, so that would be "no-unused-sockets" or I don't remember
> the other name Olivier proposed, but please use the same principle
> which leaves no ambiguity on how to parse it.

Fine, just beacuse it's you, I'll change that.

Olivier

Re: [RFC][PATCHES] seamless reload

[Willy Tarreau]

Hi guys,

On Mon, Apr 10, 2017 at 11:08:56PM +0200, Pavlos Parissis wrote:
> IMHO: a better name would be 'stats nounsedsockets', as it is referring to a
> generic functionality of UNIX stats socket, rather to a very specific 
> functionality.

Just two things on this :
  - it's not directly related to the stats socket but it's a global
behaviour of the process so it should not be under "stats" ;

  - please guys, don't stick words without a '-' as a delimitor
anymore, we've made this mistake in the past with "forceclose",
"httpclose" or "dontlognull" or whatever and some of them are
really hard to read. That's why we now split the words using
dashes, so that would be "no-unused-sockets" or I don't remember
the other name Olivier proposed, but please use the same principle
which leaves no ambiguity on how to parse it.

Thanks!
Willy

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

On Tue, Apr 11, 2017 at 01:23:42PM +0200, Pavlos Parissis wrote:
> On 10/04/2017 11:52 μμ, Olivier Houchard wrote:
> > On Mon, Apr 10, 2017 at 11:08:56PM +0200, Pavlos Parissis wrote:
> >> On 10/04/2017 08:09 , Olivier Houchard wrote:
> >>>
> >>> Hi,
> >>>
> >>> On top of those patches, here a 3 more patches.
> >>> The first one makes the systemd wrapper check for a HAPROXY_STATS_SOCKET
> >>> environment variable. If set, it will use that as an argument to -x, when
> >>> reloading the process.
> >>
> >> I see you want to introduce a specific environment variable for this 
> >> functionality
> >> and then fetched in the code with getenv(). This is a way to do it.
> >>
> >> IMHO: I prefer to pass a value to an argument, for instance -x. It is also
> >> consistent with haproxy binary where someone uses -x argument as well.
> >>
> > 
> > I'm not sure what you mean by this ?
> > It is supposed to be directly the value given to -x as an argument.
> > 
> 
> Ignore what I wrote above as I was under the wrong impression that we need to
> pass -x to systemd wrapper during the reload, but the systemd wrapper is not
> invoked during the reload.
> 
> 
> >>> The second one sends listening unix sockets, as well as IPv4/v6 sockets. 
> >>> I see no reason not to, and that means we no longer have to wait until
> >>> the old process close the socket before being able to accept new 
> >>> connections
> >>> on it.
> >>
> >>> The third one adds a new global optoin, nosockettransfer, if set, we 
> >>> assume
> >>> we will never try to transfer listening sockets through the stats socket,
> >>> and close any socket nout bound to our process, to save a few file
> >>> descriptors.
> >>>
> >>
> >> IMHO: a better name would be 'stats nounsedsockets', as it is referring to 
> >> a
> >> generic functionality of UNIX stats socket, rather to a very specific 
> >> functionality.
> >>
> > 
> > Well really it is a global setting, maybe my wording isn't great, but it
> > makes haproxy close all sockets not bound to this process, as it used to,
> > instead of keeping them around in case somebody asks for them. 
> > 
> >> I hope tomorrow I will find some time to test your patches.
> >>
> > 
> > Thanks a lot ! This is greatly appreciated.
> > 
> 
> Do you have a certain way to test it?
> My plan is to have a stress environment where I fire-up X thousands of TCP
> connections and produce a graph, which contain number of TCP RST received from
> the client during a soft-reload of haproxy. I'll run this test against haproxy
> 1.8 with and without your patches. So, I will have a clear indication if your
> patches solved the issue.
> 

That sounds good to me. My testing involved an haproxy running with 4
processes, 2 listeners, 1000 ports for each, and 2 processes bound to each
listener, 2 injectors doing http requests (that merely got a redirect as an
answer from haproxy), one on each listener, and reloading haproxy in a loop.

Regards,

Olivier

Re: [RFC][PATCHES] seamless reload

[Pavlos Parissis]

On 10/04/2017 11:52 μμ, Olivier Houchard wrote:
> On Mon, Apr 10, 2017 at 11:08:56PM +0200, Pavlos Parissis wrote:
>> On 10/04/2017 08:09 , Olivier Houchard wrote:
>>>
>>> Hi,
>>>
>>> On top of those patches, here a 3 more patches.
>>> The first one makes the systemd wrapper check for a HAPROXY_STATS_SOCKET
>>> environment variable. If set, it will use that as an argument to -x, when
>>> reloading the process.
>>
>> I see you want to introduce a specific environment variable for this 
>> functionality
>> and then fetched in the code with getenv(). This is a way to do it.
>>
>> IMHO: I prefer to pass a value to an argument, for instance -x. It is also
>> consistent with haproxy binary where someone uses -x argument as well.
>>
> 
> I'm not sure what you mean by this ?
> It is supposed to be directly the value given to -x as an argument.
> 

Ignore what I wrote above as I was under the wrong impression that we need to
pass -x to systemd wrapper during the reload, but the systemd wrapper is not
invoked during the reload.


>>> The second one sends listening unix sockets, as well as IPv4/v6 sockets. 
>>> I see no reason not to, and that means we no longer have to wait until
>>> the old process close the socket before being able to accept new connections
>>> on it.
>>
>>> The third one adds a new global optoin, nosockettransfer, if set, we assume
>>> we will never try to transfer listening sockets through the stats socket,
>>> and close any socket nout bound to our process, to save a few file
>>> descriptors.
>>>
>>
>> IMHO: a better name would be 'stats nounsedsockets', as it is referring to a
>> generic functionality of UNIX stats socket, rather to a very specific 
>> functionality.
>>
> 
> Well really it is a global setting, maybe my wording isn't great, but it
> makes haproxy close all sockets not bound to this process, as it used to,
> instead of keeping them around in case somebody asks for them. 
> 
>> I hope tomorrow I will find some time to test your patches.
>>
> 
> Thanks a lot ! This is greatly appreciated.
> 

Do you have a certain way to test it?
My plan is to have a stress environment where I fire-up X thousands of TCP
connections and produce a graph, which contain number of TCP RST received from
the client during a soft-reload of haproxy. I'll run this test against haproxy
1.8 with and without your patches. So, I will have a clear indication if your
patches solved the issue.


Any ideas?

Cheers,
Pavlos



signature.asc
Description: OpenPGP digital signature

Re: [RFC][PATCHES] seamless reload

[Pavlos Parissis]

On 10/04/2017 11:48 μμ, Olivier Houchard wrote:
> On Mon, Apr 10, 2017 at 10:49:21PM +0200, Pavlos Parissis wrote:
>> On 07/04/2017 11:17 , Olivier Houchard wrote:
>>> On Fri, Apr 07, 2017 at 09:58:57PM +0200, Pavlos Parissis wrote:
 On 06/04/2017 04:57 , Olivier Houchard wrote:
> On Thu, Apr 06, 2017 at 04:56:47PM +0200, Pavlos Parissis wrote:
>> On 06/04/2017 04:25 , Olivier Houchard wrote:
>>> Hi,
>>>
>>> The attached patchset is the first cut at an attempt to work around the
>>> linux issues with SOREUSEPORT that makes haproxy refuse a few new 
>>> connections
>>> under heavy load.
>>> This works by transferring the existing sockets to the new process via 
>>> the
>>> stats socket. A new command-line flag has been added, -x, that takes the
>>> path to the unix socket as an argument, and if set, will attempt to 
>>> retrieve
>>> all the listening sockets;
>>> Right now, any error, either while connecting to the socket, or 
>>> retrieving
>>> the file descriptors, is fatal, but maybe it'd be better to just fall 
>>> back
>>> to the previous behavior instead of opening any missing socket ? I'm 
>>> still
>>> undecided about that.
>>>
>>> Any testing, comments, etc would be greatly appreciated.
>>>
>>
>> Does this patch set support HAProxy in multiprocess mode (nbproc > 1) ?
>>
>
> Hi Pavlos,
>
> If it does not, it's a bug :)
> In my few tests, it seemed to work.
>
> Olivier
>


 I run systems with systemd and I can't see how I can test the seamless 
 reload
 functionality ( thanks for that) without a proper support for the UNIX 
 socket
 file argument (-x) in the haproxy-systemd-wrapper.

 I believe you need to modify the wrapper to accept -x argument for a single
 UNIX socket file or -X for a directory path with multiple UNIX socket 
 files,
 when HAProxy runs in multi-process mode.

 What do you think?

 Cheers,
 Pavlos



>>>
>>>
>>> Hi Pavlos,
>>>
>>> I didn't consider systemd, so it may be I have to investigate there.
>>> You don't need to talk to all the processes to get the sockets, in the new
>>> world order, each process does have all the sockets, although it will accept
>>> connections only for those for which it is configured to do so (I plan to 
>>> add
>>> a configuration option to restore the old behavior, for those who don't 
>>> need 
>>> that, and want to save file descriptors).
>>> Reading the haproxy-systemd-wrapper code, it should be trivial.
>>> I just need to figure out how to properly provide the socket path to the
>>>  wrapper.
>>> I see that you already made use of a few environment variables in
>>> haproxy.service. Would that be reasonnable to add a new one, that could
>>> be set in haproxy.service.d/overwrite.conf ? I'm not super-used to systemd,
>>> and I'm not sure of how people are doing that kind of things.
>>>
>>
>> I believe you are referring to $CONFIG and PIDFILE environment variables. 
>> Those
>> two variables are passed to the two arguments, which were present but 
>> impossible
>> to adjust their input, switching to variables allowed people to overwrite 
>> their input.
>>
>> In this case, we are talking about a new functionality I guess the best 
>> approach
>> would be to have ExecStart using EXTRAOPTS:
>> ExecStart=/usr/sbin/haproxy-systemd-wrapper -f $CONFIG -p $PIDFILE $EXTRAOPTS
>>
>> This will allow users to set a value to the new argument and any other 
>> argument
>> they want
>> cat /etc/systemd/system/haproxy.service.d/overwrite.conf
>> [Service]
>> Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
>> "EXTRAOPTS=-x /foobar"
>>
>> or using default configuration file /etc/default/haproxy
>>
>> [Service]
>> Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
>> EnvironmentFile=-/etc/default/haproxy
>> ExecStart=/usr/sbin/haproxy-systemd-wrapper -f $CONFIG -p $PIDFILE $EXTRAOPTS
>>
>> cat /etc/default/haproxy
>> EXTRAOPTS="-x /foobar"
>>
>> I hope it helps,
>> Cheers,
>>
> 
> 
> 
> Hi Pavlos,
> 
> Yeah I see what you mean, it is certainly doable, though -x is a bit special,
> because you don't use it the first time you run haproxy, only for reloading,
> so that would mean the wrapper has special knowledge about it, and remove it
> from the user-supplied command line the first time it's called. I'm a bit
> uneasy about that, but if it's felt the best way to do it, I'll go ahead.
> 

I see, I forgot that the -x argument has only a meaning for the reload phase and
ExecReload uses haproxy and not the haproxy-systemd-wrapper.
So, it makes sense to pass the environment variable and let
haproxy-systemd-wrapper do its thing only on the reload.

Please ignore what I wrote above for EXTRAOPTS, sorry for the confusion.

Cheers,
Pavlos



signature.asc
Description: OpenPGP digital signature

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

On Mon, Apr 10, 2017 at 11:08:56PM +0200, Pavlos Parissis wrote:
> On 10/04/2017 08:09 , Olivier Houchard wrote:
> > 
> > Hi,
> > 
> > On top of those patches, here a 3 more patches.
> > The first one makes the systemd wrapper check for a HAPROXY_STATS_SOCKET
> > environment variable. If set, it will use that as an argument to -x, when
> > reloading the process.
> 
> I see you want to introduce a specific environment variable for this 
> functionality
> and then fetched in the code with getenv(). This is a way to do it.
> 
> IMHO: I prefer to pass a value to an argument, for instance -x. It is also
> consistent with haproxy binary where someone uses -x argument as well.
> 

I'm not sure what you mean by this ?
It is supposed to be directly the value given to -x as an argument.

> > The second one sends listening unix sockets, as well as IPv4/v6 sockets. 
> > I see no reason not to, and that means we no longer have to wait until
> > the old process close the socket before being able to accept new connections
> > on it.
> 
> > The third one adds a new global optoin, nosockettransfer, if set, we assume
> > we will never try to transfer listening sockets through the stats socket,
> > and close any socket nout bound to our process, to save a few file
> > descriptors.
> > 
> 
> IMHO: a better name would be 'stats nounsedsockets', as it is referring to a
> generic functionality of UNIX stats socket, rather to a very specific 
> functionality.
> 

Well really it is a global setting, maybe my wording isn't great, but it
makes haproxy close all sockets not bound to this process, as it used to,
instead of keeping them around in case somebody asks for them. 

> I hope tomorrow I will find some time to test your patches.
> 

Thanks a lot ! This is greatly appreciated.

Regards,

Olivier

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

On Mon, Apr 10, 2017 at 10:49:21PM +0200, Pavlos Parissis wrote:
> On 07/04/2017 11:17 , Olivier Houchard wrote:
> > On Fri, Apr 07, 2017 at 09:58:57PM +0200, Pavlos Parissis wrote:
> >> On 06/04/2017 04:57 , Olivier Houchard wrote:
> >>> On Thu, Apr 06, 2017 at 04:56:47PM +0200, Pavlos Parissis wrote:
>  On 06/04/2017 04:25 , Olivier Houchard wrote:
> > Hi,
> >
> > The attached patchset is the first cut at an attempt to work around the
> > linux issues with SOREUSEPORT that makes haproxy refuse a few new 
> > connections
> > under heavy load.
> > This works by transferring the existing sockets to the new process via 
> > the
> > stats socket. A new command-line flag has been added, -x, that takes the
> > path to the unix socket as an argument, and if set, will attempt to 
> > retrieve
> > all the listening sockets;
> > Right now, any error, either while connecting to the socket, or 
> > retrieving
> > the file descriptors, is fatal, but maybe it'd be better to just fall 
> > back
> > to the previous behavior instead of opening any missing socket ? I'm 
> > still
> > undecided about that.
> >
> > Any testing, comments, etc would be greatly appreciated.
> >
> 
>  Does this patch set support HAProxy in multiprocess mode (nbproc > 1) ?
> 
> >>>
> >>> Hi Pavlos,
> >>>
> >>> If it does not, it's a bug :)
> >>> In my few tests, it seemed to work.
> >>>
> >>> Olivier
> >>>
> >>
> >>
> >> I run systems with systemd and I can't see how I can test the seamless 
> >> reload
> >> functionality ( thanks for that) without a proper support for the UNIX 
> >> socket
> >> file argument (-x) in the haproxy-systemd-wrapper.
> >>
> >> I believe you need to modify the wrapper to accept -x argument for a single
> >> UNIX socket file or -X for a directory path with multiple UNIX socket 
> >> files,
> >> when HAProxy runs in multi-process mode.
> >>
> >> What do you think?
> >>
> >> Cheers,
> >> Pavlos
> >>
> >>
> >>
> > 
> > 
> > Hi Pavlos,
> > 
> > I didn't consider systemd, so it may be I have to investigate there.
> > You don't need to talk to all the processes to get the sockets, in the new
> > world order, each process does have all the sockets, although it will accept
> > connections only for those for which it is configured to do so (I plan to 
> > add
> > a configuration option to restore the old behavior, for those who don't 
> > need 
> > that, and want to save file descriptors).
> > Reading the haproxy-systemd-wrapper code, it should be trivial.
> > I just need to figure out how to properly provide the socket path to the
> >  wrapper.
> > I see that you already made use of a few environment variables in
> > haproxy.service. Would that be reasonnable to add a new one, that could
> > be set in haproxy.service.d/overwrite.conf ? I'm not super-used to systemd,
> > and I'm not sure of how people are doing that kind of things.
> > 
> 
> I believe you are referring to $CONFIG and PIDFILE environment variables. 
> Those
> two variables are passed to the two arguments, which were present but 
> impossible
> to adjust their input, switching to variables allowed people to overwrite 
> their input.
> 
> In this case, we are talking about a new functionality I guess the best 
> approach
> would be to have ExecStart using EXTRAOPTS:
> ExecStart=/usr/sbin/haproxy-systemd-wrapper -f $CONFIG -p $PIDFILE $EXTRAOPTS
> 
> This will allow users to set a value to the new argument and any other 
> argument
> they want
> cat /etc/systemd/system/haproxy.service.d/overwrite.conf
> [Service]
> Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
> "EXTRAOPTS=-x /foobar"
> 
> or using default configuration file /etc/default/haproxy
> 
> [Service]
> Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
> EnvironmentFile=-/etc/default/haproxy
> ExecStart=/usr/sbin/haproxy-systemd-wrapper -f $CONFIG -p $PIDFILE $EXTRAOPTS
> 
> cat /etc/default/haproxy
> EXTRAOPTS="-x /foobar"
> 
> I hope it helps,
> Cheers,
> 



Hi Pavlos,

Yeah I see what you mean, it is certainly doable, though -x is a bit special,
because you don't use it the first time you run haproxy, only for reloading,
so that would mean the wrapper has special knowledge about it, and remove it
from the user-supplied command line the first time it's called. I'm a bit
uneasy about that, but if it's felt the best way to do it, I'll go ahead.

Regards,

Olivier

Re: [RFC][PATCHES] seamless reload

[Pavlos Parissis]

On 10/04/2017 08:09 μμ, Olivier Houchard wrote:
> 
> Hi,
> 
> On top of those patches, here a 3 more patches.
> The first one makes the systemd wrapper check for a HAPROXY_STATS_SOCKET
> environment variable. If set, it will use that as an argument to -x, when
> reloading the process.

I see you want to introduce a specific environment variable for this 
functionality
and then fetched in the code with getenv(). This is a way to do it.

IMHO: I prefer to pass a value to an argument, for instance -x. It is also
consistent with haproxy binary where someone uses -x argument as well.

> The second one sends listening unix sockets, as well as IPv4/v6 sockets. 
> I see no reason not to, and that means we no longer have to wait until
> the old process close the socket before being able to accept new connections
> on it.

> The third one adds a new global optoin, nosockettransfer, if set, we assume
> we will never try to transfer listening sockets through the stats socket,
> and close any socket nout bound to our process, to save a few file
> descriptors.
> 

IMHO: a better name would be 'stats nounsedsockets', as it is referring to a
generic functionality of UNIX stats socket, rather to a very specific 
functionality.

I hope tomorrow I will find some time to test your patches.

Thanks a lot for your work on this,
Pavlos



signature.asc
Description: OpenPGP digital signature

Re: [RFC][PATCHES] seamless reload

[Pavlos Parissis]

On 07/04/2017 11:17 μμ, Olivier Houchard wrote:
> On Fri, Apr 07, 2017 at 09:58:57PM +0200, Pavlos Parissis wrote:
>> On 06/04/2017 04:57 , Olivier Houchard wrote:
>>> On Thu, Apr 06, 2017 at 04:56:47PM +0200, Pavlos Parissis wrote:
 On 06/04/2017 04:25 , Olivier Houchard wrote:
> Hi,
>
> The attached patchset is the first cut at an attempt to work around the
> linux issues with SOREUSEPORT that makes haproxy refuse a few new 
> connections
> under heavy load.
> This works by transferring the existing sockets to the new process via the
> stats socket. A new command-line flag has been added, -x, that takes the
> path to the unix socket as an argument, and if set, will attempt to 
> retrieve
> all the listening sockets;
> Right now, any error, either while connecting to the socket, or retrieving
> the file descriptors, is fatal, but maybe it'd be better to just fall back
> to the previous behavior instead of opening any missing socket ? I'm still
> undecided about that.
>
> Any testing, comments, etc would be greatly appreciated.
>

 Does this patch set support HAProxy in multiprocess mode (nbproc > 1) ?

>>>
>>> Hi Pavlos,
>>>
>>> If it does not, it's a bug :)
>>> In my few tests, it seemed to work.
>>>
>>> Olivier
>>>
>>
>>
>> I run systems with systemd and I can't see how I can test the seamless reload
>> functionality ( thanks for that) without a proper support for the UNIX socket
>> file argument (-x) in the haproxy-systemd-wrapper.
>>
>> I believe you need to modify the wrapper to accept -x argument for a single
>> UNIX socket file or -X for a directory path with multiple UNIX socket files,
>> when HAProxy runs in multi-process mode.
>>
>> What do you think?
>>
>> Cheers,
>> Pavlos
>>
>>
>>
> 
> 
> Hi Pavlos,
> 
> I didn't consider systemd, so it may be I have to investigate there.
> You don't need to talk to all the processes to get the sockets, in the new
> world order, each process does have all the sockets, although it will accept
> connections only for those for which it is configured to do so (I plan to add
> a configuration option to restore the old behavior, for those who don't need 
> that, and want to save file descriptors).
> Reading the haproxy-systemd-wrapper code, it should be trivial.
> I just need to figure out how to properly provide the socket path to the
>  wrapper.
> I see that you already made use of a few environment variables in
> haproxy.service. Would that be reasonnable to add a new one, that could
> be set in haproxy.service.d/overwrite.conf ? I'm not super-used to systemd,
> and I'm not sure of how people are doing that kind of things.
> 

I believe you are referring to $CONFIG and PIDFILE environment variables. Those
two variables are passed to the two arguments, which were present but impossible
to adjust their input, switching to variables allowed people to overwrite their 
input.

In this case, we are talking about a new functionality I guess the best approach
would be to have ExecStart using EXTRAOPTS:
ExecStart=/usr/sbin/haproxy-systemd-wrapper -f $CONFIG -p $PIDFILE $EXTRAOPTS

This will allow users to set a value to the new argument and any other argument
they want
cat /etc/systemd/system/haproxy.service.d/overwrite.conf
[Service]
Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
"EXTRAOPTS=-x /foobar"

or using default configuration file /etc/default/haproxy

[Service]
Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
EnvironmentFile=-/etc/default/haproxy
ExecStart=/usr/sbin/haproxy-systemd-wrapper -f $CONFIG -p $PIDFILE $EXTRAOPTS

cat /etc/default/haproxy
EXTRAOPTS="-x /foobar"

I hope it helps,
Cheers,



signature.asc
Description: OpenPGP digital signature

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

Hi,

On top of those patches, here a 3 more patches.
The first one makes the systemd wrapper check for a HAPROXY_STATS_SOCKET
environment variable. If set, it will use that as an argument to -x, when
reloading the process.
The second one sends listening unix sockets, as well as IPv4/v6 sockets. 
I see no reason not to, and that means we no longer have to wait until
the old process close the socket before being able to accept new connections
on it.
The third one adds a new global optoin, nosockettransfer, if set, we assume
we will never try to transfer listening sockets through the stats socket,
and close any socket nout bound to our process, to save a few file
descriptors.

Regards,

Olivier
>From 8d6c38b6824346b096ba31757ab62bc986a433b3 Mon Sep 17 00:00:00 2001
From: Olivier Houchard 
Date: Sun, 9 Apr 2017 16:28:10 +0200
Subject: [PATCH 7/9] MINOR: systemd wrapper: add support for passing the -x
 option.

Make the systemd wrapper chech if HAPROXY_STATS_SOCKET if set.
If set, it will use it as an argument to the "-x" option, which makes
haproxy asks for any listening socket, on the stats socket, in order
to achieve reloads with no new connection lost.
---
 contrib/systemd/haproxy.service.in |  2 ++
 src/haproxy-systemd-wrapper.c  | 10 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/contrib/systemd/haproxy.service.in 
b/contrib/systemd/haproxy.service.in
index dca81a2..05bb716 100644
--- a/contrib/systemd/haproxy.service.in
+++ b/contrib/systemd/haproxy.service.in
@@ -3,6 +3,8 @@ Description=HAProxy Load Balancer
 After=network.target
 
 [Service]
+# You can point the environment variable HAPROXY_STATS_SOCKET to a stats
+# socket if you want seamless reloads.
 Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
 ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q
 ExecStart=@SBINDIR@/haproxy-systemd-wrapper -f $CONFIG -p $PIDFILE
diff --git a/src/haproxy-systemd-wrapper.c b/src/haproxy-systemd-wrapper.c
index f6a9c85..1d00111 100644
--- a/src/haproxy-systemd-wrapper.c
+++ b/src/haproxy-systemd-wrapper.c
@@ -92,11 +92,15 @@ static void spawn_haproxy(char **pid_strv, int nb_pid)
pid = fork();
if (!pid) {
char **argv;
+   char *stats_socket = NULL;
int i;
int argno = 0;
 
/* 3 for "haproxy -Ds -sf" */
-   argv = calloc(4 + main_argc + nb_pid + 1, sizeof(char *));
+   if (nb_pid > 0)
+   stats_socket = getenv("HAPROXY_STATS_SOCKET");
+   argv = calloc(4 + main_argc + nb_pid + 1 +
+   stats_socket != NULL ? 2 : 0, sizeof(char *));
if (!argv) {
fprintf(stderr, SD_NOTICE "haproxy-systemd-wrapper: 
failed to calloc(), please try again later.\n");
exit(1);
@@ -121,6 +125,10 @@ static void spawn_haproxy(char **pid_strv, int nb_pid)
argv[argno++] = "-sf";
for (i = 0; i < nb_pid; ++i)
argv[argno++] = pid_strv[i];
+   if (stats_socket != NULL) {
+   argv[argno++] = "-x";
+   argv[argno++] = stats_socket;
+   }
}
argv[argno] = NULL;
 
-- 
2.9.3

>From df5e6e70f2e73fca9e28ba273904ab5c5acf53d3 Mon Sep 17 00:00:00 2001
From: Olivier Houchard 
Date: Sun, 9 Apr 2017 19:17:15 +0200
Subject: [PATCH 8/9] MINOR: cli: When sending listening sockets, send unix
 sockets too.

Send unix sockets, as well as IPv4/IPv6 sockets, so that we don't have to
wait for the old process to die before being able to bind those.
---
 src/cli.c|  6 --
 src/proto_uxst.c | 50 ++
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/src/cli.c b/src/cli.c
index d5ff11f..533f792 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -1067,7 +1067,8 @@ static int _getsocks(char **args, struct appctx *appctx, 
void *private)
list_for_each_entry(l, >conf.listeners, by_fe) {
/* Only transfer IPv4/IPv6 sockets */
if (l->proto->sock_family == AF_INET ||
-   l->proto->sock_family == AF_INET6)
+   l->proto->sock_family == AF_INET6 ||
+   l->proto->sock_family == AF_UNIX)
tot_fd_nb++;
}
px = px->next;
@@ -1120,7 +1121,8 @@ static int _getsocks(char **args, struct appctx *appctx, 
void *private)
/* Only transfer IPv4/IPv6 sockets */
if (l->state >= LI_LISTEN &&
(l->proto->sock_family == AF_INET ||
-   l->proto->sock_family == AF_INET6)) {
+   l->proto->sock_family == 

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

On Fri, Apr 07, 2017 at 09:58:57PM +0200, Pavlos Parissis wrote:
> On 06/04/2017 04:57 , Olivier Houchard wrote:
> > On Thu, Apr 06, 2017 at 04:56:47PM +0200, Pavlos Parissis wrote:
> >> On 06/04/2017 04:25 , Olivier Houchard wrote:
> >>> Hi,
> >>>
> >>> The attached patchset is the first cut at an attempt to work around the
> >>> linux issues with SOREUSEPORT that makes haproxy refuse a few new 
> >>> connections
> >>> under heavy load.
> >>> This works by transferring the existing sockets to the new process via the
> >>> stats socket. A new command-line flag has been added, -x, that takes the
> >>> path to the unix socket as an argument, and if set, will attempt to 
> >>> retrieve
> >>> all the listening sockets;
> >>> Right now, any error, either while connecting to the socket, or retrieving
> >>> the file descriptors, is fatal, but maybe it'd be better to just fall back
> >>> to the previous behavior instead of opening any missing socket ? I'm still
> >>> undecided about that.
> >>>
> >>> Any testing, comments, etc would be greatly appreciated.
> >>>
> >>
> >> Does this patch set support HAProxy in multiprocess mode (nbproc > 1) ?
> >>
> > 
> > Hi Pavlos,
> > 
> > If it does not, it's a bug :)
> > In my few tests, it seemed to work.
> > 
> > Olivier
> > 
> 
> 
> I run systems with systemd and I can't see how I can test the seamless reload
> functionality ( thanks for that) without a proper support for the UNIX socket
> file argument (-x) in the haproxy-systemd-wrapper.
> 
> I believe you need to modify the wrapper to accept -x argument for a single
> UNIX socket file or -X for a directory path with multiple UNIX socket files,
> when HAProxy runs in multi-process mode.
> 
> What do you think?
> 
> Cheers,
> Pavlos
> 
> 
> 


Hi Pavlos,

I didn't consider systemd, so it may be I have to investigate there.
You don't need to talk to all the processes to get the sockets, in the new
world order, each process does have all the sockets, although it will accept
connections only for those for which it is configured to do so (I plan to add
a configuration option to restore the old behavior, for those who don't need 
that, and want to save file descriptors).
Reading the haproxy-systemd-wrapper code, it should be trivial.
I just need to figure out how to properly provide the socket path to the
 wrapper.
I see that you already made use of a few environment variables in
haproxy.service. Would that be reasonnable to add a new one, that could
be set in haproxy.service.d/overwrite.conf ? I'm not super-used to systemd,
and I'm not sure of how people are doing that kind of things.

Thanks !

Olivier

Re: [RFC][PATCHES] seamless reload

[Pavlos Parissis]

On 06/04/2017 04:57 μμ, Olivier Houchard wrote:
> On Thu, Apr 06, 2017 at 04:56:47PM +0200, Pavlos Parissis wrote:
>> On 06/04/2017 04:25 μμ, Olivier Houchard wrote:
>>> Hi,
>>>
>>> The attached patchset is the first cut at an attempt to work around the
>>> linux issues with SOREUSEPORT that makes haproxy refuse a few new 
>>> connections
>>> under heavy load.
>>> This works by transferring the existing sockets to the new process via the
>>> stats socket. A new command-line flag has been added, -x, that takes the
>>> path to the unix socket as an argument, and if set, will attempt to retrieve
>>> all the listening sockets;
>>> Right now, any error, either while connecting to the socket, or retrieving
>>> the file descriptors, is fatal, but maybe it'd be better to just fall back
>>> to the previous behavior instead of opening any missing socket ? I'm still
>>> undecided about that.
>>>
>>> Any testing, comments, etc would be greatly appreciated.
>>>
>>
>> Does this patch set support HAProxy in multiprocess mode (nbproc > 1) ?
>>
> 
> Hi Pavlos,
> 
> If it does not, it's a bug :)
> In my few tests, it seemed to work.
> 
> Olivier
> 


I run systems with systemd and I can't see how I can test the seamless reload
functionality ( thanks for that) without a proper support for the UNIX socket
file argument (-x) in the haproxy-systemd-wrapper.

I believe you need to modify the wrapper to accept -x argument for a single
UNIX socket file or -X for a directory path with multiple UNIX socket files,
when HAProxy runs in multi-process mode.

What do you think?

Cheers,
Pavlos





signature.asc
Description: OpenPGP digital signature

Re: [RFC][PATCHES] seamless reload

[Olivier Houchard]

On Thu, Apr 06, 2017 at 04:56:47PM +0200, Pavlos Parissis wrote:
> On 06/04/2017 04:25 μμ, Olivier Houchard wrote:
> > Hi,
> > 
> > The attached patchset is the first cut at an attempt to work around the
> > linux issues with SOREUSEPORT that makes haproxy refuse a few new 
> > connections
> > under heavy load.
> > This works by transferring the existing sockets to the new process via the
> > stats socket. A new command-line flag has been added, -x, that takes the
> > path to the unix socket as an argument, and if set, will attempt to retrieve
> > all the listening sockets;
> > Right now, any error, either while connecting to the socket, or retrieving
> > the file descriptors, is fatal, but maybe it'd be better to just fall back
> > to the previous behavior instead of opening any missing socket ? I'm still
> > undecided about that.
> > 
> > Any testing, comments, etc would be greatly appreciated.
> > 
> 
> Does this patch set support HAProxy in multiprocess mode (nbproc > 1) ?
> 

Hi Pavlos,

If it does not, it's a bug :)
In my few tests, it seemed to work.

Olivier

Re: [RFC][PATCHES] seamless reload

[Pavlos Parissis]

On 06/04/2017 04:25 μμ, Olivier Houchard wrote:
> Hi,
> 
> The attached patchset is the first cut at an attempt to work around the
> linux issues with SOREUSEPORT that makes haproxy refuse a few new connections
> under heavy load.
> This works by transferring the existing sockets to the new process via the
> stats socket. A new command-line flag has been added, -x, that takes the
> path to the unix socket as an argument, and if set, will attempt to retrieve
> all the listening sockets;
> Right now, any error, either while connecting to the socket, or retrieving
> the file descriptors, is fatal, but maybe it'd be better to just fall back
> to the previous behavior instead of opening any missing socket ? I'm still
> undecided about that.
> 
> Any testing, comments, etc would be greatly appreciated.
> 

Does this patch set support HAProxy in multiprocess mode (nbproc > 1) ?

Cheers,
Pavlos



signature.asc
Description: OpenPGP digital signature