[PATCH] Fix drain logic for small number of connections

[Joel Cunningham]

> On Dec 28, 2016, at 11:05 AM, Maxim Dounin <mdounin at mdounin.ru> wrote:
> 
> Hello!
> 
> On Tue, Dec 27, 2016 at 04:37:36PM -0600, Joel Cunningham wrote:
> 
>> Ping…any interest in accepting this patch?
> 
> I'm not sure this patch improves things.
> 
> Currently, 32 means that under constant load nginx will drain old 
> connections once per 32 connection attempts.  With the patch, 32 
> is reduced to a smaller number, but this doesn't make any 
> difference under constant load: if there are enough connections 
> accepted on the current event loop iteration, all connections from 
> the previous event loop iteration will be closed as long as they 
> haven't managed to provide some data. 
> 

You’re correct that when the worker limit is small (less than 4 times 32), we’ll will free less connections during a drain and under a constant load, ngx_drain_connections ends being called more often.  This seemed a non-issue for situations with small worker limits because freeing up to half or more (in my case all) of the connections on the reusable_connections_queue seemed inappropriate.

Is it the case that 32 was chosen to limit how often we drain rather than being a limit on how much of the queue to drain (and limiting the drain is just a side-effect)? 

> The difference may occur if worker_connections limit is reached 
> occasionally: in this case smaller number can mean that not all 
> keepalive connections will be closed.  But it is not clear how 
> this is different from a server with a large number of established 
> connections and only a small number of reusable ones.

This is the case I’m running into.  You’re correct that we’d still close out newly accepted connections if the number of reusable_connections_queue is smaller than the drain limit.  It happens to be the case in my situation where the reusable_connections_queue is 99% keep alive connections and only a single accepted connection which hasn’t been given the chance to receive the request (the request is actually already queued in the socket buffer, but since accept FD has a lower value than the new connection FD, the connection gets closed out before we can receive on it)

> 
> It is also not clear why use so small number of worker 
> connections, but this is another question.

Joel