Re: [PATCH v3 1/1] MEDIUM: mworker: Add systemd `Type=notify` support

[Lukas Tribus]

Hello,


2017-11-21 11:18 GMT+01:00 Willy Tarreau :
>> That's not it, the hold-off timer is only a consequence of this
>> problem.
>
> OK but if it's really 100ms, it can be a problem for people loading GeoIP
> maps of millions of entries, or large configs (the largest I saw had 30
> backends and 60 servers and took several seconds to process).

The default timeout to start a daemon is 90 seconds if I understand correctly:
https://www.freedesktop.org/software/systemd/man/systemd-system.conf.html#DefaultTimeoutStartSec=

The 100ms "timeout" limits the restart interval itself, so those
defaults should be sane.



Regards,
Lukas

Re: [PATCH v3 1/1] MEDIUM: mworker: Add systemd `Type=notify` support

[William Lallemand]

On Tue, Nov 21, 2017 at 11:28:55AM +0100, Willy Tarreau wrote:
> On Tue, Nov 21, 2017 at 11:19:41AM +0100, William Lallemand wrote:
> > I don't like the idea of the "daemon" keyword being ignored,
> 
> We already do that with -D which ignores "debug" for example. The command
> line purposely has precedence over the config.
> 
> > however, we could
> > exit with an error when we try to start with -Ws + -D or daemon.
> 
> Errors only cause gray hair to users, especially when they concern stuff
> that can be automatically recovered from. We could definitely consider
> that -Ws automatically implies MODE_FOREGROUND which will do exactly
> what -db does, but without having to think about it. After all, "-Ws"
> is "master-worker mode with systemd awareness" so it makes sense not
> to background in this case.
> 

That's right, I agree.

-- 
William Lallemand

Re: [PATCH v3 1/1] MEDIUM: mworker: Add systemd `Type=notify` support

[William Lallemand]

On Tue, Nov 21, 2017 at 11:12:45AM +0100, Lukas Tribus wrote:
> Hello,
> 
> 2017-11-21 8:39 GMT+01:00 William Lallemand :
> 
> That's not it, the hold-off timer is only a consequence of this
> problem. I believe the notification does not work in my case, which is
> why for systemd haproxy did not start correctly which is why systemd
> continues to restart haproxy.
> I found the root of the issue: the "daemon" keyword in the global
> configuration section. We need to remove "daemon" from the
> configuration for systemd to mode work correctly in notify mode.
> 
> We can override the "daemon" keyword in the configuration by passing
> -db to haproxy. Adding it to the unit file fixes the issue even if
> they keyword is in the configuration (so we pass "-Ws -db" ).
> 
> "-Ws -db" along with a note in the documentation that "daemon" may be
> ignored in systemd mode seems like a good compromise here? I will send
> an RFC patch shortly.
> 

You should never use systemd with the daemon mode, either in notify or default
mode, it's not the recommended setup.

I don't like the idea of the "daemon" keyword being ignored, however, we could
exit with an error when we try to start with -Ws + -D or daemon.

-- 
William Lallemand

Re: [PATCH v3 1/1] MEDIUM: mworker: Add systemd `Type=notify` support

[Willy Tarreau]

Hi Lukas,

On Tue, Nov 21, 2017 at 11:12:45AM +0100, Lukas Tribus wrote:
> > I suggest we configure it to a greater value per default, it can be set to
> > infinity too, but I don't like the idea.
> 
> That's not it, the hold-off timer is only a consequence of this
> problem.

OK but if it's really 100ms, it can be a problem for people loading GeoIP
maps of millions of entries, or large configs (the largest I saw had 30
backends and 60 servers and took several seconds to process).

> I believe the notification does not work in my case, which is
> why for systemd haproxy did not start correctly which is why systemd
> continues to restart haproxy.

I'm feeling less and less confident in using this mode by default :-/

> I found the root of the issue: the "daemon" keyword in the global
> configuration section. We need to remove "daemon" from the
> configuration for systemd to mode work correctly in notify mode.
> 
> We can override the "daemon" keyword in the configuration by passing
> -db to haproxy. Adding it to the unit file fixes the issue even if
> they keyword is in the configuration (so we pass "-Ws -db" ).
> 
> "-Ws -db" along with a note in the documentation that "daemon" may be
> ignored in systemd mode seems like a good compromise here?

Yes and it definitely is needed, just like we used to suggest users to
always have -D in init scripts to force backgrounding of the process
when there was no "deamon" statement in the conf.

> I will send an RFC patch shortly.

Thank you!
Willy

Re: [PATCH v3 1/1] MEDIUM: mworker: Add systemd `Type=notify` support

[Lukas Tribus]

Hello,



2017-11-21 8:39 GMT+01:00 William Lallemand :
> On Tue, Nov 21, 2017 at 07:16:19AM +0100, Willy Tarreau wrote:
>>
>> I really don't like this. My fears with becoming more systemd-friendly
>> was that we'd make users helpless when something decides not to work
>> just to annoy them, and this reports seems to confirm this feeling :-/
>>
>> Tim, have you seen this message about a "hold-off timer over" being
>> displayed at the same second as the startup message ? Isn't there a
>> timeout setting or something like this to place in the config file ?
>> And is there a way to disable it so that people with huge configs
>> are still allowed to load them ?
>>
>
> There is indeed a timeout value, which is configurable in the unit file, the
> default value is 100ms.
>
> https://www.freedesktop.org/software/systemd/man/systemd.service.html#RestartSec=
>
>
> There is also the start one there:
>
> https://www.freedesktop.org/software/systemd/man/systemd.service.html#TimeoutStartSec=
>
> I suggest we configure it to a greater value per default, it can be set to 
> infinity too, but I don't like the idea.

That's not it, the hold-off timer is only a consequence of this
problem. I believe the notification does not work in my case, which is
why for systemd haproxy did not start correctly which is why systemd
continues to restart haproxy.
I found the root of the issue: the "daemon" keyword in the global
configuration section. We need to remove "daemon" from the
configuration for systemd to mode work correctly in notify mode.

We can override the "daemon" keyword in the configuration by passing
-db to haproxy. Adding it to the unit file fixes the issue even if
they keyword is in the configuration (so we pass "-Ws -db" ).

"-Ws -db" along with a note in the documentation that "daemon" may be
ignored in systemd mode seems like a good compromise here? I will send
an RFC patch shortly.



Regards,
Lukas

Re: [PATCH v3 1/1] MEDIUM: mworker: Add systemd `Type=notify` support

[William Lallemand]

On Tue, Nov 21, 2017 at 07:16:19AM +0100, Willy Tarreau wrote:
> 
> I really don't like this. My fears with becoming more systemd-friendly
> was that we'd make users helpless when something decides not to work
> just to annoy them, and this reports seems to confirm this feeling :-/
> 
> Tim, have you seen this message about a "hold-off timer over" being
> displayed at the same second as the startup message ? Isn't there a
> timeout setting or something like this to place in the config file ?
> And is there a way to disable it so that people with huge configs
> are still allowed to load them ?
> 

There is indeed a timeout value, which is configurable in the unit file, the
default value is 100ms.

https://www.freedesktop.org/software/systemd/man/systemd.service.html#RestartSec=
 

There is also the start one there:

https://www.freedesktop.org/software/systemd/man/systemd.service.html#TimeoutStartSec=

I suggest we configure it to a greater value per default, it can be set to 
infinity too, but I don't like the idea.

-- 
William Lallemand

Re: [PATCH v3 1/1] MEDIUM: mworker: Add systemd `Type=notify` support

[Willy Tarreau]

Hi Lukas,

On Tue, Nov 21, 2017 at 01:12:10AM +0100, Lukas Tribus wrote:
> 2017-11-20 15:58 GMT+01:00 Tim Düsterhus :
> > From: Tim Duesterhus 
> >
> > This patch adds support for `Type=notify` to the systemd unit.
> >
> > Supporting `Type=notify` improves both starting as well as reloading
> > of the unit, because systemd will be let known when the action completed.
> 
> I can't start haproxy anymore this way.
> 
> I compiled with USE_SYSTEMD, updated the unit file with both changes,
> and reloaded systemd (systemctl daemon-reload), but systemd aborts the
> start for some reason:
> 
> 
> Nov 21 00:46:20 www systemd[1]: Starting HAProxy Load Balancer...
> Nov 21 00:46:20 www haproxy[814]: [WARNING] 324/004620 (814) : parsing
> [/etc/haproxy/haproxy.cfg:118] : a 'http-request' rule placed after a
> 'reqxxx' rule will still be processed before.
> [...]
> Nov 21 00:46:20 www haproxy[814]: Proxy port443 started.
> Nov 21 00:46:20 www haproxy[814]: Proxy port443 started.
> [...]
> Nov 21 00:46:20 www systemd[1]: Started HAProxy Load Balancer.
> Nov 21 00:46:20 www systemd[1]: haproxy.service: Service hold-off time
> over, scheduling restart.
> Nov 21 00:46:20 www systemd[1]: Stopped HAProxy Load Balancer.
> Nov 21 00:46:20 www systemd[1]: haproxy.service: Start request
> repeated too quickly.
> Nov 21 00:46:20 www systemd[1]: Failed to start HAProxy Load Balancer.
> 
> 
> Reverting back to "Type=forking" (while still using -Ws and
> USE_SYSTEMD=1) works for me. This is ubuntu xenial (16.04) with
> systemd 229. No multiprocess mode, no threading mode used. My
> configuration does produce its fair share of config warnings though,
> but that should not matter. Any troubleshooting advice for me here?

I really don't like this. My fears with becoming more systemd-friendly
was that we'd make users helpless when something decides not to work
just to annoy them, and this reports seems to confirm this feeling :-/

Tim, have you seen this message about a "hold-off timer over" being
displayed at the same second as the startup message ? Isn't there a
timeout setting or something like this to place in the config file ?
And is there a way to disable it so that people with huge configs
are still allowed to load them ?

Regards,
willy

Re: [PATCH v3 1/1] MEDIUM: mworker: Add systemd `Type=notify` support

[Lukas Tribus]

Hello Tim,



2017-11-20 15:58 GMT+01:00 Tim Düsterhus :
> From: Tim Duesterhus 
>
> This patch adds support for `Type=notify` to the systemd unit.
>
> Supporting `Type=notify` improves both starting as well as reloading
> of the unit, because systemd will be let known when the action completed.

I can't start haproxy anymore this way.

I compiled with USE_SYSTEMD, updated the unit file with both changes,
and reloaded systemd (systemctl daemon-reload), but systemd aborts the
start for some reason:


Nov 21 00:46:20 www systemd[1]: Starting HAProxy Load Balancer...
Nov 21 00:46:20 www haproxy[814]: [WARNING] 324/004620 (814) : parsing
[/etc/haproxy/haproxy.cfg:118] : a 'http-request' rule placed after a
'reqxxx' rule will still be processed before.
[...]
Nov 21 00:46:20 www haproxy[814]: Proxy port443 started.
Nov 21 00:46:20 www haproxy[814]: Proxy port443 started.
[...]
Nov 21 00:46:20 www systemd[1]: Started HAProxy Load Balancer.
Nov 21 00:46:20 www systemd[1]: haproxy.service: Service hold-off time
over, scheduling restart.
Nov 21 00:46:20 www systemd[1]: Stopped HAProxy Load Balancer.
Nov 21 00:46:20 www systemd[1]: haproxy.service: Start request
repeated too quickly.
Nov 21 00:46:20 www systemd[1]: Failed to start HAProxy Load Balancer.


Reverting back to "Type=forking" (while still using -Ws and
USE_SYSTEMD=1) works for me. This is ubuntu xenial (16.04) with
systemd 229. No multiprocess mode, no threading mode used. My
configuration does produce its fair share of config warnings though,
but that should not matter. Any troubleshooting advice for me here?



Thanks,
Lukas

Re: [PATCH v3 1/1] MEDIUM: mworker: Add systemd `Type=notify` support

[William Lallemand]

On Mon, Nov 20, 2017 at 03:58:35PM +0100, Tim Düsterhus wrote:
> From: Tim Duesterhus 
> 
> This patch adds support for `Type=notify` to the systemd unit.
 
Looks good to me, thanks!

Willy can you merge it?

-- 
William Lallemand