Analysis of the mogilefsd busywait bug

[Brad Fitzpatrick]

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
>