Analysis of the mogilefsd busywait bug

[Brad Fitzpatrick]

I kill -9'ed the child in a loop while doing tons of queries and wasn't
able to reproduce...

David, if this patch works for you and fixing your problem, I'll happily
commit... I just wanted to see it work for myself first, but I'm not that
concerned.  Your analysis seems correct.

- Brad


On Tue, 13 Feb 2007, Brad Fitzpatrick wrote:

> Sounds correct.  Thanks!
>
> I'll try and reproduce and verify the fix later today.  Should be as easy
> as kill -9'ing some child processes during heavy traffic/communication
> between them?
>
>
> On Mon, 12 Feb 2007, David Weekly wrote:
>
> > Nathan,
> >
> > So I've taken a peek at the mogilefsd issue you posted to the list in more
> > detail. What follows is my possibly-flawed analysis of what's causing your
> > issue as well as the issue of the others on the list.
> >
> > The epoll fds causing spinning in busywait are waiting on inputs from unix
> > sockets, which appear to be sockets created fairly "early" (9-13 in the case
> > above, just above epoll's own fd of 7) - this tells me the sockets are
> > likely the socketpair()s meant to communicate with children. This
> > corresponds well with us seeing unexpected queryworker deaths in our syslog
> > and corresponds exactly with what the others on the mailing list have seen
> > for problems. That seems to line up with Brad's analysis.
> >
> > The issue seems to be that MogileFS::Connection::Worker->close() isn't
> > called explicitly when MogileFS::ProcManager::PostEventLoopChecker() notices
> > the pid's death (via a successful wait() reaping) . Consequently, I don't
> > believe Danga::Socket->close() is called, which means
> > Danga::Socket->_cleanup() isn't called. Here's the important bit -
> > Danga::Socket->_cleanup() tells epoll it's no longer interested in the file
> > descriptor (EPOLL_CTL_DEL). So epoll continues to report the socketpair()
> > socket as being available for write (although we might get a SIGPIPE if we
> > actually tried writing to the socket).
> >
> > Causing MogileFS::Connection::Worker->close() to be called on the worker
> > whose pid died should (in theory) fix this. Here's the suggested patch (on
> > latest svn checkout). It could kill your mother and is wildly untested.
> > Brad? Brett? Am I on crack?
> >
> > +++ ProcManager.pm      2007-02-13 01:14:01.000000000 +0000
> > @@ -141,6 +141,7 @@
> >              my $extra = $todie{$pid} ? "expected" : "UNEXPECTED";
> >              error("Child $pid ($job) died: $? ($extra)");
> >              MogileFS::ProcManager->NoteDeadChild($pid);
> > +            $jobconn->close();
> >
> >              if (my $jobstat = $jobs{$job}) {
> >                  # if the pid is in %todie, then we have asked it to shut
> > down
> >
>
>