Hi, I just figured out some of the missing pieces. When sending USR1 to the workers, they just close() their listening sockets instead of shutdown() (like they do for TTOU), causing the file descriptor to stay open in the parent process, which is why there are still epoll events coming in for it.
Does this make sense? But I guess the patch remains valid. Or maybe one could use shutdown() for the USR1 path, too? Any input appreciated :) Cheers, Conrad On 05/15/2014 11:26 AM, Conrad Hoffmann wrote: > Hi everyone, > > I am still somewhat new to haproxy, so I maybe missing a few bits and > pieces here. If so, don't hesitate to educate me :) > > First of all, we are using HAProxy here at SoundCloud, so a big thanks > to everyone who invested time in this wonderful project! > > We are very keen on using SSL termination with HAProxy, so we have been > running the 1.5-dev builds on non-critical infrastructure. We kept > running into one problem though. Sorry for this rather long description, > put reproduction is kind of difficult, so I want to mention everything > that might be related. > > We use "nbproc 12". To restart HAProxy, we send USR1 to the worker > processes. Usually, as expected, they stop accepting connections, shut > down once all connections are terminated, then the parent process > terminates and gets restarted by the systemd-wrapper (I am aware we > could/should send USR2 to the systemd-wrapper, but I think the use case > is still valid). > > If haproxy has been running for a certain amount of time (this part > makes reproduction cumbersome), we quite often run into the following > issue when restarting haproxy: if some of the workers have active > connections that they must wait for to be terminated, a fraction of > those would start hogging CPU like crazy. Attaching a debugger showed > that epoll_wait() would immediately return with events that they did not > process because they had already closed the file descriptor (and thus > owner == 0x0). This led me to the epoll(7) man page [1], which in its > Q&A section states for Q6 that just closing a fd may not be sufficient > if the inode referred to by that fd is still open in another process, > which is the case when you fork() with open fds like haproxy does. > > The scenario I constructed from this information (and here I am still > missing some knowledge) is that if a processes has a connection going on > when the signal hits, it can happen that it closes a file descriptor > that it doesn't have any connections on but then, while polling for > events on the remaining connection, receives events for the closed fd > because that one is still open in another process. Sorry for this being > kind of vague, but I am still trying to understand the relevant code paths. > > However, based on the above assumption, I came up with the attached > patch as a remedy. Basically it just implements the poller.clo(fd) > handler for epoll, which is already implemented for other pollers. I > can't say with certainty, but while we had bug hit rate of at least 80% > before, I have not been able to trigger the bug in 5 attempts with the > patch applied, so I currently think it actually works. > > I would love to hear some feedback on this issue, maybe someone can give > insight on the actual code path that might be at hand here, or possibly > any side effects this patch might introduce that I have not thought of, etc. > > Thanks a lot and keep being awesome! > Conrad > > [1] http://linux.die.net/man/7/epoll >
