Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Made atomic usable when autocommit is off.

Thanks Anssi for haggling until I implemented this.

This change alleviates the need for atomic_if_autocommit. When
autocommit is disabled for a database, atomic will simply create and
release savepoints, and not commit anything. This honors the contract of
not doing any transaction management.

This change also makes the hack to allow using atomic within the legacy
transaction management redundant.

None of the above will work with SQLite, because of a flaw in the design
of the sqlite3 library. This is a known limitation that cannot be lifted
without unacceptable side effects eg. triggering arbitrary commits.
  • Loading branch information...
commit 83a416f5e764705ed553f3513bdf9587270554c6 1 parent bd0cba5
Aymeric Augustin authored March 13, 2013
2  django/core/cache/backends/db.py
@@ -109,7 +109,7 @@ def _base_set(self, mode, key, value, timeout=None):
109 109
         if six.PY3:
110 110
             b64encoded = b64encoded.decode('latin1')
111 111
         try:
112  
-            with transaction.atomic_if_autocommit(using=db):
  112
+            with transaction.atomic(using=db):
113 113
                 cursor.execute("SELECT cache_key, expires FROM %s "
114 114
                                "WHERE cache_key = %%s" % table, [key])
115 115
                 result = cursor.fetchone()
7  django/db/backends/__init__.py
@@ -50,15 +50,16 @@ def __init__(self, settings_dict, alias=DEFAULT_DB_ALIAS,
50 50
         # set somewhat aggressively, as the DBAPI doesn't make it easy to
51 51
         # deduce if the connection is in transaction or not.
52 52
         self._dirty = False
53  
-        # Tracks if the connection is in a transaction managed by 'atomic'
  53
+        # Tracks if the connection is in a transaction managed by 'atomic'.
54 54
         self.in_atomic_block = False
  55
+        # Tracks if the outermost 'atomic' block should commit on exit,
  56
+        # ie. if autocommit was active on entry.
  57
+        self.commit_on_exit = True
55 58
         # Tracks if the transaction should be rolled back to the next
56 59
         # available savepoint because of an exception in an inner block.
57 60
         self.needs_rollback = False
58 61
         # List of savepoints created by 'atomic'
59 62
         self.savepoint_ids = []
60  
-        # Hack to provide compatibility with legacy transaction management
61  
-        self._atomic_forced_unmanaged = False
62 63
 
63 64
         # Connection termination related attributes
64 65
         self.close_at = None
155  django/db/transaction.py
@@ -206,18 +206,6 @@ def __init__(self, using, savepoint):
206 206
         self.using = using
207 207
         self.savepoint = savepoint
208 208
 
209  
-    def _legacy_enter_transaction_management(self, connection):
210  
-        if not connection.in_atomic_block:
211  
-            if connection.transaction_state and connection.transaction_state[-1]:
212  
-                connection._atomic_forced_unmanaged = True
213  
-                connection.enter_transaction_management(managed=False)
214  
-            else:
215  
-                connection._atomic_forced_unmanaged = False
216  
-
217  
-    def _legacy_leave_transaction_management(self, connection):
218  
-        if not connection.in_atomic_block and connection._atomic_forced_unmanaged:
219  
-            connection.leave_transaction_management()
220  
-
221 209
     def __enter__(self):
222 210
         connection = get_connection(self.using)
223 211
 
@@ -225,12 +213,31 @@ def __enter__(self):
225 213
         # autocommit status.
226 214
         connection.ensure_connection()
227 215
 
228  
-        # Remove this when the legacy transaction management goes away.
229  
-        self._legacy_enter_transaction_management(connection)
230  
-
231  
-        if not connection.in_atomic_block and not connection.autocommit:
232  
-            raise TransactionManagementError(
233  
-                "'atomic' cannot be used when autocommit is disabled.")
  216
+        if not connection.in_atomic_block:
  217
+            # Reset state when entering an outermost atomic block.
  218
+            connection.commit_on_exit = True
  219
+            connection.needs_rollback = False
  220
+            if not connection.autocommit:
  221
+                # Some database adapters (namely sqlite3) don't handle
  222
+                # transactions and savepoints properly when autocommit is off.
  223
+                # Turning autocommit back on isn't an option; it would trigger
  224
+                # a premature commit. Give up if that happens.
  225
+                if connection.features.autocommits_when_autocommit_is_off:
  226
+                    raise TransactionManagementError(
  227
+                        "Your database backend doesn't behave properly when "
  228
+                        "autocommit is off. Turn it on before using 'atomic'.")
  229
+                # When entering an atomic block with autocommit turned off,
  230
+                # Django should only use savepoints and shouldn't commit.
  231
+                # This requires at least a savepoint for the outermost block.
  232
+                if not self.savepoint:
  233
+                    raise TransactionManagementError(
  234
+                        "The outermost 'atomic' block cannot use "
  235
+                        "savepoint = False when autocommit is off.")
  236
+                # Pretend we're already in an atomic block to bypass the code
  237
+                # that disables autocommit to enter a transaction, and make a
  238
+                # note to deal with this case in __exit__.
  239
+                connection.in_atomic_block = True
  240
+                connection.commit_on_exit = False
234 241
 
235 242
         if connection.in_atomic_block:
236 243
             # We're already in a transaction; create a savepoint, unless we
@@ -255,63 +262,58 @@ def __enter__(self):
255 262
             else:
256 263
                 connection.set_autocommit(False)
257 264
             connection.in_atomic_block = True
258  
-            connection.needs_rollback = False
259 265
 
260 266
     def __exit__(self, exc_type, exc_value, traceback):
261 267
         connection = get_connection(self.using)
262  
-        if exc_value is None and not connection.needs_rollback:
263  
-            if connection.savepoint_ids:
264  
-                # Release savepoint if there is one
265  
-                sid = connection.savepoint_ids.pop()
266  
-                if sid is not None:
  268
+
  269
+        if connection.savepoint_ids:
  270
+            sid = connection.savepoint_ids.pop()
  271
+        else:
  272
+            # Prematurely unset this flag to allow using commit or rollback.
  273
+            connection.in_atomic_block = False
  274
+
  275
+        try:
  276
+            if exc_value is None and not connection.needs_rollback:
  277
+                if connection.in_atomic_block:
  278
+                    # Release savepoint if there is one
  279
+                    if sid is not None:
  280
+                        try:
  281
+                            connection.savepoint_commit(sid)
  282
+                        except DatabaseError:
  283
+                            connection.savepoint_rollback(sid)
  284
+                            raise
  285
+                else:
  286
+                    # Commit transaction
267 287
                     try:
268  
-                        connection.savepoint_commit(sid)
  288
+                        connection.commit()
269 289
                     except DatabaseError:
270  
-                        connection.savepoint_rollback(sid)
271  
-                        # Remove this when the legacy transaction management goes away.
272  
-                        self._legacy_leave_transaction_management(connection)
  290
+                        connection.rollback()
273 291
                         raise
274 292
             else:
275  
-                # Commit transaction
276  
-                connection.in_atomic_block = False
277  
-                try:
278  
-                    connection.commit()
279  
-                except DatabaseError:
280  
-                    connection.rollback()
281  
-                    # Remove this when the legacy transaction management goes away.
282  
-                    self._legacy_leave_transaction_management(connection)
283  
-                    raise
284  
-                finally:
285  
-                    if connection.features.autocommits_when_autocommit_is_off:
286  
-                        connection.autocommit = True
  293
+                # This flag will be set to True again if there isn't a savepoint
  294
+                # allowing to perform the rollback at this level.
  295
+                connection.needs_rollback = False
  296
+                if connection.in_atomic_block:
  297
+                    # Roll back to savepoint if there is one, mark for rollback
  298
+                    # otherwise.
  299
+                    if sid is None:
  300
+                        connection.needs_rollback = True
287 301
                     else:
288  
-                        connection.set_autocommit(True)
289  
-        else:
290  
-            # This flag will be set to True again if there isn't a savepoint
291  
-            # allowing to perform the rollback at this level.
292  
-            connection.needs_rollback = False
293  
-            if connection.savepoint_ids:
294  
-                # Roll back to savepoint if there is one, mark for rollback
295  
-                # otherwise.
296  
-                sid = connection.savepoint_ids.pop()
297  
-                if sid is None:
298  
-                    connection.needs_rollback = True
  302
+                        connection.savepoint_rollback(sid)
299 303
                 else:
300  
-                    connection.savepoint_rollback(sid)
301  
-            else:
302  
-                # Roll back transaction
303  
-                connection.in_atomic_block = False
304  
-                try:
  304
+                    # Roll back transaction
305 305
                     connection.rollback()
306  
-                finally:
307  
-                    if connection.features.autocommits_when_autocommit_is_off:
308  
-                        connection.autocommit = True
309  
-                    else:
310  
-                        connection.set_autocommit(True)
311  
-
312  
-        # Remove this when the legacy transaction management goes away.
313  
-        self._legacy_leave_transaction_management(connection)
314 306
 
  307
+        finally:
  308
+            # Outermost block exit when autocommit was enabled.
  309
+            if not connection.in_atomic_block:
  310
+                if connection.features.autocommits_when_autocommit_is_off:
  311
+                    connection.autocommit = True
  312
+                else:
  313
+                    connection.set_autocommit(True)
  314
+            # Outermost block exit when autocommit was disabled.
  315
+            elif not connection.savepoint_ids and not connection.commit_on_exit:
  316
+                connection.in_atomic_block = False
315 317
 
316 318
     def __call__(self, func):
317 319
         @wraps(func, assigned=available_attrs(func))
@@ -331,24 +333,6 @@ def atomic(using=None, savepoint=True):
331 333
         return Atomic(using, savepoint)
332 334
 
333 335
 
334  
-def atomic_if_autocommit(using=None, savepoint=True):
335  
-    # This variant only exists to support the ability to disable transaction
336  
-    # management entirely in the DATABASES setting. It doesn't care about the
337  
-    # autocommit state at run time.
338  
-    db = DEFAULT_DB_ALIAS if callable(using) else using
339  
-    autocommit = get_connection(db).settings_dict['AUTOCOMMIT']
340  
-
341  
-    if autocommit:
342  
-        return atomic(using, savepoint)
343  
-    else:
344  
-        # Bare decorator: @atomic_if_autocommit
345  
-        if callable(using):
346  
-            return using
347  
-        # Decorator: @atomic_if_autocommit(...)
348  
-        else:
349  
-            return lambda func: func
350  
-
351  
-
352 336
 ############################################
353 337
 # Deprecated decorators / context managers #
354 338
 ############################################
@@ -472,16 +456,15 @@ def commit_on_success_unless_managed(using=None, savepoint=False):
472 456
     Transitory API to preserve backwards-compatibility while refactoring.
473 457
 
474 458
     Once the legacy transaction management is fully deprecated, this should
475  
-    simply be replaced by atomic_if_autocommit. Until then, it's necessary to
476  
-    avoid making a commit where Django didn't use to, since entering atomic in
477  
-    managed mode triggers a commmit.
  459
+    simply be replaced by atomic. Until then, it's necessary to guarantee that
  460
+    a commit occurs on exit, which atomic doesn't do when it's nested.
478 461
 
479 462
     Unlike atomic, savepoint defaults to False because that's closer to the
480 463
     legacy behavior.
481 464
     """
482 465
     connection = get_connection(using)
483 466
     if connection.autocommit or connection.in_atomic_block:
484  
-        return atomic_if_autocommit(using, savepoint)
  467
+        return atomic(using, savepoint)
485 468
     else:
486 469
         def entering(using):
487 470
             pass
26  docs/topics/db/transactions.txt
@@ -153,9 +153,6 @@ Django provides a single API to control database transactions.
153 153
     to commit, roll back, or change the autocommit state of the database
154 154
     connection within an ``atomic`` block will raise an exception.
155 155
 
156  
-    ``atomic`` can only be used in autocommit mode. It will raise an exception
157  
-    if autocommit is turned off.
158  
-
159 156
     Under the hood, Django's transaction management code:
160 157
 
161 158
     - opens a transaction when entering the outermost ``atomic`` block;
@@ -171,6 +168,10 @@ Django provides a single API to control database transactions.
171 168
     the overhead of savepoints is noticeable. It has the drawback of breaking
172 169
     the error handling described above.
173 170
 
  171
+    You may use ``atomic`` when autocommit is turned off. It will only use
  172
+    savepoints, even for the outermost block, and it will raise an exception
  173
+    if the outermost block is declared with ``savepoint=False``.
  174
+
174 175
 .. admonition:: Performance considerations
175 176
 
176 177
     Open transactions have a performance cost for your database server. To
@@ -271,9 +272,8 @@ another. Review the documentation of the adapter you're using carefully.
271 272
 You must ensure that no transaction is active, usually by issuing a
272 273
 :func:`commit` or a :func:`rollback`, before turning autocommit back on.
273 274
 
274  
-:func:`atomic` requires autocommit to be turned on; it will raise an exception
275  
-if autocommit is off. Django will also refuse to turn autocommit off when an
276  
-:func:`atomic` block is active, because that would break atomicity.
  275
+Django will refuse to turn autocommit off when an :func:`atomic` block is
  276
+active, because that would break atomicity.
277 277
 
278 278
 Transactions
279 279
 ------------
@@ -392,8 +392,11 @@ When autocommit is enabled, savepoints don't make sense. When it's disabled,
392 392
 commits before any statement other than ``SELECT``, ``INSERT``, ``UPDATE``,
393 393
 ``DELETE`` and ``REPLACE``.)
394 394
 
395  
-As a consequence, savepoints are only usable inside a transaction ie. inside
396  
-an :func:`atomic` block.
  395
+This has two consequences:
  396
+
  397
+- The low level APIs for savepoints are only usable inside a transaction ie.
  398
+  inside an :func:`atomic` block.
  399
+- It's impossible to use :func:`atomic` when autocommit is turned off.
397 400
 
398 401
 Transactions in MySQL
399 402
 ---------------------
@@ -584,9 +587,6 @@ During the deprecation period, it's possible to use :func:`atomic` within
584 587
 However, the reverse is forbidden, because nesting the old decorators /
585 588
 context managers breaks atomicity.
586 589
 
587  
-If you enter :func:`atomic` while you're in managed mode, it will trigger a
588  
-commit to start from a clean slate.
589  
-
590 590
 Managing autocommit
591 591
 ~~~~~~~~~~~~~~~~~~~
592 592
 
@@ -623,8 +623,8 @@ Disabling transaction management
623 623
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
624 624
 
625 625
 Instead of setting ``TRANSACTIONS_MANAGED = True``, set the ``AUTOCOMMIT`` key
626  
-to ``False`` in the configuration of each database, as explained in :ref
627  
-:`deactivate-transaction-management`.
  626
+to ``False`` in the configuration of each database, as explained in
  627
+:ref:`deactivate-transaction-management`.
628 628
 
629 629
 Backwards incompatibilities
630 630
 ---------------------------
30  tests/transactions/tests.py
@@ -6,7 +6,7 @@
6 6
 from django.db import connection, transaction, IntegrityError
7 7
 from django.test import TransactionTestCase, skipUnlessDBFeature
8 8
 from django.utils import six
9  
-from django.utils.unittest import skipUnless
  9
+from django.utils.unittest import skipIf, skipUnless
10 10
 
11 11
 from .models import Reporter
12 12
 
@@ -197,6 +197,23 @@ def tearDown(self):
197 197
         self.atomic.__exit__(*sys.exc_info())
198 198
 
199 199
 
  200
+@skipIf(connection.features.autocommits_when_autocommit_is_off,
  201
+        "This test requires a non-autocommit mode that doesn't autocommit.")
  202
+class AtomicWithoutAutocommitTests(AtomicTests):
  203
+    """All basic tests for atomic should also pass when autocommit is turned off."""
  204
+
  205
+    def setUp(self):
  206
+        transaction.set_autocommit(False)
  207
+
  208
+    def tearDown(self):
  209
+        # The tests access the database after exercising 'atomic', initiating
  210
+        # a transaction ; a rollback is required before restoring autocommit.
  211
+        transaction.rollback()
  212
+        transaction.set_autocommit(True)
  213
+
  214
+
  215
+@skipIf(connection.features.autocommits_when_autocommit_is_off,
  216
+        "This test requires a non-autocommit mode that doesn't autocommit.")
200 217
 class AtomicInsideLegacyTransactionManagementTests(AtomicTests):
201 218
 
202 219
     def setUp(self):
@@ -268,16 +285,7 @@ def test_merged_outer_rollback_after_inner_failure_and_inner_success(self):
268 285
         "'atomic' requires transactions and savepoints.")
269 286
 class AtomicErrorsTests(TransactionTestCase):
270 287
 
271  
-    def test_atomic_requires_autocommit(self):
272  
-        transaction.set_autocommit(False)
273  
-        try:
274  
-            with self.assertRaises(transaction.TransactionManagementError):
275  
-                with transaction.atomic():
276  
-                    pass
277  
-        finally:
278  
-            transaction.set_autocommit(True)
279  
-
280  
-    def test_atomic_prevents_disabling_autocommit(self):
  288
+    def test_atomic_prevents_setting_autocommit(self):
281 289
         autocommit = transaction.get_autocommit()
282 290
         with transaction.atomic():
283 291
             with self.assertRaises(transaction.TransactionManagementError):

0 notes on commit 83a416f

Please sign in to comment.
Something went wrong with that request. Please try again.