Discussion:
[PATCH] BUG/MEDIUM: systemd: set KillMode to 'mixed'
Apollon Oikonomopoulos
2014-10-08 12:14:41 UTC
Permalink
By default systemd will send SIGTERM to all processes in the service's
control group. In our case, this includes the wrapper, the master
process and all worker processes.

Since commit c54bdd2a the wrapper actually catches SIGTERM and survives
to see the master process getting killed by systemd and regard this as
an error, placing the unit in a failed state during "systemctl stop".

Since the wrapper now handles SIGTERM by itself, we switch the kill mode
to 'mixed', which means that systemd will deliver the initial SIGTERM to
the wrapper only, and if the actual haproxy processes don't exit after a
given amount of time (default: 90s), a SIGKILL is sent to all remaining
processes in the control group. See systemd.kill(5) for more
information.

This should also be backported to 1.5.
---
contrib/systemd/haproxy.service.in | 1 +
1 file changed, 1 insertion(+)

diff --git a/contrib/systemd/haproxy.service.in b/contrib/systemd/haproxy.service.in
index 1a3d2c0..0bc5420 100644
--- a/contrib/systemd/haproxy.service.in
+++ b/contrib/systemd/haproxy.service.in
@@ -5,6 +5,7 @@ After=network.target
[Service]
ExecStart=@SBINDIR@/haproxy-systemd-wrapper -f /etc/haproxy/haproxy.cfg -p /run/haproxy.pid
ExecReload=/bin/kill -USR2 $MAINPID
+KillMode=mixed
Restart=always

[Install]
--
2.1.1
Willy Tarreau
2014-10-09 09:26:31 UTC
Permalink
Hi Apollon,
Post by Apollon Oikonomopoulos
By default systemd will send SIGTERM to all processes in the service's
control group. In our case, this includes the wrapper, the master
process and all worker processes.
Since commit c54bdd2a the wrapper actually catches SIGTERM and survives
to see the master process getting killed by systemd and regard this as
an error, placing the unit in a failed state during "systemctl stop".
Then shouldn't we fix this by letting the wrapper die after receiving the
SIGTERM ? Otherwise I'm happy to merge your patch, but I'd rather ensure
that we don't encounter yet another issue.

I'm really amazed by the amount of breakage these new service managers are
causing to a simple process management that has been working well for over
40 years of UNIX existence now, and the difficulty we have to work around
this whole mess!

Thanks!
Willy
Jason J. W. Williams
2014-10-09 09:34:18 UTC
Permalink
Post by Willy Tarreau
I'm really amazed by the amount of breakage these new service managers are
causing to a simple process management that has been working well for over
40 years of UNIX existence now, and the difficulty we have to work around
this whole mess!
If there was a poster child for "knowing better" than the UNIX way and doing violence to it, it would be systemd.
Apollon Oikonomopoulos
2014-10-09 09:35:10 UTC
Permalink
Hi Willy,
Post by Willy Tarreau
Hi Apollon,
Post by Apollon Oikonomopoulos
By default systemd will send SIGTERM to all processes in the service's
control group. In our case, this includes the wrapper, the master
process and all worker processes.
Since commit c54bdd2a the wrapper actually catches SIGTERM and survives
to see the master process getting killed by systemd and regard this as
an error, placing the unit in a failed state during "systemctl stop".
Then shouldn't we fix this by letting the wrapper die after receiving the
SIGTERM ? Otherwise I'm happy to merge your patch, but I'd rather ensure
that we don't encounter yet another issue.
The wrapper does exit on its own when the haproxy "master" process
exits, which is done as soon as all "worker" processes exit. The problem
is that the wrapper wants to control all worker processes on its own,
while systemd second-guesses it by delivering SIGTERM to all processes
by default.
Post by Willy Tarreau
I'm really amazed by the amount of breakage these new service managers are
causing to a simple process management that has been working well for over
40 years of UNIX existence now, and the difficulty we have to work around
this whole mess!
I guess every new system has its difficulties and learning curve,
especially when it breaks implicit assumptions that hold for a long
time.

Cheers,
Apollon
Willy Tarreau
2014-10-09 09:44:46 UTC
Permalink
Post by Apollon Oikonomopoulos
Hi Willy,
Post by Willy Tarreau
Hi Apollon,
Post by Apollon Oikonomopoulos
By default systemd will send SIGTERM to all processes in the service's
control group. In our case, this includes the wrapper, the master
process and all worker processes.
Since commit c54bdd2a the wrapper actually catches SIGTERM and survives
to see the master process getting killed by systemd and regard this as
an error, placing the unit in a failed state during "systemctl stop".
Then shouldn't we fix this by letting the wrapper die after receiving the
SIGTERM ? Otherwise I'm happy to merge your patch, but I'd rather ensure
that we don't encounter yet another issue.
The wrapper does exit on its own when the haproxy "master" process
exits, which is done as soon as all "worker" processes exit. The problem
is that the wrapper wants to control all worker processes on its own,
while systemd second-guesses it by delivering SIGTERM to all processes
by default.
OK, so I'm merging your patch if you think it's the best solution.
Post by Apollon Oikonomopoulos
Post by Willy Tarreau
I'm really amazed by the amount of breakage these new service managers are
causing to a simple process management that has been working well for over
40 years of UNIX existence now, and the difficulty we have to work around
this whole mess!
I guess every new system has its difficulties and learning curve,
especially when it breaks implicit assumptions that hold for a long
time.
Well, we're far away from the learning curve, we're writing a wrapper
to help a stupid service manager handle daemons, because the people who
wrote it did not know that in the unix world, there were some services
running in background. "ps aux" could have educated them by discovering
that there were other processes than "ps" and their shell :-/

Thanks,
Willy
Apollon Oikonomopoulos
2014-10-09 09:55:25 UTC
Permalink
Post by Willy Tarreau
Post by Apollon Oikonomopoulos
Hi Willy,
Post by Willy Tarreau
Hi Apollon,
Post by Apollon Oikonomopoulos
By default systemd will send SIGTERM to all processes in the service's
control group. In our case, this includes the wrapper, the master
process and all worker processes.
Since commit c54bdd2a the wrapper actually catches SIGTERM and survives
to see the master process getting killed by systemd and regard this as
an error, placing the unit in a failed state during "systemctl stop".
Then shouldn't we fix this by letting the wrapper die after receiving the
SIGTERM ? Otherwise I'm happy to merge your patch, but I'd rather ensure
that we don't encounter yet another issue.
The wrapper does exit on its own when the haproxy "master" process
exits, which is done as soon as all "worker" processes exit. The problem
is that the wrapper wants to control all worker processes on its own,
while systemd second-guesses it by delivering SIGTERM to all processes
by default.
OK, so I'm merging your patch if you think it's the best solution.
Well, I think it's the most sane thing to do and is behaviour-compatible
with the current wrapper version.
Post by Willy Tarreau
Post by Apollon Oikonomopoulos
Post by Willy Tarreau
I'm really amazed by the amount of breakage these new service managers are
causing to a simple process management that has been working well for over
40 years of UNIX existence now, and the difficulty we have to work around
this whole mess!
I guess every new system has its difficulties and learning curve,
especially when it breaks implicit assumptions that hold for a long
time.
Well, we're far away from the learning curve, we're writing a wrapper
to help a stupid service manager handle daemons, because the people who
wrote it did not know that in the unix world, there were some services
running in background. "ps aux" could have educated them by discovering
that there were other processes than "ps" and their shell :-/
Truth is, we're writing a wrapper to handle gracefully reloading HAProxy
by completely replacing the master process. Other than that, systemd is
plain happy with just HAProxy running in the foreground using -Ds. I
even have a suspicion that we don't need the wrapper at all to do
graceful reloading. I have to do some experiments on that and I'll come
back to you.

Thanks,
Apollon
Willy Tarreau
2014-10-09 10:07:27 UTC
Permalink
Post by Apollon Oikonomopoulos
Post by Willy Tarreau
OK, so I'm merging your patch if you think it's the best solution.
Well, I think it's the most sane thing to do and is behaviour-compatible
with the current wrapper version.
It's already merged in both 1.5 and 1.6.
Post by Apollon Oikonomopoulos
Post by Willy Tarreau
Post by Apollon Oikonomopoulos
Post by Willy Tarreau
I'm really amazed by the amount of breakage these new service managers are
causing to a simple process management that has been working well for over
40 years of UNIX existence now, and the difficulty we have to work around
this whole mess!
I guess every new system has its difficulties and learning curve,
especially when it breaks implicit assumptions that hold for a long
time.
Well, we're far away from the learning curve, we're writing a wrapper
to help a stupid service manager handle daemons, because the people who
wrote it did not know that in the unix world, there were some services
running in background. "ps aux" could have educated them by discovering
that there were other processes than "ps" and their shell :-/
Truth is, we're writing a wrapper to handle gracefully reloading HAProxy
by completely replacing the master process.
Yes, which seems normal to me. Otherwise how do you upgrade a service
without replacing the master process ? People are performing their seamless
version upgrades everywhere thanks to this.
Post by Apollon Oikonomopoulos
Other than that, systemd is
plain happy with just HAProxy running in the foreground using -Ds. I
even have a suspicion that we don't need the wrapper at all to do
graceful reloading. I have to do some experiments on that and I'll come
back to you.
Quite frankly, I don't see how it makes sense to run a *daemon* in
foreground, except to hide the flaws of the service manager. It also
prevents running in multi-process mode. A daemon runs in background,
with or without sub-processes, and may be replaced at any moment for
various reasons ranging from config changes, upgrades and operations
or mistakes from the admin.

Anyway we're not there to discuss the benefits or defaults of systemd,
some major distros have adopted it and now we have to work around its
breakages so that users can continue to use their systems as if it was
still a regular, manageable UNIX system.

So thanks for your patch :-)
Willy
Apollon Oikonomopoulos
2014-10-09 10:10:02 UTC
Permalink
Post by Willy Tarreau
Anyway we're not there to discuss the benefits or defaults of systemd,
some major distros have adopted it and now we have to work around its
breakages so that users can continue to use their systems as if it was
still a regular, manageable UNIX system.
Trust me, there are lots of things I don't like about systemd either.
But given that it seems to be here to stay for a while, I don't think we
have much choice :)

Regards,
Apollon

Loading...