Opened 11 years ago

Closed 11 years ago

Last modified 7 years ago

#20537 closed Bug (fixed)

`request_finished` signal not being called with Django 1.5 and uWSGI < 1.2.6(?)

Reported by: Jaap Roes Owned by: Aymeric Augustin
Component: Documentation Version: 1.5
Severity: Normal Keywords: uwsgi postgres mysql idle connections
Cc: Alan Justino da Silva Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

As discovered on the django-users mailinglist in the thread request_finished signal not being called with Django 1.5.1 with uwsgi

The version of uWSGI in the Ubuntu/Debian apt repositories (1.0.3) doesn't call close on the wsgi application, this in turn causes the request_finished signal to never fire resulting in open database connections.

According to this blogpost uWSGI >= 1.2.6 should be the first version of uWSGI that supports the expected behavior. But the same blogpost warns about other wsgi middleware potentially misbehaving.

I'm wondering why Django < 1.5 didn't show this behavior and what the best course for Django regarding this issue is.

Should there be a warning in the docs about the minimum required uWSGI version? Or should the previous behavior be restored somehow?

Change History (19)

comment:1 by Aymeric Augustin, 11 years ago

This is a bug in uWSGI, I've hit it in production too. Thanks for filing this ticket.

Django < 1.5 didn't exhibit this behavior because it closed the database connection earlier. But code could reopen the connection later on, before the end of the request, in the case of streaming responses. That was a bug, I fixed it, and I still think it's the right thing to do.

Since Django 1.5 provides explicit support for streaming responses, the closing of the database connection must be performed after the content is fully sent, ie. in close().

I think the best we can do is document this bug of uWSGI in the deployment docs and in the 1.5 release notes.

comment:2 by Claude Paroz, 11 years ago

Component: UncategorizedDocumentation
Triage Stage: UnreviewedAccepted

comment:3 by Jaap Roes, 11 years ago

Owner: changed from nobody to Jaap Roes
Status: newassigned

I think the following pages need at least a note about the known issue with uWSGI. I can confirm the fist version that should no longer show this behavior is 1.2.6 and up (uWSGI 1.2.6 announcement.

Release notes - request-finished

How to use Django with uWSGI

Maybe include a more general description of the problem on these pages since the issue might crop up with "badly implemented" wsgi middleware as well.

Signals - request-finished

How to deploy with WSGI

I'll try to whip up a patch.

comment:4 by Jaap Roes, 11 years ago

There's a note for the request-finished signal that says:

When a view returns a streaming response, this signal is sent only after the entire response is consumed by the client (strictly speaking, by the WSGI gateway).

If I'm understanding correctly this is not entirely true. This behavior occurs regardless of a streaming response, but was implemented to facilitate streaming responses. Going to slightly alter that note as well.

comment:5 by Aymeric Augustin, 11 years ago

Well, technically the sentence is correct :) But it implies that the behavior is different for regular responses, which isn't true :(

I'm not a native speaker, I struggle with writing good docs in English, improvements are welcome.

comment:6 by Jaap Roes, 11 years ago

Easy pickings: set
Has patch: set

I've struggled a bit as well, but I think I've hit the mark in this pull request.

I've folded the note in the signals page under the "Changed in Django 1.5" as both of those were trying to say the same thing.

comment:7 by Aymeric Augustin, 11 years ago

Owner: changed from Jaap Roes to Aymeric Augustin

Thank you. I'll review your proposal.

comment:8 by Tim Graham, 11 years ago

Triage Stage: AcceptedReady for checkin

Looks good to me, left one comment on the pull request. I'll let Aymeric review it as well.

comment:9 by Aymeric Augustin, 11 years ago

PR looks good, thank you, I'll push it (with negligible edits).

comment:10 by Aymeric Augustin <aymeric.augustin@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In 3ce1d303daa92ecb1bc3ba6344f1ec9746a79c70:

Warned that request_finished isn't sent by some buggy setups.

Older versions of uWSGI and Sentry's middleware do not adhere to
the WSGI spec and cause the request_finished signal to never
fire. Added notes to the appropriate places in the docs.

Fixed #20537.

comment:11 by Aymeric Augustin <aymeric.augustin@…>, 11 years ago

In 60353458ae7105ea943dc425954403c581b05255:

[1.5.x] Warned that request_finished isn't sent by some buggy setups.

Older versions of uWSGI and Sentry's middleware do not adhere to
the WSGI spec and cause the request_finished signal to never
fire. Added notes to the appropriate places in the docs.

Fixed #20537.

Backport of 3ce1d30.

Conflicts:

docs/howto/deployment/wsgi/index.txt

comment:12 by Jaap Roes, 11 years ago

Cool, thanks!

comment:13 by anonymous, 11 years ago

Bug exists in Django 1.5.2 and uwsgi 1.9.12

comment:14 by Aymeric Augustin, 11 years ago

Do you have evidence of that behavior?

comment:15 by tomek@…, 11 years ago

I found it after first test deployment. Symptoms are most suitable for this ticket.
Everything works fine for devserver,gunicorn and [uwsgi] processes=1

$uname -a
Linux ip-10-55-75-70 3.2.0-40-virtual #64-Ubuntu SMP Mon Mar 25 22:07:32 UTC 2013 i686 i686 i386 GNU/Linux
$ django-admin.py --version
1.5.2
$cat booking.ini
uid = djangoapps
gid = djangoapps
vhost = true
plugins = http,python
master = true
enable-threads = false
processes = 1
socket = 127.0.0.1:3031
;wsgi-file = /home/djangoapps/projects/booking/booking/wsgi.py
module=booking.wsgi:application
virtualenv = /home/djangoapps/virtualenvs/booking
chdir = /home/djangoapps/projects/booking
pythonpath = /home/djangoapps/projects/booking
env = DJANGO_SETTINGS_MODULE=booking.settings.test
touch-reload = /home/djangoapps/projects/booking/reload
daemonize = /var/log/uwsgi/app/booking.log

#settings.py
DATABASES = {
    'default': {
        'ENGINE': 'django.db.backends.mysql', # Add 'postgresql_psycopg2', 'mysql', 'sqlite3' or 'oracle'.
        'NAME': 'booking',
        # The following settings are not used with sqlite3:
        'USER': 'root',
        'PASSWORD': 'test',
        'HOST': '127.0.0.1',
        'PORT': '3306',                      # Set to empty string for default.
    }
}

CACHES = {
    'default': {
        'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache',
        'LOCATION': '127.0.0.1:11211',
    }
}


SESSION_ENGINE = 'django.contrib.sessions.backends.cache'
MIDDLEWARE_CLASSES = (
    'django.middleware.common.CommonMiddleware',
    'django.contrib.sessions.middleware.SessionMiddleware',
    'django.middleware.csrf.CsrfViewMiddleware',
    'django.contrib.auth.middleware.AuthenticationMiddleware',
    'django.contrib.messages.middleware.MessageMiddleware',
    # Uncomment the next line for simple clickjacking protection:
    # 'django.middleware.clickjacking.XFrameOptionsMiddleware',
)

INSTALLED_APPS = (
    'django.contrib.auth',
    'django.contrib.contenttypes',
    'django.contrib.sessions',
    'django.contrib.sites',
    'django.contrib.messages',
    'django.contrib.staticfiles',
    #'stacks',
    #'mptt',
    #'menus',
    'south',
    #'sekizai',
    'reversion',
    # Uncomment the next line
)

#views.py
def index(request)
    """Works for
    [uwsgi]processes=1 
    gunicorn,dev server
    """
    user = authenticate(username='username', password='password1')
    login(request, user)
    return HttpResponseRedirect('/test_login/')

def index2(request)
    """Works for:
    [uwsgi]processes>1
    gunicorn, dev server
    """
    user = authenticate(username='username', password='password1')
    login(request, user)
    return test_login(request)

comment:16 by Alan Justino da Silva, 10 years ago

Cc: Alan Justino da Silva added

Just to clarify, this signal is sent before or after middleware processing?

comment:17 by Aymeric Augustin, 10 years ago

After.

comment:18 by Tim Graham <timograham@…>, 7 years ago

In 351835f2:

Fixed #28031 -- Removed notes about old uWSGI/sentry versions (refs #20537).

comment:19 by Tim Graham <timograham@…>, 7 years ago

In 0cb009a:

[1.11.x] Fixed #28031 -- Removed notes about old uWSGI/sentry versions (refs #20537).

Backport of 351835f26234cade43f0bed45441fc144c33a785 from master

Note: See TracTickets for help on using tickets.
Back to Top