Răsfoiți Sursa

Speed up bulk-delete objects

Arthur 1 săptămână în urmă
părinte
comite
b86843aaad

+ 3 - 1
netbox/netbox/models/deletion.py

@@ -79,7 +79,9 @@ class DeleteMixin:
                 )
             )
 
-        collector = CustomCollector(using=using)
+        # Pass origin=self (matching Django's Model.delete) so signal receivers can tell that
+        # cascaded child objects are being deleted as part of deleting this object.
+        collector = CustomCollector(using=using, origin=self)
         collector.collect([self], keep_parents=keep_parents)
 
         return collector.delete()

+ 38 - 6
netbox/utilities/counters.py

@@ -1,5 +1,5 @@
 from django.apps import apps
-from django.db.models import Count, F, OuterRef, Subquery
+from django.db.models import Count, F, OuterRef, QuerySet, Subquery
 from django.db.models.signals import post_delete, post_save, pre_delete
 
 from netbox.registry import registry
@@ -63,22 +63,54 @@ def post_save_receiver(sender, instance, created, **kwargs):
             update_counter(parent_model, new_pk, counter_name, 1)
 
 
+def _parent_is_being_deleted(origin, parent_model, parent_pk):
+    """
+    Return True if `origin` (the object or queryset that `delete()` was called on) indicates that
+    the parent identified by (parent_model, parent_pk) is itself being deleted as part of the same
+    operation. In that case, decrementing its counter is wasted work: the parent row is going away,
+    so the UPDATE would be a no-op. Skipping it avoids an N+1 storm of pointless UPDATEs when a
+    parent with many tracked children is deleted (e.g. a Device with thousands of Interfaces).
+    """
+    if origin is None:
+        return False
+    if isinstance(origin, QuerySet):
+        # A bulk delete; every collected child belongs to an object in this queryset by construction
+        return origin.model is parent_model
+    # A single object delete
+    return isinstance(origin, parent_model) and origin.pk == parent_pk
+
+
 def pre_delete_receiver(sender, instance, origin, **kwargs):
-    model = instance._meta.model
-    if not model.objects.filter(pk=instance.pk).exists():
-        instance._previously_removed = True
+    """
+    Before a tracked object is deleted, check whether its row has already been removed (e.g. by an
+    earlier cascade) and, if so, flag it so post_delete_receiver skips the now-redundant counter
+    update. The existence check is skipped when the tracked parent is itself being deleted, since
+    the counter update would be skipped regardless — this avoids a SELECT per cascaded child.
+    """
+    for field_name, counter_name in get_counters_for_model(sender):
+        parent_model = sender._meta.get_field(field_name).related_model
+        parent_pk = getattr(instance, field_name, None)
+        if parent_pk is None or _parent_is_being_deleted(origin, parent_model, parent_pk):
+            continue
+        # A tracked parent will survive this operation, so the double-delete guard is needed
+        if not sender.objects.filter(pk=instance.pk).exists():
+            instance._previously_removed = True
+        return
 
 
 def post_delete_receiver(sender, instance, origin, **kwargs):
     """
     Update counter fields on related objects when a TrackingModelMixin subclass is deleted.
     """
+    if hasattr(instance, '_previously_removed'):
+        return
+
     for field_name, counter_name in get_counters_for_model(sender):
         parent_model = sender._meta.get_field(field_name).related_model
         parent_pk = getattr(instance, field_name, None)
 
-        # Decrement the parent's counter by one
-        if parent_pk is not None and not hasattr(instance, '_previously_removed'):
+        # Decrement the parent's counter by one, unless the parent is itself being deleted
+        if parent_pk is not None and not _parent_is_being_deleted(origin, parent_model, parent_pk):
             update_counter(parent_model, parent_pk, counter_name, -1)
 
 

+ 29 - 0
netbox/utilities/tests/test_counters.py

@@ -1,3 +1,5 @@
+from django.db import connection
+from django.test.utils import CaptureQueriesContext
 from django.urls import reverse
 
 from dcim.models import *
@@ -64,6 +66,33 @@ class CountersTestCase(TestCase):
         self.assertEqual(device1.interface_count, 1)
         self.assertEqual(device2.interface_count, 1)
 
+    def test_counter_skipped_when_parent_deleted(self):
+        """
+        Deleting a parent object should not issue a counter update for each cascaded child on that
+        same parent (the row is being removed, so the UPDATE is a no-op). Counters on surviving
+        related objects must still be updated.
+        """
+        device1 = Device.objects.get(name='Device 1')
+        device_type = device1.device_type
+        self.assertEqual(device_type.device_count, 2)
+
+        with CaptureQueriesContext(connection) as ctx:
+            device1.delete()
+
+        # No counter UPDATE should target the interface counter of the Device being deleted
+        interface_counter_updates = [
+            q['sql'] for q in ctx.captured_queries
+            if q['sql'].upper().startswith('UPDATE') and '_interface_count' in q['sql']
+        ]
+        self.assertEqual(
+            interface_counter_updates, [],
+            "Deleting a Device must not update its own interface counter for each cascaded Interface"
+        )
+
+        # The counter on the surviving parent (DeviceType) must still be decremented
+        device_type.refresh_from_db()
+        self.assertEqual(device_type.device_count, 1)
+
     def test_interface_count_move(self):
         """
         When a tracked object (Interface) is moved, the tracking counter should be updated.