Opened 4 years ago
Last modified 2 years ago
#31637 assigned New feature
Registering database connections for cleanup on fork
Reported by: | Aarni Koskela | Owned by: | Josh Thomas |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 3.0 |
Severity: | Normal | Keywords: | |
Cc: | Simon Charette | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
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.
Change History (6)
comment:1 by , 4 years ago
Cc: | added |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 2 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Working on this right now, using the os.register_at_fork
hook as suggested. I think I've got a solution in place, just working on the actual tests.
comment:4 by , 2 years ago
Patch needs improvement: | set |
---|
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.
comment:5 by , 2 years ago
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.
comment:6 by , 2 years ago
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?
I think we should do this even if we're moving away from
fork
'ing internally (e.g. parallel test runner moving tospawn
) as it's a really common pitfall when using multiprocessing with the ORM.