On Fri, Mar 11, 2011 at 06:55:57AM +0100, Willy Tarreau wrote:
> On Fri, Mar 11, 2011 at 08:36:49AM +0900, Simon Horman wrote:
> > As it stands the master needs the entire configuration as it actually
> > starts all the proxies and then forks the workers. This is in order to
> > allow the master to know what sockets to bind and unbind.  But other than
> > that it doesn't care about most of the configuration.
> 
> OK that indeed makes sense.
> 
> > > The idea would be that we could have all the global config and all
> > > proxies in a same struct behind a pointer, and instantiate a new
> > > one when reloading the config, then once everything's OK, we can
> > > switch the configs and fork new processes. The socket cache just
> > > has to be global. And even if we add new listening sockets to this
> > > cache when trying to start a new config, it's not dramatic.
> > 
> > Yes, I think that would work. But I wonder if we need to go that far,
> > as its probably ok for the master to have an invalid configuration so long
> > as we are careful about the (few?) configuration parameters that
> > both affect the master and can be changed on restart.
> 
> OK.
> 
> > > That makes me think that we should most likely keep the master out
> > > of the chroot anyway, since it does not participate into network
> > > communications, it's not exposed.
> > 
> > I don't have any strong opinions on that either way.
> > As it is, having it in the chroot isn't a major burden.
> 
> But if it's in the chroot, it can't reread is configuration file.

True, I have a slightly dubious way to deal with that.
After chrooting the configuration file is opened relative
to the chroot. So you need to do something like this.

edit /etc/haproxy.cfg
mkdir -p /a/chroot/etc
cp /etc/haproxy.cfg /a/chroot/etc
# chmod / chown /a/chroot/etc/aproxy.cfg as needed
haproxy -sr $(cat /var/run/haproxy.pid) # signal restart

> > But I don't think that changing thing to keep it outside would
> > be much trouble at all.
> 
> I don't think it should be an issue either, maybe a minor change
> of the starting sequence. Also, probably that this master will later
> become the one able to log to real files and to do other things that
> need to be done outside of the chroot

As it stands that should be easy enough to handle using the same technique
that is used for the pid file.  Of course adding such log directives on
restart would need to be prohibited.  But that probably needs to happen for
permissions/capabilities reasons anyway.  And in any case it should also
not be difficult.

> > > > As an aside, having the worker pids in the pid file isn't strictly
> > > > necessary.
> > > 
> > > Yes it is, because if the master dies, you have no way to tell which
> > > children were related to the dead service and must be killed to achieve
> > > a sane restart.
> > 
> > True. But the children will detect that the master has died and exit
> > of their own accord.
> 
> I don't see how the children could easily detect that. There's no reason
> they'd get a signal, and we should avoid running getppid() or equivalent
> in the polling loop just to detect a condition which happens once in the
> life of the process.

getppid is run once just before before entering the loop.
And the workers send kill(ppid, 0) while in the loop.
This only occurs in workers when master_worker mode is enabled.

I didn't pay particular attention to performance considerations.
If you are worried about that then perhaps the workers could
kill less often, perhaps using a task.

The feature was a request to allow situations where there is more
than one process but not detached from the terminal - master_worker
either without daemon or with -db (patch forthcoming) to play nicely
with a monitor such as runit[1].

[1] http://smarden.org/runit/

> > > Well, if master_worker mode with -db doesn't daemonize it, that's already
> > > OK then.
> > 
> > Understood. I'll verify that something reasonable occurs on -db.
> 
> OK fine.
> 
> I'll try to find some time ASAP to review your work, it will be easier
> to discuss it and you won't have to explain me things that I should have
> figured by myself if I had tested it :-)

I don't mind explaining things. But I do look forward to a review.

Reply via email to