[syslog-ng] time_sleep() and inadvertent admission control

[John Morrissey]

On Wed, Feb 20, 2008 at 06:48:09PM +0100, Balazs Scheidler wrote:
> On Sat, 2008-01-26 at 15:18 +0100, Balazs Scheidler wrote:
> > On Thu, 2008-01-24 at 14:18 -0500, John Morrissey wrote:
> > > We noticed recently that time_sleep() exhibits some inadvertent
> > > admission control behavior. When poll() indicates that the listener
> > > socket has activity (new connections), syslog-ng seems to accept()
> > > only once on it, allowing one new connection per poll(). As a result,
> > > it only allows:
> > > 
> > >   1000 / time_sleep()
> > > 
> > > connections per second. Accordingly, with time_sleep(30), only 33
> > > connections would be allowed every second. 
> > 
> >  * do multiple accepts per poll loop; or
> >  * increase the I/O priority for the listeners
> > 
> > The first one easily increases the incoming connection rate and is
> > simple to implement, the second is more complex and might cause further
> > unexpected behaviour:
> > 
> > if the priority of the listeners is increased, that would mean that any
> > incoming connection might starve the incoming message stream, e.g. if
> > there's a continous stream of incoming connections, then long-living
> > connections might be starved.
> > 
> > So I'd choose the first option, what do you think?
> 
> Just a quick note to let you know that I've just pushed a patch that
> implements multiple accepts per poll iteration.
> 
> It's in the 2.1 branch, but it is probably trivial to backport it over
> to 2.0, I'm going to do that when a maintenance release for 2.0 is due.
> 
> It seems to work for me, and I think it should solve your performance
> issues.
> 
> http://git.balabit.hu/?p=bazsi/syslog-ng-2.1.git;a=commit;h=33f45d65490c2e8e3c52e468ec733266a0230b13

The backport was trivial; it patched cleanly (save for a small offset in one
hunk) and the resulting code looks sane. It's great to have a response like
this; I really appreciate your work with syslog-ng.

Running this patched version under the affected workload seems to completely
fix the problem. The maximum number of accept() calls per iteration was only
15, and we only got over 10 accepts/iter about 1% of the time.

john
-- 
John Morrissey          _o            /\         ----  __o
jwm at horde.net        _-< \_          /  \       ----  <  \,
www.horde.net/    __(_)/_(_)________/    \_______(_) /_(_)__