Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #9479 -- Corrected an edge case in bulk queryset deletion that …

…could cause an infinite loop when using MySQL InnoDB.

git-svn-id: http://code.djangoproject.com/svn/django/branches/soc2009/http-wsgi-improvements@10994 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit cb847aa2249d10b14ec53e419f4610e4e7d08811 1 parent ae2dd58
Christopher Cahoon authored June 13, 2009
3  django/db/models/query.py
@@ -355,10 +355,11 @@ def delete(self):
355 355
 
356 356
         # Delete objects in chunks to prevent the list of related objects from
357 357
         # becoming too long.
  358
+        seen_objs = None
358 359
         while 1:
359 360
             # Collect all the objects to be deleted in this chunk, and all the
360 361
             # objects that are related to the objects that are to be deleted.
361  
-            seen_objs = CollectedObjects()
  362
+            seen_objs = CollectedObjects(seen_objs)
362 363
             for object in del_query[:CHUNK_SIZE]:
363 364
                 object._collect_sub_objects(seen_objs)
364 365
 
15  django/db/models/query_utils.py
@@ -32,11 +32,21 @@ class CollectedObjects(object):
32 32
 
33 33
     This is used for the database object deletion routines so that we can
34 34
     calculate the 'leaf' objects which should be deleted first.
  35
+
  36
+    previously_seen is an optional argument. It must be a CollectedObjects
  37
+    instance itself; any previously_seen collected object will be blocked from
  38
+    being added to this instance.
35 39
     """
36 40
 
37  
-    def __init__(self):
  41
+    def __init__(self, previously_seen=None):
38 42
         self.data = {}
39 43
         self.children = {}
  44
+        if previously_seen:
  45
+            self.blocked = previously_seen.blocked
  46
+            for cls, seen in previously_seen.data.items():
  47
+                self.blocked.setdefault(cls, SortedDict()).update(seen)
  48
+        else:
  49
+            self.blocked = {}
40 50
 
41 51
     def add(self, model, pk, obj, parent_model, nullable=False):
42 52
         """
@@ -53,6 +63,9 @@ def add(self, model, pk, obj, parent_model, nullable=False):
53 63
         Returns True if the item already existed in the structure and
54 64
         False otherwise.
55 65
         """
  66
+        if pk in self.blocked.get(model, {}):
  67
+            return True
  68
+
56 69
         d = self.data.setdefault(model, SortedDict())
57 70
         retval = pk in d
58 71
         d[pk] = obj
1  tests/regressiontests/delete_regress/__init__.py
... ...
@@ -0,0 +1 @@
  1
+
61  tests/regressiontests/delete_regress/models.py
... ...
@@ -0,0 +1,61 @@
  1
+from django.conf import settings
  2
+from django.db import models, backend, connection, transaction
  3
+from django.db.models import sql, query
  4
+from django.test import TransactionTestCase
  5
+
  6
+class Book(models.Model):
  7
+    pagecount = models.IntegerField()
  8
+
  9
+# Can't run this test under SQLite, because you can't
  10
+# get two connections to an in-memory database.
  11
+if settings.DATABASE_ENGINE != 'sqlite3':
  12
+    class DeleteLockingTest(TransactionTestCase):
  13
+        def setUp(self):
  14
+            # Create a second connection to the database
  15
+            self.conn2 = backend.DatabaseWrapper({
  16
+                'DATABASE_HOST': settings.DATABASE_HOST,
  17
+                'DATABASE_NAME': settings.DATABASE_NAME,
  18
+                'DATABASE_OPTIONS': settings.DATABASE_OPTIONS,
  19
+                'DATABASE_PASSWORD': settings.DATABASE_PASSWORD,
  20
+                'DATABASE_PORT': settings.DATABASE_PORT,
  21
+                'DATABASE_USER': settings.DATABASE_USER,
  22
+                'TIME_ZONE': settings.TIME_ZONE,
  23
+            })
  24
+
  25
+            # Put both DB connections into managed transaction mode
  26
+            transaction.enter_transaction_management()
  27
+            transaction.managed(True)
  28
+            self.conn2._enter_transaction_management(True)
  29
+
  30
+        def tearDown(self):
  31
+            # Close down the second connection.
  32
+            transaction.leave_transaction_management()
  33
+            self.conn2.close()
  34
+
  35
+        def test_concurrent_delete(self):
  36
+            "Deletes on concurrent transactions don't collide and lock the database. Regression for #9479"
  37
+
  38
+            # Create some dummy data
  39
+            b1 = Book(id=1, pagecount=100)
  40
+            b2 = Book(id=2, pagecount=200)
  41
+            b3 = Book(id=3, pagecount=300)
  42
+            b1.save()
  43
+            b2.save()
  44
+            b3.save()
  45
+
  46
+            transaction.commit()
  47
+
  48
+            self.assertEquals(3, Book.objects.count())
  49
+
  50
+            # Delete something using connection 2.
  51
+            cursor2 = self.conn2.cursor()
  52
+            cursor2.execute('DELETE from delete_regress_book WHERE id=1')
  53
+            self.conn2._commit();
  54
+
  55
+            # Now perform a queryset delete that covers the object
  56
+            # deleted in connection 2. This causes an infinite loop
  57
+            # under MySQL InnoDB unless we keep track of already
  58
+            # deleted objects.
  59
+            Book.objects.filter(pagecount__lt=250).delete()
  60
+            transaction.commit()
  61
+            self.assertEquals(1, Book.objects.count())

0 notes on commit cb847aa

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