emmett-framework / granian

A Rust HTTP server for Python applications
BSD 3-Clause "New" or "Revised" License
2.87k stars 83 forks source link

WSGI mode & Django: Idle database connections aren't closed #306

Closed matthiask closed 5 months ago

matthiask commented 5 months ago

Maybe this rings a bell, and if not, do you have any hints on how to debug this?

When using granian in WSGI mode with a Django app, idle database connections aren't reaped for some reason. The exact same configuration with gunicorn runs fine. I have the suspicion that close_old_connections doesn't run under some circumstances: https://github.com/django/django/blob/d3a7ed5bcc45000a6c3dd55d85a4caaa83299f83/django/db/__init__.py#L56-L62 but I don't really have a way to verify it yet. It takes a few days to use up all database connections, so it's quite certainly not a thing which happens after each request.

gi0baro commented 5 months ago

@matthiask it looks to me that signal depends on the close call on the response iterator https://github.com/django/django/blob/d3a7ed5bcc45000a6c3dd55d85a4caaa83299f83/django/http/response.py#L335, which Granian should call here https://github.com/emmett-framework/granian/blob/master/src/wsgi/types.rs#L175

Probably the easiest way to debug this would be:

I can add a simple test to the suite to validate the iterator close call gets actually called in the meantime, at least to have some "generic" green state.

gi0baro commented 5 months ago

@matthiask actually looking at the code, it seems Granian doesn't close the iterator in case of exceptions: https://github.com/emmett-framework/granian/blob/master/src/wsgi/types.rs#L192

Can you confirm that the signal is not getting called in case of exceptions and it does normally?

matthiask commented 5 months ago

Thanks for your answer!

I'm not sure if I did it correctly; the handling has to happen outside Django's handler since it will eat all exceptions and convert them into proper internal server error responses.

I wrapped the WSGI app with the following middleware, and now I get an idle connection after each page load on the app. A group of idle connections are closed from time to time, but each request/response cycle creates a new idle connection. Unfortunately everything runs in a cluster and instrumenting and debugging everything is much more annoying than it should be. Your hypothesis is probably correct though, at least it does sound like it.

class Middleware:
    def __init__(self, application):
        self.application = application

    def __call__(self, environ, start_response):
        iterable = self.application(environ, start_response)
        yield from iterable

        raise Exception  # not StopIteration

application = Middleware(application)
gi0baro commented 5 months ago

@matthiask thank you for confirming this, gonna be fixed in the next patch release

matthiask commented 5 months ago

@gi0baro Thank you!

gi0baro commented 5 months ago

@matthiask also fyi with Granian 1.4.x in WSGI the maximum number of parallel threads interacting with Python will be the one from --backpressure. So if you have a database connection pool, I suggest to set backpressure to the max of that pool.

matthiask commented 5 months ago

@gi0baro Thanks for the heads up!

By the way, I loved the TalkPython episode. Thanks for the insights.