On Thu, Jan 25, 2018 at 8:52 PM Willy Tarreau <[email protected]> wrote:

> Hi Chris,
>
> On Wed, Jan 24, 2018 at 10:07:03PM +0000, Christopher Lane wrote:
> > Ping; this is a safe change to wanr about a confusing source of error
> when
> > you don't quote the arguments to haproxy in a "soft reloading new config
> a
> > lot" type of environment.
> >
> > On Thu, Dec 7, 2017 at 1:17 PM Chris Lane <[email protected]>
> wrote:
> >
> > >
> > > Previously, -sf and -sd command line parsing used atol which cannot
> > > detect errors.  I had a problem where I was doing -sf "$pid1 $pid2
> $pid"
> > > and it was sending the gracefully terminate signal only to the first
> pid.
> > > The change uses strtol and checks endptr and errno to see if the
> parsing
> > > worked.  It doesn't exit so as not to cause failures but will allow
> > > trouble-shooting
> > > to be faster.
>
> I don't know much what to think about such a change to be honnest. What
> valid use case do you have for passing multiple processes as a single
> argument after -sf instead of passing a normal list of arguments ?
>

We don't have a use case, we were doing it by mistake and haproxy silently
misparsed the argument because it is using atoi which is unable to detect
errors.  The change is to detect and warn about errors.


>
> I suspect you're running a script doing :
>
>      haproxy ... -sf "${pid[*]}"
>
> instead of doing :
>
>      haproxy ... -sf "${pid[@]}"
>
> or :
>
>      haproxy ... -sf ${pid[*]}
>
> Am I wrong ?
> Willy
>

It was some java  Runtime.exec() call but essentially.  The problem was not
that we had to switch to putting each PID into a separate argv, it is that
it took us a while to figure out the problem:  the new process signalled
the first old PID but not the 2nd and so on.  So we had a process leak and
no errors in the logs anywhere.  This change means we'll see a log msg at
start up, which hopefully will help other people with the same bad
invocation of haproxy.

Our setup is we have faily dynamic pool membership and have a java process
listen to ZooKeeper where the ports and hosts are stored.  Each time we get
notified of a change, we write out a new haproxy.conf file and reload with
-sf.  Works very well now.

Hopefully with that clarification this is ok; I could have supported the
multiple PIDs in one argv work as a feature, or make the haproxy fail to
start, but this change will not make an existing haproxy that starts with a
certain behavior change its behavior except the one log msg.

Thanks,


--Chris

Reply via email to