From ce184a620d6c012ebf0eed82838560fd6a959d9a Mon Sep 17 00:00:00 2001 From: Vincent Simonin Date: Thu, 28 May 2026 18:19:05 +0200 Subject: [PATCH 1/3] :bug: Fix container deletion stuck on large journal entry count Override Container.delete() to remove journal entries via _raw_delete before the Django cascade, bypassing per-row post_delete signals (ObjectChange writes) that made deletion O(n) slow for containers with thousands of journal entries. Co-Authored-By: Claude Sonnet 4.6 --- netbox_docker_plugin/models/container.py | 20 ++- .../tests/container/test_container_delete.py | 137 ++++++++++++++++++ 2 files changed, 154 insertions(+), 3 deletions(-) create mode 100644 netbox_docker_plugin/tests/container/test_container_delete.py diff --git a/netbox_docker_plugin/models/container.py b/netbox_docker_plugin/models/container.py index 870e319..61a1fbf 100644 --- a/netbox_docker_plugin/models/container.py +++ b/netbox_docker_plugin/models/container.py @@ -2,17 +2,19 @@ # pylint: disable=E1101 +from django.contrib.contenttypes.models import ContentType from django.contrib.postgres.fields import ArrayField from django.core.exceptions import ValidationError -from django.db import models -from django.db.models.functions import Lower -from django.urls import reverse from django.core.validators import ( MinLengthValidator, MaxLengthValidator, MinValueValidator, MaxValueValidator, ) +from django.db import models +from django.db.models.functions import Lower +from django.urls import reverse +from extras.models import JournalEntry from utilities.choices import ChoiceSet from utilities.querysets import RestrictedQuerySet from netbox.models import NetBoxModel @@ -306,6 +308,18 @@ class Meta: ), ) + def delete(self, using=None, keep_parents=False): + ct = ContentType.objects.get_for_model(self) + qs = JournalEntry.objects.filter( + assigned_object_type=ct, + assigned_object_id=self.pk, + ) + # _raw_delete issues a single SQL DELETE bypassing Django's Collector, + # avoiding per-row post_delete signals (ObjectChange writes) that make + # bulk journal cleanup O(n) slow. + qs._raw_delete(qs.db) # pylint: disable=protected-access + return super().delete(using, keep_parents) + def __str__(self): return f"{self.name}" diff --git a/netbox_docker_plugin/tests/container/test_container_delete.py b/netbox_docker_plugin/tests/container/test_container_delete.py new file mode 100644 index 0000000..998cb2d --- /dev/null +++ b/netbox_docker_plugin/tests/container/test_container_delete.py @@ -0,0 +1,137 @@ +"""Container deletion tests — journal entry cleanup""" + +import time + +from django.contrib.contenttypes.models import ContentType +from django.test import TestCase + +from extras.models import JournalEntry +from netbox_docker_plugin.models.container import Container +from netbox_docker_plugin.models.host import Host +from netbox_docker_plugin.models.image import Image +from netbox_docker_plugin.models.registry import Registry + + +def _make_journal_entries(container, count): + ct = ContentType.objects.get_for_model(Container) + batch_size = 10000 + for start in range(0, count, batch_size): + JournalEntry.objects.bulk_create( + [ + JournalEntry( + assigned_object_type=ct, + assigned_object_id=container.pk, + created_by=None, + kind="info", + comments=f"entry {start + i}", + ) + for i in range(min(batch_size, count - start)) + ] + ) + + +class ContainerDeleteJournalTestCase(TestCase): + """Verify that container deletion cleans up journal entries in batches.""" + + @classmethod + def setUpTestData(cls): + cls.host = Host.objects.create(endpoint="http://localhost:8080", name="host1") + cls.registry = Registry.objects.create( + host=cls.host, + name="registry1", + serveraddress="http://localhost:8080", + ) + cls.image = Image.objects.create( + host=cls.host, name="image1", registry=cls.registry + ) + + def _make_container(self, name="container1", state="exited"): + """Create and return a Container in the given state.""" + return Container.objects.create( + host=self.host, + image=self.image, + name=name, + operation="none", + state=state, + ) + + def test_delete_container_with_no_journal_entries(self): + """Container with no journal entries can be deleted without errors.""" + container = self._make_container() + pk = container.pk + t0 = time.monotonic() + container.delete() + print( + f"Deleted container (0 journal entries) in {(time.monotonic() - t0) * 1000:.1f}ms" + ) + + self.assertFalse(Container.objects.filter(pk=pk).exists()) + + def test_delete_container_removes_journal_entries(self): + """Journal entries linked to the deleted container are removed.""" + container = self._make_container(name="container2") + _make_journal_entries(container, 5) + ct = ContentType.objects.get_for_model(Container) + + self.assertEqual( + JournalEntry.objects.filter( + assigned_object_type=ct, assigned_object_id=container.pk + ).count(), + 5, + ) + + pk = container.pk + t0 = time.monotonic() + container.delete() + print( + f"Deleted container (5 journal entries) in {(time.monotonic() - t0) * 1000:.1f}ms" + ) + + self.assertFalse(Container.objects.filter(pk=pk).exists()) + self.assertEqual( + JournalEntry.objects.filter( + assigned_object_type=ct, assigned_object_id=pk + ).count(), + 0, + ) + + def test_delete_container_removes_many_journal_entries(self): + """Deletion removes all journal entries regardless of count.""" + container = self._make_container(name="container3") + _make_journal_entries(container, 500000) + ct = ContentType.objects.get_for_model(Container) + + pk = container.pk + t0 = time.monotonic() + container.delete() + elapsed = (time.monotonic() - t0) * 1000 + print(f"Deleted container (500000 journal entries) in {elapsed:.1f}ms") + + self.assertFalse(Container.objects.filter(pk=pk).exists()) + self.assertEqual( + JournalEntry.objects.filter( + assigned_object_type=ct, assigned_object_id=pk + ).count(), + 0, + ) + + def test_delete_does_not_remove_journal_entries_of_other_containers(self): + """Deleting one container must not remove journal entries of another.""" + container_a = self._make_container(name="container4a") + container_b = self._make_container(name="container4b") + _make_journal_entries(container_a, 3) + _make_journal_entries(container_b, 3) + ct = ContentType.objects.get_for_model(Container) + + t0 = time.monotonic() + container_a.delete() + print( + f"Deleted container (3 journal entries) in {(time.monotonic() - t0) * 1000:.1f}ms" + ) + + self.assertEqual( + JournalEntry.objects.filter( + assigned_object_type=ct, assigned_object_id=container_b.pk + ).count(), + 3, + ) From 7ba5fc86dd4bdd6b48956968f019a1d16b12279d Mon Sep 17 00:00:00 2001 From: Vincent Simonin Date: Thu, 28 May 2026 18:21:23 +0200 Subject: [PATCH 2/3] :bookmark: Bump version to 5.1.1 Co-Authored-By: Claude Sonnet 4.6 --- netbox_docker_plugin/__init__.py | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/netbox_docker_plugin/__init__.py b/netbox_docker_plugin/__init__.py index 49c8c57..d7f68e6 100644 --- a/netbox_docker_plugin/__init__.py +++ b/netbox_docker_plugin/__init__.py @@ -11,7 +11,7 @@ class NetBoxDockerConfig(PluginConfig): name = "netbox_docker_plugin" verbose_name = " NetBox Docker Plugin" description = "Manage Docker" - version = "5.1.0" + version = "5.1.1" base_url = "docker" min_version = "4.5.0" author = "Vincent Simonin , David Delassus " diff --git a/pyproject.toml b/pyproject.toml index fac40ce..616471d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "netbox-docker-plugin" -version = "5.1.0" +version = "5.1.1" authors = [ { name="Vincent Simonin", email="vincent@saashup.com" }, { name="David Delassus", email="david.jose.delassus@gmail.com" } From ebfa6b475a657b06c108cd9e5f620bd0eb1e66d7 Mon Sep 17 00:00:00 2001 From: Vincent Simonin Date: Thu, 28 May 2026 19:10:46 +0200 Subject: [PATCH 3/3] :zap: Optimise ContainerDeleteView confirmation page for large journal counts Override _get_dependent_objects to skip JournalEntry from the Django Collector (avoids fetching all rows) and replace it with a lazy COUNT proxy that answers len() with a single indexed COUNT(*) query and yields nothing on iteration. The accordion header shows the correct count while the page loads instantly regardless of journal entry volume. Also add select_related("host", "image") on the queryset to avoid N+1 queries when rendering the confirmation page. Co-Authored-By: Claude Sonnet 4.6 --- netbox_docker_plugin/views/container.py | 61 ++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/netbox_docker_plugin/views/container.py b/netbox_docker_plugin/views/container.py index 3c48406..346a561 100644 --- a/netbox_docker_plugin/views/container.py +++ b/netbox_docker_plugin/views/container.py @@ -1,5 +1,9 @@ """Container views definitions""" +from collections import defaultdict +from django.db import router +from django.db.models.deletion import Collector +from extras.models import JournalEntry from utilities.query import count_related from utilities.views import ViewTab, register_model_view from netbox.views import generic @@ -125,11 +129,66 @@ class ContainerBulkDeleteView(generic.BulkDeleteView): table = tables.ContainerTable +class _JournalEntryCount: + """Proxy passed to the deletion template in place of a full JournalEntry list. + + The template calls len() for the accordion header count, then iterates to + render individual rows. This object answers len() with a single COUNT query + and yields nothing on iteration, avoiding fetching hundreds of thousands of + rows just to display a number.""" + + def __init__(self, queryset): + self._queryset = queryset + self._count = None + + def __len__(self): + if self._count is None: + self._count = self._queryset.count() + return self._count + + def __iter__(self): + return iter([]) + + def __bool__(self): + return len(self) > 0 + + class ContainerDeleteView(generic.ObjectDeleteView): """Container delete view definition""" default_return_url = "plugins:netbox_docker_plugin:container_list" - queryset = Container.objects.all() + queryset = Container.objects.select_related("host", "image") + + def _get_dependent_objects(self, obj): + class SkipJournalCollector(Collector): + """Collector that skips JournalEntry to avoid a slow full scan.""" + + def collect(self, objs, **kwargs): # pylint: disable=arguments-differ + if getattr(objs, "model", None) is JournalEntry: + return + super().collect(objs, **kwargs) + + using = router.db_for_write(obj._meta.model) + collector = SkipJournalCollector(using=using) + collector.collect([obj]) + + dependent_objects = defaultdict(list) + for model, instances in collector.instances_with_model(): + if model._meta.auto_created: + continue + if instances == obj: + continue + dependent_objects[model].append(instances) + + journal_qs = JournalEntry.objects.filter( + assigned_object_type__app_label="netbox_docker_plugin", + assigned_object_type__model="container", + assigned_object_id=obj.pk, + ) + if journal_qs.exists(): + dependent_objects[JournalEntry] = _JournalEntryCount(journal_qs) + + return dict(dependent_objects) class ContainerOperationView(generic.ObjectEditView):