[thread] close sockets at graceful shutdown

[tilgovi]

The run loop has to change slightly to support graceful shutdown.
There is no way to interrupt a call to futures.wait so instead
the pattern, used by the async workers, is to sleep for only one
second at the most. The poll is extended to a one second timeout
to match.

Since threads are preemptively scheduled, it's possible that the
listener is closed when the request is actually handled. For this
reason it is necessary to slightly refactor the TConn class to store
the listening socket name. The name is checked once at the start of
the worker run loop.

Ref #922

[tilgovi]

Open questions:

1. Should we cancel all pending futures after exiting the main loop?
   Doing so means that connections that have been accepted but have not started execution on the thread pool are closed immediately. Not doing so means that keep-alive connections can continue to make requests until the graceful timeout elapses. Both behaviors seem sane.
2. Should we close all keep-alive requests when gracefully shutting down? Currently, the other workers do not.

[benoitc]

@tilgovi sorry for the delay... response follows

1. The second behaviour seems better to me
2. You mean until the worker timeout right? Imo On graceful shutdown we should stop to accept any connections on old workers. We should fix other workers to handle it, ie stopping the keepalive loop once we stop to accept. Do you want to make that change? I can take if you don't have much time for it :)

[benoitc]

We may want to return faster in the loop there. Why increasing the wait time?

[tilgovi]

Why faster? We sleep for 1.0 in gevent and eventlet. The granularity here only matters for closing keep-alive connections and responding to graceful shutdown signals. The poll returns as soon as a connection is ready for accept or ready to read a keep-alive request, and the actual requests are handled in other threads. This main thread should sleep as long as it can.

[benoitc]

the reason to be faster there is that this selector is done on read to check if the socket enter in a state where we can accept. If we wait too much we can queue more accepts than we want and we fail to balance correctly the connections between threads and workers. So we should be fast there imo.

[tilgovi]

It will return as soon as the first socket is ready. There is no danger
such as you describe.

On Fri, Mar 25, 2016, 03:10 Benoit Chesneau notifications@github.com
wrote:

In gunicorn/workers/gthread.py
#1230 (comment):

@@ -205,33 +207,37 @@ def run(self):
# can we accept more connections?
if self.nr_conns < self.worker_connections:
# wait for an event

•            events = self.poller.select(0.02)
  

•            events = self.poller.select(1.0)
  

the reason to be faster there is that this selector is done on read to
check if the socket enter in a state where we can accept. If we wait too
much we can queue more accepts than we want and we fail to balance
correctly the connections between threads and workers. So we should be fast
there imo.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/benoitc/gunicorn/pull/1230/files/f2418a95e0df21ae53fc8dba96995e18b2efa961#r57432600

[benoitc]

hrm actually probably. go ahead then :)

[tilgovi]

This change brings the thread worker in line with asyncio and gevent (and eventlet if #1222) is merged, so that we can close #922.

I can make the keep-alive pool shut down at the start of graceful close, but that is a change to every worker that maybe I should make a separate issue.