[Django] #31315: Closing all connections in function tagged with @transaction_atomic or transaction atomic block breaks atomicity
Django
transaction atomic block breaks atomicity
-------------------------------------+-------------------------------------
Reporter: David | Owner: nobody
Eling |
Type: Bug | Status: new
Component: Database | Version: 2.1
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 |
-------------------------------------+-------------------------------------
Bug might be related to https://code.djangoproject.com/ticket/21239
If you use the transaction atomic tag on a function ala
{{{
@transaction.atomic
def post(self, request):
Machine.objects.create(name="a")
connections.close_all()
connection.connect()
list(Machine.objects.all())
Machine.objects.create(name="b")
return JsonResponse({})
}}}
b will exist but a will not.
Now the code above is way simplified compared to what actually caused the
bug in my codebase but it replicates the issue (although it seems very
arbitrary in its current form) The codebase in question is very complex
and involves multiple databases on different machines, this is my best
attempt at boiling it down to what bits caused what. So it calls
connections.close_all() and then reopens the connection to the current db
then does a query then later down the line it does a create.
The issue will occur if you do this instead
{{{
def post(self, request):
with transaction.atomic():
Machine.objects.create(name="a")
connections.close_all()
connection.connect()
list(Machine.objects.all())
Machine.objects.create(name="b")
return JsonResponse({})
}}}
During further testing it looks like swapping connections.close_all() with
connection.close() still causes the issue to occur, so maybe the fix in
the issue linked above did not fix the issue all the way.
It's possible this bug is only happening due to the code exploiting some
loophole or something but I just thought I would put this here just in
case it might be a real problem that needs to be fixed. Within our code
base we had to isolate the offending bit and do it before entering the
atomic block.
This is my first time reporting a django bug so I apologize for any
issues.
--
Ticket URL: <https://code.djangoproject.com/ticket/31315>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
transaction atomic block breaks atomicity
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => wontfix
Comment:
Hi David. Thanks for the report. Interesting... :)
I think this is expected behaviour.
[https://docs.djangoproject.com/en/3.0/topics/db/sql/#connections-and-
cursors The docs go as far as]:
> `connection` and `cursor` mostly implement the standard Python DB-API
described in PEP 249
[https://www.python.org/dev/peps/pep-0249/#Connection.close That defines
the behaviour for `close()`]:
> Note that closing a connection without committing the changes first will
cause an implicit rollback to be performed.
[https://github.com/django/django/blob/d4fff711d4c97356bd6ba1273d2a5e349326eb5f/django/db/backends/base/base.py#L296-L298
Looking at the code], we have exactly that:
{{{
if self.in_atomic_block:
self.closed_in_transaction = True
self.needs_rollback = True
}}}
> Within our code base we had to isolate the offending bit and do it
before entering the atomic block.
I think that's the right approach. My first guess reading this would be
that closing the connection inside a transaction would be at best
undefined. ''What do I expect to happen there?'' — I could see a few
answers to that. That it's ''rollback'' is no real surprise.
I hope that makes sense?
Kind Regards,
Carlton
--
Ticket URL: <https://code.djangoproject.com/ticket/31315#comment:1>