[Django] #33157: Consider back-porting the fix of #32889 to version 3.2.x
Django
-----------------------------------------+------------------------
Reporter: acu192 | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: 3.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
Prior to the fix for #32889 (and when deployed under an ASGI server, and
for *synchronous* views), a single Django process would only serve one
request at a time. Any blocking (e.g. for the ORM) during serving a single
request would block _all other requests_ from being served. This caused us
issues as we scaled up.
I've manually ported the fix for #32889 into our codebase and *WOOT* it
works great!
We're running Django 3.2.7 and Python 3.7 (btw) and this back-port works
well.
It seems that fix will only go to Django v4.x but I think this should be
back-ported to v3.2.x so more people can benefit. It seems to work as-is,
so the back-port *should* be easy.
--
Ticket URL: <https://code.djangoproject.com/ticket/33157>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: acu192 | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: 3.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by acu192:
Old description:
> Prior to the fix for #32889 (and when deployed under an ASGI server, and
> for *synchronous* views), a single Django process would only serve one
> request at a time. Any blocking (e.g. for the ORM) during serving a
> single request would block _all other requests_ from being served. This
> caused us issues as we scaled up.
>
> I've manually ported the fix for #32889 into our codebase and *WOOT* it
> works great!
>
> We're running Django 3.2.7 and Python 3.7 (btw) and this back-port works
> well.
>
> It seems that fix will only go to Django v4.x but I think this should be
> back-ported to v3.2.x so more people can benefit. It seems to work as-is,
> so the back-port *should* be easy.
New description:
Prior to the fix for #32889 (and when deployed under an ASGI server, and
for *synchronous* views), a single Django process would only serve one
request at a time. Any blocking (e.g. for the ORM) during serving a single
request would block _all other requests_ from being served. This caused us
issues as we scaled up.
I've manually ported the fix for #32889 into our codebase and *WOOT* it
works great!
We're running Django 3.2.7 and Python 3.7 (btw) and this back-port works
well.
It seems that the fix is scheduled only for Django v4.x ... but I think
this should be back-ported to v3.2.x so more people can benefit. It seems
to work as-is, so the back-port *should* be easy.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/33157#comment:1>
Django
Reporter: Ryan Henning | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: 3.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
Does the issue meet a backport criteria described in the
[https://docs.djangoproject.com/en/dev/internals/release-process
/#supported-versions supported versions policy]? (I'm guessing not, since
the committer probably would have recognized the severity of the issue and
already done the backport.)
The description of #32889 says "With Django 4.0 being 3.8+ the required
contextvars module is available." Django 3.2 supports Python 3.6 and 3.7
-- does the patch doesn't work in those cases?
--
Ticket URL: <https://code.djangoproject.com/ticket/33157#comment:2>
Django
Reporter: Ryan Henning | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: 3.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Ryan Henning):
The comment from #32889 is ''almost'' correct ...
[https://docs.python.org/3.7/library/contextvars.html contextvars] was new
in Python 3.7
The fix to #32889 is rather simple, it just makes use of asgiref's
ThreadSensitiveContext which is in asgiref since version 3.3.2 ... and
asgiref handles the lack of contextvars in Python 3.6 smartly (see
[https://github.com/django/asgiref/blob/4364f9b4bb052d9ec54b806867fd859b07c39186/asgiref/sync.py#L16-L19
here] and
[https://github.com/django/asgiref/blob/4364f9b4bb052d9ec54b806867fd859b07c39186/asgiref/sync.py#L58
here] from the asgiref codebase). Btw, Django 3.2.7 already requires
asgiref >= 3.3.2, therefore there are no dependency issues (see
[https://github.com/django/django/blob/45a0c54b670a5d9766fd466c0e05bcc544e47890/setup.cfg#L44
here]).
Therefore the backport indeed will handle both Python 3.6 (no behavior
change will be seen) and Python 3.7 (fix works!). Also it does not require
any dependency changes.
As far as if it is part of the packport policy... I cannot say. The
document you linked gives the following: "The rule of thumb is that fixes
will be backported to the last feature release for bugs that would have
prevented a release in the first place (release blockers)." I would
attempt to argue that this should have been fixed at the first release of
the asgi handler in Django... because it is a step back into the stone
ages when your linux process can only handle one request at a time, thus
it should be backported now under that reasoning.
PS: I've contributed to django/channels_redis (and was added as a
maintainer of that repo) + I'd love the opportunity to contribute to core
Django as well 🤓, so if you'd like I can prepare the PR for this -- just
let me know your thoughts. 😊
--
Ticket URL: <https://code.djangoproject.com/ticket/33157#comment:3>
Django
Reporter: Ryan Henning | Owner: nobody
Type: New feature | Status: closed
Component: HTTP handling | Version: 3.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* cc: Carlton Gibson (added)
* status: new => closed
* resolution: => wontfix
Comment:
Thanks for this report, however it doesn't qualify for a backport based on
our [https://docs.djangoproject.com/en/dev/internals/release-process
/#supported-versions supported versions policy]. #32889 is not even
consider a bug, but a cleanup. Moreover, `ASGIHandler` was added in Django
3.0 so it will not qualify for a backport, even if we consider it a bug,
because the issue has been present since the feature was introduced.
--
Ticket URL: <https://code.djangoproject.com/ticket/33157#comment:4>
Django
Reporter: Ryan Henning | Owner: nobody
Type: New feature | Status: closed
Component: HTTP handling | Version: 3.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Ryan Henning):
Okay, thank you for considering. I understand your opinion on not
backporting. We have to draw the line somewhere -- we cannot backport
everything!
I do still feel concerned for the many Django 3.x + ASGI users and who are
likely unsure why their Django processes are under-performing (e.g. if
looking at requests-per-second). This was our position until I (finally)
found #32889 and backported it into our project. I'll go ahead and file
another ticket about the poor performance of Django 3.x + ASGI so that it
can be fixed independently of #32889.
--
Ticket URL: <https://code.djangoproject.com/ticket/33157#comment:5>
Django
Reporter: Ryan Henning | Owner: nobody
Type: New feature | Status: closed
Component: HTTP handling | Version: 3.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak):
Replying to [comment:5 Ryan Henning]:
> I'll go ahead and file another ticket about the poor performance of
Django 3.x + ASGI so that it can be fixed independently of #32889.
As far as I'm aware this doesn't qualify as a "release blocker" (it's not
a regression), so if that's not an issue in Django 4.0+ these tickets will
be marked as invalid/wontfix.
--
Ticket URL: <https://code.djangoproject.com/ticket/33157#comment:6>