Repository: nautobot/nautobot
Issue category: Performance Issue

Issue:
Hundreds of SQL queries made when fetching device details page with ~ 20 custom fields
<!--
    NOTE: IF YOUR ISSUE DOES NOT FOLLOW THIS TEMPLATE, IT WILL BE CLOSED.

    This form is only for reporting reproducible bugs. If you need assistance
    with Nautobot installation, or if you have a general question, please start a
    discussion instead: https://github.com/nautobot/nautobot/discussions

    Please describe the environment in which you are running Nautobot. Be sure
    that you are running an unmodified instance of the latest stable release
    before submitting a bug report, and that any Apps have been disabled.
-->
### Environment
* Nautobot version (Docker tag too if applicable): 2.3.8
* Python version: 3.10.15
* Database platform, version: PostgreSQL 15.4

<!--
    Describe in detail the exact steps that someone else can take to reproduce
    this bug using the current stable release of Nautobot. Begin with the
    creation of any necessary database objects and call out every operation
    being performed explicitly. If reporting a bug in the REST API, be sure to
    reconstruct the raw HTTP request(s) being made: Don't rely on a client
    library such as pynautobot.
-->
### Steps to Reproduce
1. Create 19 Custom Fields for dcim|device
2. Custom fields are of types:
- Bool - 1 cf
- Date - 1 cf
- Integer - 3 cfs
- Selection (2 and 5 choices) - 2 cfs
- Text - 11 cfs
- URLs - 3 cfs

3. Enable Silk profiling
4. Retrieve device details page and observe number of SQL queries being made and how long it takes to render the page

<!-- What did you expect to happen? -->
### Expected Behavior

Device details page is rendered quickly and without hundreds of SQL calls.

<!-- What happened instead? -->
### Observed Behavior

Device details page takes long time (~5-15 seconds) to load and hundreds (~500) of SQL calls are observed.


Hints:
Hi @progala, thank you for submitting the issue. Do you have a high number of device components attached to the device as well (ConsoleServerPort, ConsolePort, Interface, VRFs, etc.) I want to make sure whether querying those device components is causing the slowdown instead of the custom fields. It would be really helpful if you could post the SQL queries.
An issue that is possibly related #6456
We think the root cause is a re-lookup of custom fields in order to construct the filter set used to apply relationship filters. And we think that the appropriate fix is to add caching to the custom field filter set.
to @HanlinMiao point, I raised this back in December: https://github.com/nautobot/nautobot/issues/6594
Looking more closely at the Silk profile and the code, we are using `CustomField.objects.get_for_model()` which *does* cache CustomField instances/querysets. However, we are then (in the case of select/multiselect custom fields) looking up the associated CustomFieldChoice records for said CustomField multiple times without caching so far as I can tell.

Patch:
diff --git a/changes/6767.added b/changes/6767.added
new file mode 100644
index 0000000000..f9de871e1a
--- /dev/null
+++ b/changes/6767.added
@@ -0,0 +1,1 @@
+Added cacheable `CustomField.choices` property for retrieving the list of permissible values for a select/multiselect Custom Field.
diff --git a/changes/6767.fixed b/changes/6767.fixed
new file mode 100644
index 0000000000..6e0316fc6a
--- /dev/null
+++ b/changes/6767.fixed
@@ -0,0 +1,3 @@
+Improved performance of object detail views when a large number of select/multiselect Custom Fields and also filtered Relationships are defined on the model class.
+Improved performance of Device detail view by adding appropriate `select_related`/`prefetch_related` calls.
+Fixed display of Cluster Group in Device detail view.
diff --git a/nautobot/core/views/utils.py b/nautobot/core/views/utils.py
index e9579ef6bd..d9a640181f 100644
--- a/nautobot/core/views/utils.py
+++ b/nautobot/core/views/utils.py
@@ -137,10 +137,10 @@ def get_csv_form_fields_from_serializer_class(serializer_class):
                 elif cf.type == CustomFieldTypeChoices.TYPE_DATE:
                     field_info["format"] = mark_safe("<code>YYYY-MM-DD</code>")  # noqa: S308
                 elif cf.type == CustomFieldTypeChoices.TYPE_SELECT:
-                    field_info["choices"] = {cfc.value: cfc.value for cfc in cf.custom_field_choices.all()}
+                    field_info["choices"] = {value: value for value in cf.choices}
                 elif cf.type == CustomFieldTypeChoices.TYPE_MULTISELECT:
                     field_info["format"] = mark_safe('<code>"value,value"</code>')  # noqa: S308
-                    field_info["choices"] = {cfc.value: cfc.value for cfc in cf.custom_field_choices.all()}
+                    field_info["choices"] = {value: value for value in cf.choices}
                 fields.append(field_info)
             continue
 
diff --git a/nautobot/dcim/templates/dcim/device.html b/nautobot/dcim/templates/dcim/device.html
index 9bc268961b..7f0a47b5d1 100644
--- a/nautobot/dcim/templates/dcim/device.html
+++ b/nautobot/dcim/templates/dcim/device.html
@@ -214,8 +214,8 @@
                                         <tr>
                                             <td>Cluster</td>
                                             <td>
-                                                {% if object.cluster.group %}
-                                                    {{ object.cluster.group|hyperlinked_object }} /
+                                                {% if object.cluster.cluster_group %}
+                                                    {{ object.cluster.cluster_group|hyperlinked_object }} /
                                                 {% endif %}
                                                 {{ object.cluster|hyperlinked_object }}
                                             </td>
diff --git a/nautobot/dcim/views.py b/nautobot/dcim/views.py
index 72a8871a4d..13bd6902af 100644
--- a/nautobot/dcim/views.py
+++ b/nautobot/dcim/views.py
@@ -1747,16 +1747,21 @@ class DeviceListView(generic.ObjectListView):
 
 class DeviceView(generic.ObjectView):
     queryset = Device.objects.select_related(
+        "cluster__cluster_group",
+        "controller_managed_device_group__controller",
+        "device_redundancy_group",
+        "device_type__device_family",
         "location",
-        "rack__rack_group",
-        "tenant__tenant_group",
-        "role",
         "platform",
         "primary_ip4",
         "primary_ip6",
+        "rack__rack_group",
+        "role",
+        "secrets_group",
         "software_version",
         "status",
-    )
+        "tenant__tenant_group",
+    ).prefetch_related("images", "software_image_files")
 
     object_detail_content = object_detail.ObjectDetailContent(
         extra_buttons=(
diff --git a/nautobot/extras/models/customfields.py b/nautobot/extras/models/customfields.py
index ba9d87494b..7d6c3a652b 100644
--- a/nautobot/extras/models/customfields.py
+++ b/nautobot/extras/models/customfields.py
@@ -370,11 +370,11 @@ class CustomFieldManager(BaseManager.from_queryset(RestrictedQuerySet)):
 
     def get_for_model(self, model, exclude_filter_disabled=False):
         """
-        Return all CustomFields assigned to the given model.
+        Return (and cache) all CustomFields assigned to the given model.
 
         Args:
-            model: The django model to which custom fields are registered
-            exclude_filter_disabled: Exclude any custom fields which have filter logic disabled
+            model (Model): The django model to which custom fields are registered
+            exclude_filter_disabled (bool): Exclude any custom fields which have filter logic disabled
         """
         concrete_model = model._meta.concrete_model
         cache_key = (
@@ -521,6 +521,26 @@ class Meta:
     def __str__(self):
         return self.label
 
+    @property
+    def choices_cache_key(self):
+        return f"nautobot.extras.customfield.choices.{self.pk}"
+
+    @property
+    def choices(self) -> list[str]:
+        """
+        Cacheable shorthand for retrieving custom_field_choices values associated with this model.
+
+        Returns:
+            list[str]: List of choice values, ordered by weight.
+        """
+        if self.type not in [CustomFieldTypeChoices.TYPE_SELECT, CustomFieldTypeChoices.TYPE_MULTISELECT]:
+            return []
+        choices = cache.get(self.choices_cache_key)
+        if choices is None:
+            choices = list(self.custom_field_choices.order_by("weight", "value").values_list("value", flat=True))
+            cache.set(self.choices_cache_key, choices)
+        return choices
+
     def save(self, *args, **kwargs):
         self.clean()
         super().save(*args, **kwargs)
@@ -679,12 +699,11 @@ def to_form_field(
 
         # Select or Multi-select
         else:
-            choices = [(cfc.value, cfc.value) for cfc in self.custom_field_choices.all()]
-            default_choice = self.custom_field_choices.filter(value=self.default).first()
+            choices = [(value, value) for value in self.choices]
 
             # Set the initial value to the first available choice (if any)
             if self.type == CustomFieldTypeChoices.TYPE_SELECT and not for_filter_form:
-                if not required or default_choice is None:
+                if not required or self.default not in self.choices:
                     choices = add_blank_choice(choices)
                 field_class = CSVChoiceField if for_csv_import else forms.ChoiceField
                 field = field_class(
@@ -779,17 +798,15 @@ def validate(self, value):
 
             # Validate selected choice
             elif self.type == CustomFieldTypeChoices.TYPE_SELECT:
-                if value not in self.custom_field_choices.values_list("value", flat=True):
-                    raise ValidationError(
-                        f"Invalid choice ({value}). Available choices are: {', '.join(self.custom_field_choices.values_list('value', flat=True))}"
-                    )
+                if value not in self.choices:
+                    raise ValidationError(f"Invalid choice ({value}). Available choices are: {', '.join(self.choices)}")
 
             elif self.type == CustomFieldTypeChoices.TYPE_MULTISELECT:
                 if isinstance(value, str):
                     value = value.split(",")
-                if not set(value).issubset(self.custom_field_choices.values_list("value", flat=True)):
+                if not set(value).issubset(self.choices):
                     raise ValidationError(
-                        f"Invalid choice(s) ({value}). Available choices are: {', '.join(self.custom_field_choices.values_list('value', flat=True))}"
+                        f"Invalid choice(s) ({value}). Available choices are: {', '.join(self.choices)}"
                     )
 
         elif self.required:
diff --git a/nautobot/extras/signals.py b/nautobot/extras/signals.py
index 06d8bc160e..e8de8d3b2c 100644
--- a/nautobot/extras/signals.py
+++ b/nautobot/extras/signals.py
@@ -28,6 +28,7 @@
     ComputedField,
     ContactAssociation,
     CustomField,
+    CustomFieldChoice,
     DynamicGroup,
     DynamicGroupMembership,
     GitRepository,
@@ -97,6 +98,19 @@ def invalidate_models_cache(sender, **kwargs):
             cache.delete_pattern(f"{manager.keys_for_model.cache_key_prefix}.*")
 
 
+@receiver(post_delete, sender=CustomField)
+@receiver(post_delete, sender=CustomFieldChoice)
+@receiver(post_save, sender=CustomFieldChoice)
+@receiver(post_save, sender=CustomField)
+def invalidate_choices_cache(sender, instance, **kwargs):
+    """Invalidate the choices cache for CustomFields."""
+    with contextlib.suppress(redis.exceptions.ConnectionError):
+        if sender is CustomField:
+            cache.delete(instance.choices_cache_key)
+        else:
+            cache.delete(instance.custom_field.choices_cache_key)
+
+
 @receiver(post_save, sender=Relationship)
 @receiver(m2m_changed, sender=Relationship)
 @receiver(post_delete, sender=Relationship)


Test patch:
diff --git a/nautobot/extras/tests/test_customfields.py b/nautobot/extras/tests/test_customfields.py
index aaa3dcc4ed..ef2d6a1a69 100644
--- a/nautobot/extras/tests/test_customfields.py
+++ b/nautobot/extras/tests/test_customfields.py
@@ -53,12 +53,12 @@ def test_immutable_fields(self):
 
         instance.refresh_from_db()
         instance.key = "custom_field_2"
-        with self.assertRaises(ValidationError):
+        with self.assertRaisesRegex(ValidationError, "Key cannot be changed once created"):
             instance.validated_save()
 
         instance.refresh_from_db()
         instance.type = CustomFieldTypeChoices.TYPE_SELECT
-        with self.assertRaises(ValidationError):
+        with self.assertRaisesRegex(ValidationError, "Type cannot be changed once created"):
             instance.validated_save()
 
     def test_simple_fields(self):
@@ -165,9 +165,14 @@ def test_select_field(self):
         cf.save()
         cf.content_types.set([obj_type])
 
-        CustomFieldChoice.objects.create(custom_field=cf, value="Option A")
-        CustomFieldChoice.objects.create(custom_field=cf, value="Option B")
-        CustomFieldChoice.objects.create(custom_field=cf, value="Option C")
+        CustomFieldChoice.objects.create(custom_field=cf, value="Option A", weight=100)
+        self.assertEqual(["Option A"], cf.choices)
+        CustomFieldChoice.objects.create(custom_field=cf, value="Option B", weight=200)
+        self.assertEqual(["Option A", "Option B"], cf.choices)
+        CustomFieldChoice.objects.create(custom_field=cf, value="Option C", weight=300)
+        self.assertEqual(["Option A", "Option B", "Option C"], cf.choices)
+        with self.assertNumQueries(0):  # verify caching
+            self.assertEqual(["Option A", "Option B", "Option C"], cf.choices)
 
         # Assign a value to the first Location
         location = Location.objects.get(name="Location A")
@@ -199,9 +204,14 @@ def test_multi_select_field(self):
         cf.save()
         cf.content_types.set([obj_type])
 
-        CustomFieldChoice.objects.create(custom_field=cf, value="Option A")
-        CustomFieldChoice.objects.create(custom_field=cf, value="Option B")
-        CustomFieldChoice.objects.create(custom_field=cf, value="Option C")
+        CustomFieldChoice.objects.create(custom_field=cf, value="Option A", weight=100)
+        self.assertEqual(["Option A"], cf.choices)
+        CustomFieldChoice.objects.create(custom_field=cf, value="Option B", weight=200)
+        self.assertEqual(["Option A", "Option B"], cf.choices)
+        CustomFieldChoice.objects.create(custom_field=cf, value="Option C", weight=300)
+        self.assertEqual(["Option A", "Option B", "Option C"], cf.choices)
+        with self.assertNumQueries(0):  # verify caching
+            self.assertEqual(["Option A", "Option B", "Option C"], cf.choices)
 
         # Assign a value to the first Location
         location = Location.objects.get(name="Location A")
@@ -293,21 +303,18 @@ def test_text_field_value(self):
         # Assign a disallowed value (list) to the first Location
         location = Location.objects.get(name="Location A")
         location.cf[cf.key] = ["I", "am", "a", "list"]
-        with self.assertRaises(ValidationError) as context:
+        with self.assertRaisesRegex(ValidationError, "Value must be a string"):
             location.validated_save()
-        self.assertIn("Value must be a string", str(context.exception))
 
         # Assign another disallowed value (int) to the first Location
         location.cf[cf.key] = 2
-        with self.assertRaises(ValidationError) as context:
+        with self.assertRaisesRegex(ValidationError, "Value must be a string"):
             location.validated_save()
-        self.assertIn("Value must be a string", str(context.exception))
 
         # Assign another disallowed value (bool) to the first Location
         location.cf[cf.key] = True
-        with self.assertRaises(ValidationError) as context:
+        with self.assertRaisesRegex(ValidationError, "Value must be a string"):
             location.validated_save()
-        self.assertIn("Value must be a string", str(context.exception))
 
         # Delete the stored value
         location.cf.pop(cf.key)
@@ -1294,7 +1301,7 @@ def test_custom_field_required(self):
         custom_field.content_types.set([ContentType.objects.get_for_model(Provider)])
 
         provider = Provider.objects.create(name="Test")
-        with self.assertRaises(ValidationError):
+        with self.assertRaisesRegex(ValidationError, "Missing required custom field 'custom_field'"):
             provider.validated_save()
 
     def test_custom_field_required_on_update(self):
@@ -1312,7 +1319,7 @@ def test_custom_field_required_on_update(self):
         provider = Provider.objects.create(name="Test", _custom_field_data={"custom_field": "Value"})
         provider.validated_save()
         provider._custom_field_data.pop("custom_field")
-        with self.assertRaises(ValidationError):
+        with self.assertRaisesRegex(ValidationError, "Missing required custom field 'custom_field'"):
             provider.validated_save()
 
     def test_update_removed_custom_field(self):
@@ -1373,7 +1380,7 @@ def test_missing_required_field(self):
         """
         Check that a ValidationError is raised if any required custom fields are not present.
         """
-        cf3 = CustomField(type=CustomFieldTypeChoices.TYPE_TEXT, label="Baz", required=True)
+        cf3 = CustomField(key="baz", type=CustomFieldTypeChoices.TYPE_TEXT, label="Baz", required=True)
         cf3.save()
         cf3.content_types.set([ContentType.objects.get_for_model(Location)])
 
@@ -1381,7 +1388,7 @@ def test_missing_required_field(self):
 
         # Set custom field data with a required field omitted
         location.cf["foo"] = "abc"
-        with self.assertRaises(ValidationError):
+        with self.assertRaisesRegex(ValidationError, "Missing required custom field 'baz'"):
             location.clean()
 
         location.cf["baz"] = "def"
@@ -1427,38 +1434,20 @@ def test_check_if_key_is_graphql_safe(self):
         """
         Check the GraphQL validation method on CustomField Key Attribute.
         """
-        # Check if it catches the cf.key starting with a digit.
-        cf1 = CustomField(type=CustomFieldTypeChoices.TYPE_TEXT, label="Test 1", key="12_test_1")
-        with self.assertRaises(ValidationError) as error:
-            cf1.validated_save()
-        self.assertIn(
-            "This key is not Python/GraphQL safe. Please do not start the key with a digit and do not use hyphens or whitespace",
-            str(error.exception),
-        )
-        # Check if it catches the cf.key with whitespace.
-        cf1.key = "test 1"
-        with self.assertRaises(ValidationError) as error:
-            cf1.validated_save()
-        self.assertIn(
-            "This key is not Python/GraphQL safe. Please do not start the key with a digit and do not use hyphens or whitespace",
-            str(error.exception),
-        )
-        # Check if it catches the cf.key with hyphens.
-        cf1.key = "test-1-custom-field"
-        with self.assertRaises(ValidationError) as error:
-            cf1.validated_save()
-        self.assertIn(
-            "This key is not Python/GraphQL safe. Please do not start the key with a digit and do not use hyphens or whitespace",
-            str(error.exception),
-        )
-        # Check if it catches the cf.key with special characters
-        cf1.key = "test_1_custom_f)(&d"
-        with self.assertRaises(ValidationError) as error:
-            cf1.validated_save()
-        self.assertIn(
-            "This key is not Python/GraphQL safe. Please do not start the key with a digit and do not use hyphens or whitespace",
-            str(error.exception),
-        )
+        cf1 = CustomField(type=CustomFieldTypeChoices.TYPE_TEXT, label="Test 1")
+        for invalid_key in [
+            "12_test_1",  # Check if it catches the cf.key starting with a digit.
+            "test 1",  # Check if it catches the cf.key with whitespace.
+            "test-1-custom-field",  # Check if it catches the cf.key with hyphens.
+            "test_1_custom_f)(&d",  # Check if it catches the cf.key with special characters
+        ]:
+            with self.assertRaisesRegex(
+                ValidationError,
+                "This key is not Python/GraphQL safe. "
+                "Please do not start the key with a digit and do not use hyphens or whitespace",
+            ):
+                cf1.key = invalid_key
+                cf1.validated_save()
 
 
 class CustomFieldFilterTest(TestCase):
@@ -1934,9 +1923,11 @@ def setUp(self):
         )
         self.cf.save()
         self.cf.content_types.set([obj_type])
+        self.assertEqual(self.cf.choices, [])
 
         self.choice = CustomFieldChoice(custom_field=self.cf, value="Foo")
         self.choice.save()
+        self.assertEqual(self.cf.choices, ["Foo"])
 
         location_status = Status.objects.get_for_model(Location).first()
         self.location_type = LocationType.objects.get(name="Campus")
@@ -1952,7 +1943,7 @@ def setUp(self):
 
     def test_default_value_must_be_valid_choice_sad_path(self):
         self.cf.default = "invalid value"
-        with self.assertRaises(ValidationError):
+        with self.assertRaisesRegex(ValidationError, 'Invalid default value "invalid value"'):
             self.cf.full_clean()
 
     def test_default_value_must_be_valid_choice_happy_path(self):
@@ -1965,6 +1956,13 @@ def test_active_choice_cannot_be_deleted(self):
         with self.assertRaises(ProtectedError):
             self.choice.delete()
 
+    def test_inactive_choice_can_be_deleted(self):
+        self.location._custom_field_data.pop("cf1")
+        self.location.validated_save()
+        self.assertEqual(self.cf.choices, ["Foo"])
+        self.choice.delete()
+        self.assertEqual(self.cf.choices, [])
+
     def test_custom_choice_deleted_with_field(self):
         self.cf.delete()
         self.assertEqual(CustomField.objects.count(), 1)  # custom field automatically added by the Example App