[PATCHES] CTDB - fix PID file problem

[Sumit Bose]

On Wed, Jul 02, 2014 at 09:01:37PM +1000, Martin Schwenke wrote:
> On Wed, 2 Jul 2014 12:36:33 +0200, Sumit Bose <sbose at redhat.com> wrote:
> 
> > On Wed, Jul 02, 2014 at 07:56:43PM +1000, Martin Schwenke wrote:
> > > 2 patches:
> > > 
> > > * ctdb-daemon: Check PID in ctdb_remove_pidfile(), not unreliable flag
> > > 
> > >   If something unexpectedly uses fork() then an exiting child will
> > >   remove the PID file while the main daemon is still running.  The real
> > >   test is whether the current process has the PID of the main CTDB
> > >   daemon.
> > > 
> > > * ctdb-daemon: Remove ctdb->ctdbd_pid, just use ctdbd_pid
> > 
> > Wouldn't it be nicer to remove the global variable?
> > 
> > > 
> > >   The global variable is needed anyway so just use it.
> > 
> > Why? I have found 4 places where ctdbd_pid is used. One is the
> > assignment, at two other place the ctdb_context is already available.
> > The remaining one is in ctdb_tevent_trace() where the ctdb_context must
> > be given as private data in tevent_set_trace_callback() to make
> > ctdb->ctdbd_pid available.
> 
> ... and an extra use is introduced in the 1st patch.  We need a
> reliable way of checking whether the main ctdbd is exiting so that we
> can remove the PID file.  The current method is unreliable (e.g. if a
> library call unexpectedly forks a child that then exits then the PID
> file is mistakenly removed). So, we need to compare against the main
> ctdbd PID.  We remove the PID file in an atexit() handler, which needs a
> global variable.
> 
> If there's a better way of doing the 1st patch then I'm happy to drop
> the 2nd patch.  :-)

Well, you can read the PID from the PID file :-) A more elegant solution
might be getpgrp() which returns the process group ID aka the PID of the
main ctdb daemon. So, getpgrp() == getpid() should be only true for the
main ctdb daemon as long as you do not do weird stuff with setsid(),
setpgid() and friends in the child processes.

HTH

bye,
Sumit

> 
> Personally, I don't understand what people have against global
> variables.  If 90% of the functions need a "struct ctdb_context" that
> is never freed during the lifetime of the daemon, then you've basically
> got a global variable anyway...  ;-)
> 
> peace & happiness,
> martin