[Django] #31637: Registering database connections for cleanup on fork
Django
-------------------------------------+-------------------------------------
Reporter: Aarni | Owner: nobody
Koskela |
Type: New | Status: new
feature |
Component: Database | Version: 3.0
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
When using `multiprocessing.Pool()` or other process-forking APIs, one
might bump into
{{{
django.db.utils.OperationalError: SSL error: decryption failed or bad
record mac
}}}
or similar inconsistency errors in the child processes, since the socket
is passed down into the forked process.
The quick fix is to
{{{
from django import db
db.connections.close_all()
}}}
before forking.
Python 3.7 introduced the `os.register_at_fork` function:
[https://docs.python.org/3.8/library/os.html#os.register_at_fork]
It could be a good idea for Django to use this function to register
database connections to be discarded (not cleanly closed, just dropped, as
far as a forked process is concerned!) in forked child processes? That way
the parent process could use established connection state as before.
--
Ticket URL: <https://code.djangoproject.com/ticket/31637>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Simon Charette (added)
* stage: Unreviewed => Accepted
Comment:
I think we should do this even if we're moving away from `fork`'ing
internally (e.g. parallel test runner moving to `spawn`) as it's a really
common pitfall when using multiprocessing with the ORM.
--
Ticket URL: <https://code.djangoproject.com/ticket/31637#comment:1>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: assigned
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Josh
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/31637#comment:2>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: assigned
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
https://github.com/django/django/pull/16212
--
Ticket URL: <https://code.djangoproject.com/ticket/31637#comment:3>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: assigned
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
Thanks for the patch Josh, it was nice meeting you during the sprint! I
left some comments for improvements on the PR so I marked the ticket
accordingly. Please uncheck ''patch needs improvement'' once you've
addressed them so the PR ticket appears in the review queue.
--
Ticket URL: <https://code.djangoproject.com/ticket/31637#comment:4>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: assigned
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Florian Apolloner):
I am not sure we should be doing this (at least not at this level). We do
not have any control over what happens in `close_all`. If any of the
connections where to perform some cleanup that affects server state this
might fail horribly.
--
Ticket URL: <https://code.djangoproject.com/ticket/31637#comment:5>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: assigned
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
Florian, are you referring to the possibility that closing a connection in
a subprocess could cause the connection in the parent process to be closed
as well?
If you believe this is a possibility could we at least emit a warning
within child process if they are initialized with opened connections that
points at issues that might arise?
--
Ticket URL: <https://code.djangoproject.com/ticket/31637#comment:6>