Opened 19 months ago
Closed 19 months ago
#34523 closed Bug (fixed)
Model.objects.update_or_create method sometimes raises TransactionManagementError
Reported by: | gatello-s | Owned by: | Mariusz Felisiak |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 4.2 |
Severity: | Normal | Keywords: | update_or_create TransactionManagementError |
Cc: | Anton Plotkin, Francesco Panico | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
When using with myisam-only database the method update_or_create can occur a TransactionManagementError exception if creating a record was unsuccessful (because of IntegrityError, for example the record with a same primary key was already created by another process).
The problem has started after the upgrading from Django 3.2.16 to 4.1.7
The test below just simulates a parallel insertion of the record with the same PK
Database backend: mariadb server (10.5.19) (default-storage-engine is myisam).
This test works fine with Django 3.2.16 and fails on Django 4.1.7+:
class TransactionManagementErrorTest(TestCase): class TestModel(models.Model): managed = False field = models.IntegerField(null=True) class Meta(object): db_table = 'test_model_update_or_create' class QuerySet(models.QuerySet): def create(self, **kwargs): super().create(**kwargs) # simulate parallel insertion return super().create(**kwargs) class TestModelManager(models.Manager.from_queryset(QuerySet)): pass objects = TestModelManager() def exec_sql(self, sql): from django.db import connections, router db = router.db_for_write(self.TestModel) connection = connections[db] connection.cursor().execute(sql) def setUp(self): super().setUp() self.exec_sql( 'CREATE TABLE IF NOT EXISTS `test_model_update_or_create` (`id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY, `field` integer NULL);' ) def tearDown(self): self.exec_sql( 'DROP TABLE IF EXISTS `test_model_update_or_create`;' ) super().tearDown() def test_update_or_create(self): self.TestModel.objects.update_or_create(id=1, defaults={'field': 2})
Exception: django.db.transaction.TransactionManagementError: An error occurred in the current transaction. You can't execute queries until the end of the 'atomic' block.
Failure stack:
Traceback (most recent call last): File "/home/alex/workspace/test_TransactionManagementError/mysite/polls/tests.py", line 47, in test_update_or_create self.TestModel.objects.update_or_create(id=1, defaults={'field': 2}) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/manager.py", line 87, in manager_method return getattr(self.get_queryset(), name)(*args, **kwargs) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 949, in update_or_create obj, created = self.select_for_update().get_or_create(defaults, **kwargs) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 926, in get_or_create return self.get(**kwargs), False File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 633, in get num = len(clone) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 380, in __len__ self._fetch_all() File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 1881, in _fetch_all self._result_cache = list(self._iterable_class(self)) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 91, in __iter__ results = compiler.execute_sql( File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/sql/compiler.py", line 1560, in execute_sql cursor.execute(sql, params) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/backends/utils.py", line 67, in execute return self._execute_with_wrappers( File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/backends/utils.py", line 80, in _execute_with_wrappers return executor(sql, params, many, context) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/backends/utils.py", line 83, in _execute self.db.validate_no_broken_transaction() File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/backends/base/base.py", line 531, in validate_no_broken_transaction raise TransactionManagementError( django.db.transaction.TransactionManagementError: An error occurred in the current transaction. You can't execute queries until the end of the 'atomic' block.
Change History (19)
comment:1 by , 19 months ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
comment:2 by , 19 months ago
Resolution: | needsinfo |
---|---|
Status: | closed → new |
comment:3 by , 19 months ago
I ran the test above (I had to add managed=False to the model) on 3.2.16 and I get the same error as it does on main.
Here's the failure with stack trace:
====================================================================== ERROR: test_update_or_create (ticket_34523.tests.TransactionManagementErrorTest) ---------------------------------------------------------------------- Traceback (most recent call last): File "/Users/dsanders/projects/django-sample/django/db/models/query.py", line 581, in get_or_create return self.get(**kwargs), False File "/Users/dsanders/projects/django-sample/django/db/models/query.py", line 435, in get raise self.model.DoesNotExist( ticket_34523.tests.TransactionManagementErrorTest.TestModel.DoesNotExist: TestModel matching query does not exist. During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/Users/dsanders/projects/django-sample/django/db/backends/utils.py", line 84, in _execute return self.cursor.execute(sql, params) File "/Users/dsanders/projects/django-sample/django/db/backends/mysql/base.py", line 73, in execute return self.cursor.execute(query, args) File "/Users/dsanders/projects/django-sample/.direnv/python-3.10.8/lib/python3.10/site-packages/MySQLdb/cursors.py", line 206, in execute res = self._query(query) File "/Users/dsanders/projects/django-sample/.direnv/python-3.10.8/lib/python3.10/site-packages/MySQLdb/cursors.py", line 319, in _query db.query(q) File "/Users/dsanders/projects/django-sample/.direnv/python-3.10.8/lib/python3.10/site-packages/MySQLdb/connections.py", line 255, in query _mysql.connection.query(self, query) MySQLdb.IntegrityError: (1062, "Duplicate entry '1' for key 'test_model_update_or_create.PRIMARY'") The above exception was the direct cause of the following exception: Traceback (most recent call last): File "/Users/dsanders/projects/django-sample/ticket_34523/tests.py", line 44, in test_update_or_create self.TestModel.objects.update_or_create(id=1, defaults={"field": 2}) File "/Users/dsanders/projects/django-sample/django/db/models/manager.py", line 85, in manager_method return getattr(self.get_queryset(), name)(*args, **kwargs) File "/Users/dsanders/projects/django-sample/django/db/models/query.py", line 608, in update_or_create obj, created = self.select_for_update().get_or_create(defaults, **kwargs) File "/Users/dsanders/projects/django-sample/django/db/models/query.py", line 588, in get_or_create return self.create(**params), True File "/Users/dsanders/projects/django-sample/ticket_34523/tests.py", line 16, in create return super().create(**kwargs) File "/Users/dsanders/projects/django-sample/django/db/models/query.py", line 453, in create obj.save(force_insert=True, using=self.db) File "/Users/dsanders/projects/django-sample/django/db/models/base.py", line 739, in save self.save_base(using=using, force_insert=force_insert, File "/Users/dsanders/projects/django-sample/django/db/models/base.py", line 776, in save_base updated = self._save_table( File "/Users/dsanders/projects/django-sample/django/db/models/base.py", line 881, in _save_table results = self._do_insert(cls._base_manager, using, fields, returning_fields, raw) File "/Users/dsanders/projects/django-sample/django/db/models/base.py", line 919, in _do_insert return manager._insert( File "/Users/dsanders/projects/django-sample/django/db/models/manager.py", line 85, in manager_method return getattr(self.get_queryset(), name)(*args, **kwargs) File "/Users/dsanders/projects/django-sample/django/db/models/query.py", line 1270, in _insert return query.get_compiler(using=using).execute_sql(returning_fields) File "/Users/dsanders/projects/django-sample/django/db/models/sql/compiler.py", line 1416, in execute_sql cursor.execute(sql, params) File "/Users/dsanders/projects/django-sample/django/db/backends/utils.py", line 66, in execute return self._execute_with_wrappers(sql, params, many=False, executor=self._execute) File "/Users/dsanders/projects/django-sample/django/db/backends/utils.py", line 75, in _execute_with_wrappers return executor(sql, params, many, context) File "/Users/dsanders/projects/django-sample/django/db/backends/utils.py", line 79, in _execute with self.db.wrap_database_errors: File "/Users/dsanders/projects/django-sample/django/db/utils.py", line 90, in __exit__ raise dj_exc_value.with_traceback(traceback) from exc_value File "/Users/dsanders/projects/django-sample/django/db/backends/utils.py", line 84, in _execute return self.cursor.execute(sql, params) File "/Users/dsanders/projects/django-sample/django/db/backends/mysql/base.py", line 73, in execute return self.cursor.execute(query, args) File "/Users/dsanders/projects/django-sample/.direnv/python-3.10.8/lib/python3.10/site-packages/MySQLdb/cursors.py", line 206, in execute res = self._query(query) File "/Users/dsanders/projects/django-sample/.direnv/python-3.10.8/lib/python3.10/site-packages/MySQLdb/cursors.py", line 319, in _query db.query(q) File "/Users/dsanders/projects/django-sample/.direnv/python-3.10.8/lib/python3.10/site-packages/MySQLdb/connections.py", line 255, in query _mysql.connection.query(self, query) django.db.utils.IntegrityError: (1062, "Duplicate entry '1' for key 'test_model_update_or_create.PRIMARY'")
comment:5 by , 19 months ago
Besides knowing what exception the reporter is getting, I would like to understand what is the desired outcome, considering that the test is setting the same id for both insertions. I would expect an IntegrityError as shown by David.
comment:6 by , 19 months ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
I'd be surprise at any change in the behavior here. Also, MyISAM storage engine doesn't support transactions, etc. I don't think you've explained the issue in enough detail to confirm a bug in Django. Please reopen the ticket if you can debug your issue and provide details about why and where Django is at fault.
comment:7 by , 19 months ago
code from db/models/query.py
return self.create(**params), True
# test generate IntegrityError, this ok
return self.get(**kwargs), False
# this must work fine, but raise django.db.transaction.TransactionManagementError
def get_or_create(self, defaults=None, **kwargs): """ Look up an object with the given kwargs, creating one if necessary. Return a tuple of (object, created), where created is a boolean specifying whether an object was created. """ # The get() needs to be targeted at the write database in order # to avoid potential transaction consistency problems. self._for_write = True try: return self.get(**kwargs), False except self.model.DoesNotExist: params = self._extract_model_params(defaults, **kwargs) # Try to create an object using passed params. try: with transaction.atomic(using=self.db): params = dict(resolve_callables(params)) return self.create(**params), True # test generate IntegrityError, this ok except IntegrityError: try: return self.get(**kwargs), False # this must work fine, but raise django.db.transaction.TransactionManagementError except self.model.DoesNotExist: pass raise
ERROR: test_update_or_create (polls.tests.TransactionManagementErrorTest) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 916, in get_or_create return self.get(**kwargs), False File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 637, in get raise self.model.DoesNotExist( polls.tests.TransactionManagementErrorTest.TestModel.DoesNotExist: TestModel matching query does not exist. During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/backends/utils.py", line 89, in _execute return self.cursor.execute(sql, params) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/backends/mysql/base.py", line 75, in execute return self.cursor.execute(query, args) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/MySQLdb/cursors.py", line 206, in execute res = self._query(query) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/MySQLdb/cursors.py", line 320, in _query self._do_get_result(db) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/MySQLdb/cursors.py", line 145, in _do_get_result self._result = result = self._get_result() File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/MySQLdb/cursors.py", line 352, in _get_result return self._get_db().store_result() MySQLdb.IntegrityError: (1062, "Duplicate entry '1' for key 'PRIMARY'") The above exception was the direct cause of the following exception: Traceback (most recent call last): File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 923, in get_or_create return self.create(**params), True File "/home/alex/workspace/test_TransactionManagementError/mysite/polls/tests.py", line 19, in create return super().create(**kwargs) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 658, in create obj.save(force_insert=True, using=self.db) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/base.py", line 814, in save self.save_base( File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/base.py", line 877, in save_base updated = self._save_table( File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/base.py", line 1020, in _save_table results = self._do_insert( File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/base.py", line 1061, in _do_insert return manager._insert( File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/manager.py", line 87, in manager_method return getattr(self.get_queryset(), name)(*args, **kwargs) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 1805, in _insert return query.get_compiler(using=using).execute_sql(returning_fields) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/sql/compiler.py", line 1820, in execute_sql cursor.execute(sql, params) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/backends/utils.py", line 67, in execute return self._execute_with_wrappers( File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/backends/utils.py", line 80, in _execute_with_wrappers return executor(sql, params, many, context) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/backends/utils.py", line 89, in _execute return self.cursor.execute(sql, params) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/utils.py", line 91, in __exit__ raise dj_exc_value.with_traceback(traceback) from exc_value File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/backends/utils.py", line 89, in _execute return self.cursor.execute(sql, params) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/backends/mysql/base.py", line 75, in execute return self.cursor.execute(query, args) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/MySQLdb/cursors.py", line 206, in execute res = self._query(query) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/MySQLdb/cursors.py", line 320, in _query self._do_get_result(db) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/MySQLdb/cursors.py", line 145, in _do_get_result self._result = result = self._get_result() File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/MySQLdb/cursors.py", line 352, in _get_result return self._get_db().store_result() django.db.utils.IntegrityError: (1062, "Duplicate entry '1' for key 'PRIMARY'") The above exception was the direct cause of the following exception: Traceback (most recent call last): File "/home/alex/workspace/test_TransactionManagementError/mysite/polls/tests.py", line 47, in test_update_or_create self.TestModel.objects.update_or_create(id=1, defaults={'field': 2}) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/manager.py", line 87, in manager_method return getattr(self.get_queryset(), name)(*args, **kwargs) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 949, in update_or_create obj, created = self.select_for_update().get_or_create(defaults, **kwargs) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 926, in get_or_create return self.get(**kwargs), False File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 633, in get num = len(clone) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 380, in __len__ self._fetch_all() File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 1881, in _fetch_all self._result_cache = list(self._iterable_class(self)) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 91, in __iter__ results = compiler.execute_sql( File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/models/sql/compiler.py", line 1560, in execute_sql cursor.execute(sql, params) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/backends/utils.py", line 67, in execute return self._execute_with_wrappers( File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/backends/utils.py", line 80, in _execute_with_wrappers return executor(sql, params, many, context) File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/backends/utils.py", line 83, in _execute self.db.validate_no_broken_transaction() File "/home/alex/workspace/test_TransactionManagementError/.venv/lib/python3.9/site-packages/django/db/backends/base/base.py", line 531, in validate_no_broken_transaction raise TransactionManagementError( django.db.transaction.TransactionManagementError: An error occurred in the current transaction. You can't execute queries until the end of the 'atomic' block.
comment:8 by , 19 months ago
Backends that don't support transactions is not a particularly well tested area of the code base. Do we even run the full test suite against MyISAM?
I'm curious as to whether you run into the same issue by making your TransactionManagementErrorTest
testcase extend TransationTestCase
instead of TestCase
. I would expect transaction.atomic
to be a noop when the backend doesn't support transactions so I wouldn't be surprised if there was an issue with nested atomic
blocks on such backends as they also don't support savepoints by definition (Remember that TestCase
wraps each tests in an outer atomic
block).
I don't see why a TransactionManagementError
is raised in the first place when atomic
is meant to be a noop as there are no transactions to manage.
comment:9 by , 19 months ago
Ok so I think I get what OP is reporting here, but when I run the above test on latest main, it works fine.
- The above test creates an InnoDB table; this causes the test with the duplicate
create()
calls in the manager to abort the transaction because we can't have 2 rows with pk=1 - After editing the test to create a MyISAM the test starts passing - I see no issue.
I'd ask if OP can please make sure the test is run against latest main.
comment:10 by , 19 months ago
Backends that don't support transactions is not a particularly well tested area of the code base. Do we even run the full test suite against MyISAM?
I'm doing this from time to time (e.g. 73766c118781a7f7052bf0a5fbee38b944964e31, 8e89dfe1c24540d33b577377af633694ff57f505, or 331a460f8f2e4f447b68fba491464b68c9b21fd1.)
comment:11 by , 19 months ago
Cc: | added |
---|---|
Description: | modified (diff) |
Resolution: | needsinfo |
Status: | closed → new |
Summary: | update_or_create not work in parallel insertion → Model.objects.update_or_create method sometimes raises TransactionManagementError |
comment:12 by , 19 months ago
Bisected to the 331a460f8f2e4f447b68fba491464b68c9b21fd1, however the MyISAM
storage engine doesn't support transactions so I'm not sure how it worked before.
follow-up: 14 comment:13 by , 19 months ago
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
. Remember that MySQL does support transactions and will happily create valid savepoints that can be rolled back and discarded independently of the default storage engine used.
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.
-
django/db/transaction.py
diff --git a/django/db/transaction.py b/django/db/transaction.py index 4150cbcbbe..28229f8f3a 100644
a b def __init__(self, using, savepoint, durable): 181 181 182 182 def __enter__(self): 183 183 connection = get_connection(self.using) 184 if not connection.features.supports_transactions: 185 return 184 186 185 187 if ( 186 188 self.durable … … def __enter__(self): 223 225 224 226 def __exit__(self, exc_type, exc_value, traceback): 225 227 connection = get_connection(self.using) 228 if not connection.features.supports_transactions: 229 return 226 230 227 231 if connection.in_atomic_block: 228 232 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.
comment:14 by , 19 months ago
Replying to 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)
comment:15 by , 19 months ago
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.
comment:16 by , 19 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Triage Stage: | Unreviewed → Accepted |
comment:18 by , 19 months ago
Cc: | added |
---|
What kind of exception are you getting?