Repository: nautobot/nautobot
Issue category: Performance Issue

Resources that are "too large" cause the nautobot process to crash
### Environment
* Nautobot version (Docker tag too if applicable): 2.3.4 (Docker tag 'latest')
* Python version: 3.10.15
* Database platform, version: PostgreSQL 15.1
* Middleware(s): None

### Steps to Reproduce
1. Create 200k devices
2. Create a location
3. Create a new Dynamic Group -> dcim | device -> filter-defined
4. Edit the new Dynamic Group -> Add Location -> Save
5. Watch nautobot spin for 10 minutes before returning 502.

### Expected Behavior
Dynamic group gets updated to have the new location, and it takes far less than 10 minutes.

### Observed Behavior
Nautobot crashes. This is from the nautobot-server logs:

```
Mon Sep 30 19:39:09 2024 - *** HARAKIRI ON WORKER 14 (pid: 101, try: 1, graceful: yes) ***
Mon Sep 30 19:39:09 2024 - HARAKIRI !!! worker 14 status !!!
Mon Sep 30 19:39:09 2024 - HARAKIRI [core 0] 172.19.0.1 - POST /extras/dynamic-groups/61ba43ba-2eb7-4689-aeba-c37b978d2630/edit/ since 1727724549
Mon Sep 30 19:39:09 2024 - HARAKIRI !!! end of worker 14 status !!!
Mon Sep 30 19:39:09 2024 - HARAKIRI triggered by worker 14 core 0 !!!
2024/09/30 19:39:09 [error] 83#83: *1 upstream prematurely closed connection while reading response header from upstream, client: 172.19.0.1, server: , request: "POST /extras/dynamic-groups/61ba43ba-2eb7-4689-aeba-c37b978d2630/edit/ HTTP/1.1", upstream: "uwsgi://unix:/var/run/app.uwsgi:", host: "localhost:8081", referrer: "http://localhost:8081/extras/dynamic-groups/61ba43ba-2eb7-4689-aeba-c37b978d2630/edit/"
172.19.0.1 - - [30/Sep/2024:19:39:09 +0000] "POST /extras/dynamic-groups/61ba43ba-2eb7-4689-aeba-c37b978d2630/edit/ HTTP/1.1" 502 559 "http://localhost:8081/extras/dynamic-groups/61ba43ba-2eb7-4689-aeba-c37b978d2630/edit/" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/129.0.0.0 Safari/537.36"
DAMN ! worker 14 (pid: 101) died, killed by signal 9 :( trying respawn ...
Respawned uWSGI worker 14 (new pid: 248)
19:39:10.211 INFO    nautobot :
  Nautobot initialized!
WSGI app 0 (mountpoint='') ready in 0 seconds on interpreter 0xaaab2500e750 pid: 248 (default app)
```


### Additional notes:
I am working on upgrading an existing system from 1.6 to 2.3.4. I am experiencing this issue as part of the upgrade. While this is not a brand new installation, the only plugin enabled is an access control plugin. I have not truly attempted with a brand new installation because that "Create 200k devices" step, but I have attached a screenshot of what my dynamic group looks like before I attempt a modification:
![Screenshot 2024-09-27 at 11 37 39 AM](https://github.com/user-attachments/assets/d6efae47-1eb5-4d2a-ac9a-f8bcd3766885)

I CAN successfully modify existing dynamic groups, as long as their filters keep their associated device count below the magic threshold. I have not identified said threshold yet, but it does not immediately appear to be resource dependent. I originally encountered this error in an environment using 20GB memory and 2 cores, but it still fails in the same amount of time (about 10 minutes) with 30 GB memory and 8 cores.

I have noticed similar behavior when I attempt to open up the "IP Addresses" tab in one of the namespaces that has 100k+ IPs assigned to it, so this does not appear to be limited exclusively to Dynamic Groups.

This also happens when I try to perform the update through pynautobot, with some slightly different information in case it helps:
```
>>> dg = nautobot.extras.dynamic_groups.get(id="61ba43ba-2eb7-4689-aeba-c37b978d2630")
>>> dg.update({"filter": {"location": ["DC1"]}})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/kfischer/Workspace/stuff/scripts/nautobot_v2_migration/venv/lib/python3.12/site-packages/pynautobot/core/response.py", line 454, in update
    return self.save()
           ^^^^^^^^^^^
  File "/Users/kfischer/Workspace/stuff/scripts/nautobot_v2_migration/venv/lib/python3.12/site-packages/pynautobot/core/response.py", line 424, in save
    if req.patch({i: serialized[i] for i in diff}):
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/kfischer/Workspace/stuff/scripts/nautobot_v2_migration/venv/lib/python3.12/site-packages/pynautobot/core/query.py", line 428, in patch
    return self._make_call(verb="patch", data=data)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/kfischer/Workspace/stuff/scripts/nautobot_v2_migration/venv/lib/python3.12/site-packages/pynautobot/core/query.py", line 290, in _make_call
    raise RequestError(req)
pynautobot.core.query.RequestError: The request failed with code 502 Bad Gateway but more specific details were not returned in json. Check the Nautobot Logs or investigate this exception's error attribute.
```

And the associated nautobot server logs:
```
172.19.0.1 - - [30/Sep/2024:19:09:51 +0000] "PATCH /api/extras/dynamic-groups/61ba43ba-2eb7-4689-aeba-c37b978d2630/ HTTP/1.1" 400 38 "-" "python-requests/2.32.3"
19:09:57.465 INFO    nautobot.core.api.views.ModelViewSet :
  Updating dynamic group kfischer-test-dcim-device (PK: 61ba43ba-2eb7-4689-aeba-c37b978d2630)
Mon Sep 30 19:19:57 2024 - *** HARAKIRI ON WORKER 2 (pid: 89, try: 1, graceful: yes) ***
Mon Sep 30 19:19:57 2024 - HARAKIRI !!! worker 2 status !!!
Mon Sep 30 19:19:57 2024 - HARAKIRI [core 0] 172.19.0.1 - PATCH /api/extras/dynamic-groups/61ba43ba-2eb7-4689-aeba-c37b978d2630/ since 1727723397
Mon Sep 30 19:19:57 2024 - HARAKIRI !!! end of worker 2 status !!!
Mon Sep 30 19:19:57 2024 - HARAKIRI triggered by worker 2 core 0 !!!
2024/09/30 19:19:58 [error] 83#83: *24 upstream prematurely closed connection while reading response header from upstream, client: 172.19.0.1, server: , request: "PATCH /api/extras/dynamic-groups/61ba43ba-2eb7-4689-aeba-c37b978d2630/ HTTP/1.1", upstream: "uwsgi://unix:/var/run/app.uwsgi:", host: "localhost:8081"
172.19.0.1 - - [30/Sep/2024:19:19:58 +0000] "PATCH /api/extras/dynamic-groups/61ba43ba-2eb7-4689-aeba-c37b978d2630/ HTTP/1.1" 502 157 "-" "python-requests/2.32.3"
DAMN ! worker 2 (pid: 89) died, killed by signal 9 :( trying respawn ...
Respawned uWSGI worker 2 (new pid: 170)
19:19:59.171 INFO    nautobot :
  Nautobot initialized!
WSGI app 0 (mountpoint='') ready in 0 seconds on interpreter 0xaaaae1b46750 pid: 170 (default app)
```



Functions to edit:
nautobot/core/models/tree_queries.py:TreeModel
nautobot/core/signals.py:invalidate_max_depth_cache
nautobot/extras/models/groups.py:DynamicGroup._set_members
nautobot/extras/models/groups.py:DynamicGroup.add_members
nautobot/extras/models/groups.py:DynamicGroup._add_members
nautobot/extras/models/groups.py:DynamicGroup.remove_members
nautobot/extras/models/groups.py:DynamicGroup._remove_members
nautobot/extras/models/groups.py:DynamicGroup.update_cached_members
nautobot/extras/views.py:DynamicGroupView.get_extra_context
Functions added: nautobot/core/models/tree_queries.py:TreeModel.__init_subclass__

Patch:
diff --git a/changes/6297.fixed b/changes/6297.fixed
index f889dcbefda..e6e279aba67 100644
--- a/changes/6297.fixed
+++ b/changes/6297.fixed
@@ -1,2 +1,4 @@
 Fixed paginator widget to display the current selected `per_page` value even if it's not one of the `PER_PAGE_DEFAULTS` options.
 Added pagination of related-object tables to many IPAM views to avoid errors when very large quantities of related records are present.
+Fixed overly broad scope of the TreeModel `invalidate_max_depth_cache` signal so that it now correctly only fires for TreeModel instances rather than all models.
+Improved performance of DynamicGroup membership updates/recalculations when dealing with large numbers of member objects.
diff --git a/nautobot/core/models/tree_queries.py b/nautobot/core/models/tree_queries.py
index c3b029d2b4f..b8a0cf3d07e 100644
--- a/nautobot/core/models/tree_queries.py
+++ b/nautobot/core/models/tree_queries.py
@@ -1,9 +1,11 @@
 from django.core.cache import cache
 from django.db.models import Case, When
+from django.db.models.signals import post_delete, post_save
 from tree_queries.models import TreeNode
 from tree_queries.query import TreeManager as TreeManager_, TreeQuerySet as TreeQuerySet_
 
 from nautobot.core.models import BaseManager, querysets
+from nautobot.core.signals import invalidate_max_depth_cache
 
 
 class TreeQuerySet(TreeQuerySet_, querysets.RestrictedQuerySet):
@@ -117,3 +119,9 @@ def display(self):
         display_str += self.name
         cache.set(cache_key, display_str, 5)
         return display_str
+
+    @classmethod
+    def __init_subclass__(cls, **kwargs):
+        super().__init_subclass__(**kwargs)
+        post_save.connect(invalidate_max_depth_cache, sender=cls)
+        post_delete.connect(invalidate_max_depth_cache, sender=cls)
diff --git a/nautobot/core/signals.py b/nautobot/core/signals.py
index ed12ddc0a5b..4499ba9498a 100644
--- a/nautobot/core/signals.py
+++ b/nautobot/core/signals.py
@@ -7,7 +7,6 @@
 
 from django.contrib.auth.signals import user_logged_in, user_logged_out
 from django.core.cache import cache
-from django.db.models.signals import post_delete, post_save
 from django.dispatch import receiver, Signal
 import redis.exceptions
 
@@ -62,10 +61,12 @@ def wrapper(*args, **kwargs):
     return wrapper
 
 
-@receiver(post_save)
-@receiver(post_delete)
 def invalidate_max_depth_cache(sender, **kwargs):
-    """Clear the appropriate TreeManager.max_depth cache as the create/update/delete may have changed the tree."""
+    """
+    Clear the appropriate TreeManager.max_depth cache as the create/update/delete may have changed the tree.
+
+    Note that this signal is connected in `TreeModel.__init_subclass__()` so as to only apply to those models.
+    """
     from nautobot.core.models.tree_queries import TreeManager
 
     if not isinstance(sender.objects, TreeManager):
diff --git a/nautobot/extras/models/groups.py b/nautobot/extras/models/groups.py
index c7f81cd9996..9a15b605d25 100644
--- a/nautobot/extras/models/groups.py
+++ b/nautobot/extras/models/groups.py
@@ -8,6 +8,7 @@
 from django.core.exceptions import ValidationError
 from django.core.serializers.json import DjangoJSONEncoder
 from django.db import models
+from django.db.models.signals import pre_delete
 from django.utils.functional import cached_property
 import django_filters
 
@@ -319,24 +320,18 @@ def _set_members(self, value):
         if isinstance(value, models.QuerySet):
             if value.model != self.model:
                 raise TypeError(f"QuerySet does not contain {self.model._meta.label_lower} objects")
-            to_remove = self.members.exclude(pk__in=value.values_list("pk", flat=True))
+            to_remove = self.members.only("id").difference(value.only("id"))
             self._remove_members(to_remove)
-            to_add = value.exclude(pk__in=self.members.values_list("pk", flat=True))
+            to_add = value.only("id").difference(self.members.only("id"))
             self._add_members(to_add)
         else:
             for obj in value:
                 if not isinstance(obj, self.model):
                     raise TypeError(f"{obj} is not a {self.model._meta.label_lower}")
-            to_remove = []
-            for member in self.members:
-                if member not in value:
-                    to_remove.append(member)
+            existing_members = self.members
+            to_remove = [obj for obj in existing_members if obj not in value]
             self._remove_members(to_remove)
-            to_add = []
-            members = self.members
-            for candidate in value:
-                if candidate not in members:
-                    to_add.append(candidate)
+            to_add = [obj for obj in value if obj not in existing_members]
             self._add_members(to_add)
 
         return self.members
@@ -345,63 +340,81 @@ def add_members(self, objects_to_add):
         """Add the given list or QuerySet of objects to this staticly defined group."""
         if self.group_type != DynamicGroupTypeChoices.TYPE_STATIC:
             raise ValidationError(f"Group {self} is not staticly defined, adding members directly is not permitted.")
-        return self._add_members(objects_to_add)
-
-    def _add_members(self, objects_to_add):
-        """Internal API for adding the given list or QuerySet of objects to the cached/static members of this group."""
         if isinstance(objects_to_add, models.QuerySet):
             if objects_to_add.model != self.model:
                 raise TypeError(f"QuerySet does not contain {self.model._meta.label_lower} objects")
+            objects_to_add = objects_to_add.only("id").difference(self.members.only("id"))
         else:
             for obj in objects_to_add:
                 if not isinstance(obj, self.model):
                     raise TypeError(f"{obj} is not a {self.model._meta.label_lower}")
+            existing_members = self.members
+            objects_to_add = [obj for obj in objects_to_add if obj not in existing_members]
+        return self._add_members(objects_to_add)
 
+    def _add_members(self, objects_to_add):
+        """
+        Internal API for adding the given list or QuerySet of objects to the cached/static members of this group.
+
+        Assumes that objects_to_add has already been filtered to exclude any existing member objects.
+        """
         if self.group_type == DynamicGroupTypeChoices.TYPE_STATIC:
             for obj in objects_to_add:
                 # We don't use `.bulk_create()` currently because we want change logging for these creates.
                 # Might be a good future performance improvement though.
-                StaticGroupAssociation.all_objects.get_or_create(
+                StaticGroupAssociation.all_objects.create(
                     dynamic_group=self, associated_object_type=self.content_type, associated_object_id=obj.pk
                 )
         else:
             # Cached/hidden static group associations, so we can use bulk-create to bypass change logging.
-            existing_members = self.members
             sgas = [
                 StaticGroupAssociation(
                     dynamic_group=self, associated_object_type=self.content_type, associated_object_id=obj.pk
                 )
                 for obj in objects_to_add
-                if obj not in existing_members
             ]
-            StaticGroupAssociation.all_objects.bulk_create(sgas)
+            StaticGroupAssociation.all_objects.bulk_create(sgas, batch_size=1000)
 
     def remove_members(self, objects_to_remove):
         """Remove the given list or QuerySet of objects from this staticly defined group."""
         if self.group_type != DynamicGroupTypeChoices.TYPE_STATIC:
             raise ValidationError(f"Group {self} is not staticly defined, removing members directly is not permitted.")
-        return self._remove_members(objects_to_remove)
-
-    def _remove_members(self, objects_to_remove):
-        """Internal API for removing the given list or QuerySet from the cached/static members of this Group."""
         if isinstance(objects_to_remove, models.QuerySet):
             if objects_to_remove.model != self.model:
                 raise TypeError(f"QuerySet does not contain {self.model._meta.label_lower} objects")
-            StaticGroupAssociation.all_objects.filter(
-                dynamic_group=self,
-                associated_object_type=self.content_type,
-                associated_object_id__in=objects_to_remove.values_list("pk", flat=True),
-            ).delete()
         else:
-            pks_to_remove = set()
             for obj in objects_to_remove:
                 if not isinstance(obj, self.model):
                     raise TypeError(f"{obj} is not a {self.model._meta.label_lower}")
-                pks_to_remove.add(obj.pk)
+        return self._remove_members(objects_to_remove)
 
-            StaticGroupAssociation.all_objects.filter(
-                dynamic_group=self, associated_object_type=self.content_type, associated_object_id__in=pks_to_remove
-            ).delete()
+    def _remove_members(self, objects_to_remove):
+        """Internal API for removing the given list or QuerySet from the cached/static members of this Group."""
+        from nautobot.extras.signals import _handle_deleted_object  # avoid circular import
+
+        # For non-static groups, we aren't going to change log the StaticGroupAssociation deletes anyway,
+        # so save some performance on signals -- important especially when we're dealing with thousands of records
+        if self.group_type != DynamicGroupTypeChoices.TYPE_STATIC:
+            logger.debug("Temporarily disconnecting the _handle_deleted_object signal for performance")
+            pre_delete.disconnect(_handle_deleted_object)
+        try:
+            if isinstance(objects_to_remove, models.QuerySet):
+                StaticGroupAssociation.all_objects.filter(
+                    dynamic_group=self,
+                    associated_object_type=self.content_type,
+                    associated_object_id__in=objects_to_remove.values_list("id", flat=True),
+                ).delete()
+            else:
+                pks_to_remove = [obj.id for obj in objects_to_remove]
+                StaticGroupAssociation.all_objects.filter(
+                    dynamic_group=self,
+                    associated_object_type=self.content_type,
+                    associated_object_id__in=pks_to_remove,
+                ).delete()
+        finally:
+            if self.group_type != DynamicGroupTypeChoices.TYPE_STATIC:
+                logger.debug("Re-connecting the _handle_deleted_object signal")
+                pre_delete.connect(_handle_deleted_object)
 
     @property
     @method_deprecated("Members are now cached in the database via StaticGroupAssociations rather than in Redis.")
@@ -430,6 +443,7 @@ def update_cached_members(self, members=None):
             else:
                 raise RuntimeError(f"Unknown/invalid group_type {self.group_type}")
 
+        logger.debug("Refreshing members cache for %s", self)
         self._set_members(members)
         logger.debug("Refreshed cache for %s, now with %d members", self, self.count)
 
diff --git a/nautobot/extras/views.py b/nautobot/extras/views.py
index d95839aefd2..9d4c0cc2aff 100644
--- a/nautobot/extras/views.py
+++ b/nautobot/extras/views.py
@@ -737,7 +737,9 @@ def get_extra_context(self, request, instance):
 
         if table_class is not None:
             # Members table (for display on Members nav tab)
-            members_table = table_class(members.restrict(request.user, "view"), orderable=False)
+            members_table = table_class(
+                members.restrict(request.user, "view"), orderable=False, exclude=["dynamic_group_count"]
+            )
             paginate = {
                 "paginator_class": EnhancedPaginator,
                 "per_page": get_paginate_count(request),

Test patch: diff --git a/nautobot/extras/tests/test_dynamicgroups.py b/nautobot/extras/tests/test_dynamicgroups.py
index ccf7993186b..c2cd6974f77 100644
--- a/nautobot/extras/tests/test_dynamicgroups.py
+++ b/nautobot/extras/tests/test_dynamicgroups.py
@@ -304,6 +304,10 @@ def test_static_member_operations(self):
         sg.add_members(Prefix.objects.filter(ip_version=4))
         self.assertIsInstance(sg.members, PrefixQuerySet)
         self.assertQuerysetEqualAndNotEmpty(sg.members, Prefix.objects.filter(ip_version=4))
+        # test cumulative construction and alternate code path
+        sg.add_members(list(Prefix.objects.filter(ip_version=6)))
+        self.assertQuerysetEqualAndNotEmpty(sg.members, Prefix.objects.all())
+        self.assertEqual(sg.static_group_associations.count(), Prefix.objects.all().count())
         # test duplicate objects aren't re-added
         sg.add_members(Prefix.objects.all())
         self.assertQuerysetEqualAndNotEmpty(sg.members, Prefix.objects.all())
@@ -321,10 +325,18 @@ def test_static_member_operations(self):
         sg.remove_members(list(Prefix.objects.filter(ip_version=4)))
         self.assertQuerysetEqualAndNotEmpty(sg.members, Prefix.objects.filter(ip_version=6))
         self.assertEqual(sg.static_group_associations.count(), Prefix.objects.filter(ip_version=6).count())
+        # test cumulative removal and alternate code path
+        sg.remove_members(list(Prefix.objects.filter(ip_version=6)))
+        self.assertQuerysetEqual(sg.members, Prefix.objects.none())
+        self.assertEqual(sg.static_group_associations.count(), 0)
 
         # test property setter
         sg.members = Prefix.objects.filter(ip_version=4)
         self.assertQuerysetEqualAndNotEmpty(sg.members, Prefix.objects.filter(ip_version=4))
+        sg.members = Prefix.objects.filter(ip_version=6)
+        self.assertQuerysetEqualAndNotEmpty(sg.members, Prefix.objects.filter(ip_version=6))
+        sg.members = list(Prefix.objects.filter(ip_version=4))
+        self.assertQuerysetEqualAndNotEmpty(sg.members, Prefix.objects.filter(ip_version=4))
         sg.members = list(Prefix.objects.filter(ip_version=6))
         self.assertQuerysetEqualAndNotEmpty(sg.members, Prefix.objects.filter(ip_version=6))