Repository: nautobot/nautobot
Issue category: Performance Issue

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)
```



Hints:
Thanks for the report and the investigation.

I suspect that in the DynamicGroup case it's the update to the members cache that's causing an out-of-memory condition, specifically `_set_members()`:

```py
            to_remove = self.members.exclude(pk__in=value.values_list("pk", flat=True))
            self._remove_members(to_remove)
            to_add = value.exclude(pk__in=self.members.values_list("pk", flat=True))
            self._add_members(to_add)
```

We may have to come up with some alternate approach here rather than trying to perform set operations on a queryset containing 200k member objects.


Not sure yet on the namespace/ip addresses symptom.
I attempted to get more solid information on the Namespace issue, and now have sorta solid data.

We have a Namespace with 100k+ assigned IP Addresses, a single VRF, and 14k prefixes. I misread my notes on this issue in my original post, but I attempted to get more solid information this morning. I CAN successfully open the "IP Addresses" tab in the Namespace, possibly because it is paginated. Attempting to open the associated VRF, however, is where it crashes, but only my browser, and only sometimes.

It took me 3 attempts to try to get solid information. The first two crashed my browser, but the nautobot process stayed alive. The third actually fully loaded, after about 15 minutes and with no other tabs open.

I see nothing in the logs to indicate any issues with nautobot. VRFs appear to just have trouble loading that many "Assigned Prefixes" into a single frame.
That makes sense. From a quick look at the code, `VRFView` appears to be missing logic to paginate any of its related-object tables (prefixes, route-targets, device assignments) so it presumably is trying to render all 14k prefixes as a single table view. That should be a straightforward fix compared to the DynamicGroup issue. :-)
I do not imagine many people are in a position to experience this issue, but any idea on the timescale for a fix?
Please note that this is a serious blocker for us
Working on a quick fix for the VRF view and related IPAM views. Fixing the dynamic group case may take a bit longer but it's on my radar.

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))