Re: [Django] #34523: Model.objects.update_or_create method sometimes raises TransactionManagementError
Django
TransactionManagementError
-------------------------------------+-------------------------------------
Reporter: gatello-s | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: update_or_create | Triage Stage:
TransactionManagementError | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Bisected to the 331a460f8f2e4f447b68fba491464b68c9b21fd1, however the
`MyISAM` storage engine doesn't support transactions so I'm not sure how
it worked before.
--
Ticket URL: <https://code.djangoproject.com/ticket/34523#comment:12>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
TransactionManagementError
-------------------------------------+-------------------------------------
Reporter: gatello-s | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: update_or_create | Triage Stage:
TransactionManagementError | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
The changes in 331a460f8f2e4f447b68fba491464b68c9b21fd1 are semantically
correct but they had the side effect of making
`BaseDatabaseWrapper._savepoint_allowed` return `False` and thus
`savepoint()` creation always return `None`.
Since `update_or_create` uses nested `atomic` to handle possible integrity
errors it was previously taking the savepoints created by MySQL as way to
mark back the connection as usable. Now that savepoints are not returned
anymore it assumes the outer atomic block is tainted as well and raise
`TransactionManagementError`.
I'm not sure why `Atomic.__enter__` and `__exit__` are not simply noops
when transactions are determined to not be supported instead of trying to
handle all kind of nesting and savepoint combinations.
{{{#!diff
diff --git a/django/db/transaction.py b/django/db/transaction.py
index 4150cbcbbe..28229f8f3a 100644
--- a/django/db/transaction.py
+++ b/django/db/transaction.py
@@ -181,6 +181,8 @@ def __init__(self, using, savepoint, durable):
def __enter__(self):
connection = get_connection(self.using)
+ if not connection.features.supports_transactions:
+ return
if (
self.durable
@@ -223,6 +225,8 @@ def __enter__(self):
def __exit__(self, exc_type, exc_value, traceback):
connection = get_connection(self.using)
+ if not connection.features.supports_transactions:
+ return
if connection.in_atomic_block:
connection.atomic_blocks.pop()
}}}
As for the heuristics to determine if a backend supports transaction the
situation is definitely weird on MySQL as supports is on a per-table basis
and `default_storage_engine` is really just a guess at that.
--
Ticket URL: <https://code.djangoproject.com/ticket/34523#comment:13>
Django
TransactionManagementError
-------------------------------------+-------------------------------------
Reporter: gatello-s | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: update_or_create | Triage Stage:
TransactionManagementError | 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:13 Simon Charette]:
> As for the heuristics to determine if a backend supports transaction the
situation is definitely weird on MySQL as supports is on a per-table basis
and `default_storage_engine` is really just a guess at that. In this sense
331a460f8f2e4f447b68fba491464b68c9b21fd1 is backward incompatible as it
stopped using savepoints which could have an impact on setups that have
mixed storage engines. Not sure if this is a supported setup though.
Would you recommend reverting 331a460f8f2e4f447b68fba491464b68c9b21fd1? (I
fine with that)
--
Ticket URL: <https://code.djangoproject.com/ticket/34523#comment:14>
Django
TransactionManagementError
-------------------------------------+-------------------------------------
Reporter: gatello-s | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: update_or_create | Triage Stage:
TransactionManagementError | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
I think that reverting 331a460f8f2e4f447b68fba491464b68c9b21fd1 is less
controversial than making `Atomic` a noop on databases that don't support
transactions as the latter would require a larger discussion.
--
Ticket URL: <https://code.djangoproject.com/ticket/34523#comment:15>
Django
TransactionManagementError
-------------------------------------+-------------------------------------
| Felisiak
Type: Bug | Status: assigned
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
TransactionManagementError |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Mariusz Felisiak
* status: new => assigned
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/34523#comment:16>
Django
TransactionManagementError
-------------------------------------+-------------------------------------
Reporter: gatello-s | Owner: Mariusz
| Felisiak
Type: Bug | Status: assigned
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: update_or_create | Triage Stage: Accepted
TransactionManagementError |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/16813 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/34523#comment:17>
Django
TransactionManagementError
-------------------------------------+-------------------------------------
Reporter: gatello-s | Owner: Mariusz
| Felisiak
Type: Bug | Status: assigned
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: update_or_create | Triage Stage: Accepted
TransactionManagementError |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Francesco Panico (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/34523#comment:18>
Django
TransactionManagementError
-------------------------------------+-------------------------------------
Reporter: gatello-s | Owner: Mariusz
| Felisiak
Component: Database layer | Version: 4.2
(models, ORM) |
Keywords: update_or_create | Triage Stage: Accepted
TransactionManagementError |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"83339d2103f447377fedb3bd145b52c3ef2b667c" 83339d21]:
{{{
#!CommitTicketReference repository=""
revision="83339d2103f447377fedb3bd145b52c3ef2b667c"
Fixed #34523 -- Fixed TransactionManagementError in
QuerySet.update_or_create() with MyISAM storage engine.
QuerySet.update_or_create() uses nested atomic to handle possible
integrity errors taking savepoints as way to mark back the connection
as usable. Savepoints are not returned when
uses_savepoints/can_release_savepoints feature flags are set to False.
As a consequence, QuerySet.update_or_create() assumed the outer atomic
block is tainted and raised TransactionManagementError.
This commit partly reverts 331a460f8f2e4f447b68fba491464b68c9b21fd1.
Thanks gatello-s for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34523#comment:19>