From e038e6ac401642f722edb044934051412ee91328 Mon Sep 17 00:00:00 2001 From: Arthur Date: Mon, 2 Oct 2023 08:15:24 -0700 Subject: [PATCH 1/6] 12336 make region API calls atomic --- netbox/dcim/api/views.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/netbox/dcim/api/views.py b/netbox/dcim/api/views.py index f045f1bb42a..0421e67301e 100644 --- a/netbox/dcim/api/views.py +++ b/netbox/dcim/api/views.py @@ -1,3 +1,4 @@ +from django.db import transaction from django.http import Http404, HttpResponse from django.shortcuts import get_object_or_404 from drf_spectacular.types import OpenApiTypes @@ -109,6 +110,18 @@ class RegionViewSet(NetBoxModelViewSet): serializer_class = serializers.RegionSerializer filterset_class = filtersets.RegionFilterSet + @transaction.atomic + def create(self, request, *args, **kwargs): + return super().create(request, *args, **kwargs) + + @transaction.atomic + def update(self, request, *args, **kwargs): + return super().update(request, *args, **kwargs) + + @transaction.atomic + def destroy(self, request, *args, **kwargs): + return super().destroy(request, *args, **kwargs) + # # Site groups From b1d8acd72281965028248b7140b7573789f805b2 Mon Sep 17 00:00:00 2001 From: Arthur Date: Thu, 5 Oct 2023 10:38:24 -0700 Subject: [PATCH 2/6] 12336 switch to pg locks --- netbox/dcim/api/views.py | 10 +++++----- netbox/netbox/constants.py | 1 + 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/netbox/dcim/api/views.py b/netbox/dcim/api/views.py index 0421e67301e..9b79fac4a87 100644 --- a/netbox/dcim/api/views.py +++ b/netbox/dcim/api/views.py @@ -1,6 +1,6 @@ -from django.db import transaction from django.http import Http404, HttpResponse from django.shortcuts import get_object_or_404 +from django_pglocks import advisory_lock from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import extend_schema, OpenApiParameter from rest_framework.decorators import action @@ -23,7 +23,7 @@ from netbox.api.renderers import TextRenderer from netbox.api.viewsets import NetBoxModelViewSet from netbox.api.viewsets.mixins import SequentialBulkCreatesMixin -from netbox.constants import NESTED_SERIALIZER_PREFIX +from netbox.constants import NESTED_SERIALIZER_PREFIX, ADVISORY_LOCK_KEYS from utilities.api import get_serializer_for_model from utilities.utils import count_related from virtualization.models import VirtualMachine @@ -110,15 +110,15 @@ class RegionViewSet(NetBoxModelViewSet): serializer_class = serializers.RegionSerializer filterset_class = filtersets.RegionFilterSet - @transaction.atomic + @advisory_lock(ADVISORY_LOCK_KEYS['regions']) def create(self, request, *args, **kwargs): return super().create(request, *args, **kwargs) - @transaction.atomic + @advisory_lock(ADVISORY_LOCK_KEYS['regions']) def update(self, request, *args, **kwargs): return super().update(request, *args, **kwargs) - @transaction.atomic + @advisory_lock(ADVISORY_LOCK_KEYS['regions']) def destroy(self, request, *args, **kwargs): return super().destroy(request, *args, **kwargs) diff --git a/netbox/netbox/constants.py b/netbox/netbox/constants.py index d69edc69c38..d5da1c9128e 100644 --- a/netbox/netbox/constants.py +++ b/netbox/netbox/constants.py @@ -15,4 +15,5 @@ 'available-ips': 100200, 'available-vlans': 100300, 'available-asns': 100400, + 'regions': 100500, } From e9c447ad92a5658914bc0eb0424d39c1fba4202d Mon Sep 17 00:00:00 2001 From: Arthur Date: Thu, 12 Oct 2023 11:27:33 -0700 Subject: [PATCH 3/6] 12336 add locks to all views using mptt models --- netbox/dcim/api/views.py | 27 +++++++------------------- netbox/netbox/api/viewsets/__init__.py | 21 ++++++++++++++++++++ netbox/netbox/constants.py | 11 ++++++++++- netbox/tenancy/api/views.py | 6 +++--- netbox/wireless/api/views.py | 4 ++-- netbox/wireless/models.py | 1 - 6 files changed, 43 insertions(+), 27 deletions(-) diff --git a/netbox/dcim/api/views.py b/netbox/dcim/api/views.py index 9b79fac4a87..80a99173653 100644 --- a/netbox/dcim/api/views.py +++ b/netbox/dcim/api/views.py @@ -1,6 +1,5 @@ from django.http import Http404, HttpResponse from django.shortcuts import get_object_or_404 -from django_pglocks import advisory_lock from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import extend_schema, OpenApiParameter from rest_framework.decorators import action @@ -21,9 +20,9 @@ from netbox.api.metadata import ContentTypeMetadata from netbox.api.pagination import StripCountAnnotationsPaginator from netbox.api.renderers import TextRenderer -from netbox.api.viewsets import NetBoxModelViewSet +from netbox.api.viewsets import NetBoxModelViewSet, MPTTLockedMixin from netbox.api.viewsets.mixins import SequentialBulkCreatesMixin -from netbox.constants import NESTED_SERIALIZER_PREFIX, ADVISORY_LOCK_KEYS +from netbox.constants import NESTED_SERIALIZER_PREFIX from utilities.api import get_serializer_for_model from utilities.utils import count_related from virtualization.models import VirtualMachine @@ -99,7 +98,7 @@ def paths(self, request, pk): # Regions # -class RegionViewSet(NetBoxModelViewSet): +class RegionViewSet(MPTTLockedMixin, NetBoxModelViewSet): queryset = Region.objects.add_related_count( Region.objects.all(), Site, @@ -110,24 +109,12 @@ class RegionViewSet(NetBoxModelViewSet): serializer_class = serializers.RegionSerializer filterset_class = filtersets.RegionFilterSet - @advisory_lock(ADVISORY_LOCK_KEYS['regions']) - def create(self, request, *args, **kwargs): - return super().create(request, *args, **kwargs) - - @advisory_lock(ADVISORY_LOCK_KEYS['regions']) - def update(self, request, *args, **kwargs): - return super().update(request, *args, **kwargs) - - @advisory_lock(ADVISORY_LOCK_KEYS['regions']) - def destroy(self, request, *args, **kwargs): - return super().destroy(request, *args, **kwargs) - # # Site groups # -class SiteGroupViewSet(NetBoxModelViewSet): +class SiteGroupViewSet(MPTTLockedMixin, NetBoxModelViewSet): queryset = SiteGroup.objects.add_related_count( SiteGroup.objects.all(), Site, @@ -162,7 +149,7 @@ class SiteViewSet(NetBoxModelViewSet): # Locations # -class LocationViewSet(NetBoxModelViewSet): +class LocationViewSet(MPTTLockedMixin, NetBoxModelViewSet): queryset = Location.objects.add_related_count( Location.objects.add_related_count( Location.objects.all(), @@ -363,7 +350,7 @@ class DeviceBayTemplateViewSet(NetBoxModelViewSet): filterset_class = filtersets.DeviceBayTemplateFilterSet -class InventoryItemTemplateViewSet(NetBoxModelViewSet): +class InventoryItemTemplateViewSet(MPTTLockedMixin, NetBoxModelViewSet): queryset = InventoryItemTemplate.objects.prefetch_related('device_type__manufacturer', 'role') serializer_class = serializers.InventoryItemTemplateSerializer filterset_class = filtersets.InventoryItemTemplateFilterSet @@ -551,7 +538,7 @@ class DeviceBayViewSet(NetBoxModelViewSet): brief_prefetch_fields = ['device'] -class InventoryItemViewSet(NetBoxModelViewSet): +class InventoryItemViewSet(MPTTLockedMixin, NetBoxModelViewSet): queryset = InventoryItem.objects.prefetch_related('device', 'manufacturer', 'tags') serializer_class = serializers.InventoryItemSerializer filterset_class = filtersets.InventoryItemFilterSet diff --git a/netbox/netbox/api/viewsets/__init__.py b/netbox/netbox/api/viewsets/__init__.py index 5fe81b1f569..d481d7fcabe 100644 --- a/netbox/netbox/api/viewsets/__init__.py +++ b/netbox/netbox/api/viewsets/__init__.py @@ -3,6 +3,8 @@ from django.core.exceptions import ObjectDoesNotExist, PermissionDenied from django.db import transaction from django.db.models import ProtectedError +from django_pglocks import advisory_lock +from netbox.constants import ADVISORY_LOCK_KEYS from rest_framework import mixins as drf_mixins from rest_framework.response import Response from rest_framework.viewsets import GenericViewSet @@ -157,3 +159,22 @@ def perform_destroy(self, instance): logger.info(f"Deleting {model._meta.verbose_name} {instance} (PK: {instance.pk})") return super().perform_destroy(instance) + + +class MPTTLockedMixin(GenericViewSet): + """ + Puts pglock on objects that derive from MPTTModel for parallel API calling. + Note: If adding this to a view, must add the model name to ADVISORY_LOCK_KEYS + """ + + def create(self, request, *args, **kwargs): + with advisory_lock(ADVISORY_LOCK_KEYS[self.queryset.model._meta.verbose_name.lower()]): + return super().create(request, *args, **kwargs) + + def update(self, request, *args, **kwargs): + with advisory_lock(ADVISORY_LOCK_KEYS[self.queryset.model._meta.verbose_name.lower()]): + return super().update(request, *args, **kwargs) + + def destroy(self, request, *args, **kwargs): + with advisory_lock(ADVISORY_LOCK_KEYS[self.queryset.model._meta.verbose_name.lower()]): + return super().destroy(request, *args, **kwargs) diff --git a/netbox/netbox/constants.py b/netbox/netbox/constants.py index d5da1c9128e..2f221a4b515 100644 --- a/netbox/netbox/constants.py +++ b/netbox/netbox/constants.py @@ -15,5 +15,14 @@ 'available-ips': 100200, 'available-vlans': 100300, 'available-asns': 100400, - 'regions': 100500, + + # MPTT locks + 'region': 100500, + 'sitegroup': 100501, + 'location': 100502, + 'tenantgroup': 100503, + 'contactgroup': 100504, + 'wirelesslangroup': 100505, + 'inventoryitem': 100506, + 'inventoryitemtemplate': 100507, } diff --git a/netbox/tenancy/api/views.py b/netbox/tenancy/api/views.py index 39c86d80e84..71a4961c3a0 100644 --- a/netbox/tenancy/api/views.py +++ b/netbox/tenancy/api/views.py @@ -3,7 +3,7 @@ from circuits.models import Circuit from dcim.models import Device, Rack, Site from ipam.models import IPAddress, Prefix, VLAN, VRF -from netbox.api.viewsets import NetBoxModelViewSet +from netbox.api.viewsets import NetBoxModelViewSet, MPTTLockedMixin from tenancy import filtersets from tenancy.models import * from utilities.utils import count_related @@ -23,7 +23,7 @@ def get_view_name(self): # Tenants # -class TenantGroupViewSet(NetBoxModelViewSet): +class TenantGroupViewSet(MPTTLockedMixin, NetBoxModelViewSet): queryset = TenantGroup.objects.add_related_count( TenantGroup.objects.all(), Tenant, @@ -58,7 +58,7 @@ class TenantViewSet(NetBoxModelViewSet): # Contacts # -class ContactGroupViewSet(NetBoxModelViewSet): +class ContactGroupViewSet(MPTTLockedMixin, NetBoxModelViewSet): queryset = ContactGroup.objects.add_related_count( ContactGroup.objects.all(), Contact, diff --git a/netbox/wireless/api/views.py b/netbox/wireless/api/views.py index 1103cec371e..a6cc9f53574 100644 --- a/netbox/wireless/api/views.py +++ b/netbox/wireless/api/views.py @@ -1,6 +1,6 @@ from rest_framework.routers import APIRootView -from netbox.api.viewsets import NetBoxModelViewSet +from netbox.api.viewsets import NetBoxModelViewSet, MPTTLockedMixin from wireless import filtersets from wireless.models import * from . import serializers @@ -14,7 +14,7 @@ def get_view_name(self): return 'Wireless' -class WirelessLANGroupViewSet(NetBoxModelViewSet): +class WirelessLANGroupViewSet(MPTTLockedMixin, NetBoxModelViewSet): queryset = WirelessLANGroup.objects.add_related_count( WirelessLANGroup.objects.all(), WirelessLAN, diff --git a/netbox/wireless/models.py b/netbox/wireless/models.py index 04691853534..e8e48eef875 100644 --- a/netbox/wireless/models.py +++ b/netbox/wireless/models.py @@ -2,7 +2,6 @@ from django.db import models from django.urls import reverse from django.utils.translation import gettext_lazy as _ -from mptt.models import MPTTModel from dcim.choices import LinkStatusChoices from dcim.constants import WIRELESS_IFACE_TYPES From 960e64c3654e97af85e25a7095cba083f791de64 Mon Sep 17 00:00:00 2001 From: Arthur Date: Thu, 12 Oct 2023 11:53:40 -0700 Subject: [PATCH 4/6] 12336 fix ADVISORY_LOCK_KEYS reference --- netbox/netbox/api/viewsets/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/netbox/netbox/api/viewsets/__init__.py b/netbox/netbox/api/viewsets/__init__.py index d481d7fcabe..7a12c060d11 100644 --- a/netbox/netbox/api/viewsets/__init__.py +++ b/netbox/netbox/api/viewsets/__init__.py @@ -168,13 +168,13 @@ class MPTTLockedMixin(GenericViewSet): """ def create(self, request, *args, **kwargs): - with advisory_lock(ADVISORY_LOCK_KEYS[self.queryset.model._meta.verbose_name.lower()]): + with advisory_lock(ADVISORY_LOCK_KEYS[self.queryset.model._meta.verbose_name.lower().replace(" ", "")]): return super().create(request, *args, **kwargs) def update(self, request, *args, **kwargs): - with advisory_lock(ADVISORY_LOCK_KEYS[self.queryset.model._meta.verbose_name.lower()]): + with advisory_lock(ADVISORY_LOCK_KEYS[self.queryset.model._meta.verbose_name.lower().replace(" ", "")]): return super().update(request, *args, **kwargs) def destroy(self, request, *args, **kwargs): - with advisory_lock(ADVISORY_LOCK_KEYS[self.queryset.model._meta.verbose_name.lower()]): + with advisory_lock(ADVISORY_LOCK_KEYS[self.queryset.model._meta.verbose_name.lower().replace(" ", "")]): return super().destroy(request, *args, **kwargs) From 870b1fabe590e7bfb90255930bc427ee82f84188 Mon Sep 17 00:00:00 2001 From: Arthur Date: Fri, 13 Oct 2023 09:21:22 -0700 Subject: [PATCH 5/6] 12336 review changes --- netbox/netbox/api/viewsets/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/netbox/netbox/api/viewsets/__init__.py b/netbox/netbox/api/viewsets/__init__.py index 7a12c060d11..c6794bb6188 100644 --- a/netbox/netbox/api/viewsets/__init__.py +++ b/netbox/netbox/api/viewsets/__init__.py @@ -161,20 +161,20 @@ def perform_destroy(self, instance): return super().perform_destroy(instance) -class MPTTLockedMixin(GenericViewSet): +class MPTTLockedMixin: """ Puts pglock on objects that derive from MPTTModel for parallel API calling. Note: If adding this to a view, must add the model name to ADVISORY_LOCK_KEYS """ def create(self, request, *args, **kwargs): - with advisory_lock(ADVISORY_LOCK_KEYS[self.queryset.model._meta.verbose_name.lower().replace(" ", "")]): + with advisory_lock(ADVISORY_LOCK_KEYS[self.queryset.model._meta.model_name]): return super().create(request, *args, **kwargs) def update(self, request, *args, **kwargs): - with advisory_lock(ADVISORY_LOCK_KEYS[self.queryset.model._meta.verbose_name.lower().replace(" ", "")]): + with advisory_lock(ADVISORY_LOCK_KEYS[self.queryset.model._meta.model_name]): return super().update(request, *args, **kwargs) def destroy(self, request, *args, **kwargs): - with advisory_lock(ADVISORY_LOCK_KEYS[self.queryset.model._meta.verbose_name.lower().replace(" ", "")]): + with advisory_lock(ADVISORY_LOCK_KEYS[self.queryset.model._meta.model_name]): return super().destroy(request, *args, **kwargs) From 537ef80bde944fd611a776e2564b3240af474fdf Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Tue, 17 Oct 2023 09:39:56 -0400 Subject: [PATCH 6/6] Tweak advisory lock numbering --- netbox/netbox/constants.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/netbox/netbox/constants.py b/netbox/netbox/constants.py index 2f221a4b515..2f4ee8e6b08 100644 --- a/netbox/netbox/constants.py +++ b/netbox/netbox/constants.py @@ -11,18 +11,19 @@ # When adding a new key, pick something arbitrary and unique so that it is easily searchable in # query logs. ADVISORY_LOCK_KEYS = { + # Available object locks 'available-prefixes': 100100, 'available-ips': 100200, 'available-vlans': 100300, 'available-asns': 100400, # MPTT locks - 'region': 100500, - 'sitegroup': 100501, - 'location': 100502, - 'tenantgroup': 100503, - 'contactgroup': 100504, - 'wirelesslangroup': 100505, - 'inventoryitem': 100506, - 'inventoryitemtemplate': 100507, + 'region': 105100, + 'sitegroup': 105200, + 'location': 105300, + 'tenantgroup': 105400, + 'contactgroup': 105500, + 'wirelesslangroup': 105600, + 'inventoryitem': 105700, + 'inventoryitemtemplate': 105800, }