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.