From d30e932547c379c13c97a8f6909227bb64b1c677 Mon Sep 17 00:00:00 2001 From: Andrew Cheng Date: Thu, 2 Jul 2026 16:18:37 +0000 Subject: [PATCH 1/2] Add base_version filter to compute net content diffs across arbitrary repository versions This PR adds a base_version filter to the content viewset that lets repository_version_added and repository_version_removed compute the net content diff between two arbitray (non-adjacent) repository versions, rather than only the single-step diff against the immediate predecessor. Benchmarks on a synthetic 200k-unit system with 1000-unit diffs between repo versions show that implementing this base_filter vs running a complex q query is ~5-15x faster, with the query going from 1486 ms -> 99 ms for a 50k/50k repo version pair and 2403 ms -> 479 ms for a 199k/199k repo version pair. With base_version, planning time also decreases significantly and stays ~0.2 ms. When base_version is omitted, behavior is unchanged (single-step diff), preserving backward compatibility. If base_version is provided without repository_version_added or repository_version_removed, nothing happens. Closes #7831 Assisted by: Claude Opus 4.8 --- CHANGES/7831.feature | 4 + pulpcore/app/models/repository.py | 31 +++++--- pulpcore/app/viewsets/content.py | 6 ++ pulpcore/app/viewsets/custom_filters.py | 51 ++++++++++++- .../api/using_plugin/test_repo_versions.py | 76 +++++++++++++++++++ 5 files changed, 157 insertions(+), 11 deletions(-) create mode 100644 CHANGES/7831.feature diff --git a/CHANGES/7831.feature b/CHANGES/7831.feature new file mode 100644 index 00000000000..a24f101409a --- /dev/null +++ b/CHANGES/7831.feature @@ -0,0 +1,4 @@ +Added an optional ``base_version`` filter to the content list endpoints. When combined with +``repository_version_added`` or ``repository_version_removed``, it returns the net set of content +added or removed between two arbitrary repository versions instead of only the single-step +difference against the filtered version's immediate predecessor. diff --git a/pulpcore/app/models/repository.py b/pulpcore/app/models/repository.py index 5be034f5ffa..2fcdece61f8 100644 --- a/pulpcore/app/models/repository.py +++ b/pulpcore/app/models/repository.py @@ -1000,13 +1000,26 @@ def get_content(self, content_qs=None): content_ids = self.content_ids if len(content_ids) >= 65535: # Workaround for PostgreSQL's limit on the number of parameters in a query - content_ids = ( - RepositoryVersion.objects.filter(pk=self.pk) - .annotate(cids=Func(F("content_ids"), function="unnest")) - .values_list("cids", flat=True) - ) + content_ids = self.content_ids_subquery() return content_qs.filter(pk__in=content_ids) + def content_ids_subquery(self): + """ + Return this version's ``content_ids`` as a database-side ``unnest`` subquery. + + Using a subquery keeps the content unit UUIDs inside PostgreSQL instead of loading the + whole array into Python and passing each UUID as a bound query parameter. This avoids the + per-query parameter limit and the memory/serialization cost for large repository versions. + + Returns: + django.db.models.QuerySet: A values queryset yielding the content unit UUIDs. + """ + return ( + RepositoryVersion.objects.filter(pk=self.pk) + .annotate(cids=Func(F("content_ids"), function="unnest")) + .values_list("cids", flat=True) + ) + @property def content(self): """ @@ -1119,8 +1132,8 @@ def added(self, base_version=None): if not base_version: return Content.objects.filter(version_memberships__version_added=self) - return Content.objects.filter(pk__in=self.content_ids).exclude( - pk__in=base_version.content_ids + return Content.objects.filter(pk__in=self.content_ids_subquery()).exclude( + pk__in=base_version.content_ids_subquery() ) def removed(self, base_version=None): @@ -1134,8 +1147,8 @@ def removed(self, base_version=None): if not base_version: return Content.objects.filter(version_memberships__version_removed=self) - return Content.objects.filter(pk__in=base_version.content_ids).exclude( - pk__in=self.content_ids + return Content.objects.filter(pk__in=base_version.content_ids_subquery()).exclude( + pk__in=self.content_ids_subquery() ) def contains(self, content): diff --git a/pulpcore/app/viewsets/content.py b/pulpcore/app/viewsets/content.py index 0fd5f7024f6..48a1481d885 100644 --- a/pulpcore/app/viewsets/content.py +++ b/pulpcore/app/viewsets/content.py @@ -23,6 +23,7 @@ ContentAddedRepositoryVersionFilter, ContentRemovedRepositoryVersionFilter, ContentRepositoryVersionFilter, + RepositoryVersionBaseFilter, ) @@ -125,6 +126,10 @@ class ContentFilter(BaseFilterSet): Return Content which was added in this repository version. repository_version_removed: Return Content which was removed from this repository version. + base_version: + When combined with repository_version_added / repository_version_removed, compute the + net difference relative to this base repository version instead of the filtered + version's immediate predecessor. Has no effect on its own. orphaned_for: Return Content which has been orphaned for a given number of minutes; -1 uses ORPHAN_PROTECTION_TIME value. @@ -135,6 +140,7 @@ class ContentFilter(BaseFilterSet): repository_version = ContentRepositoryVersionFilter() repository_version_added = ContentAddedRepositoryVersionFilter() repository_version_removed = ContentRemovedRepositoryVersionFilter() + base_version = RepositoryVersionBaseFilter() orphaned_for = OrphanedFilter( help_text="Minutes Content has been orphaned for. -1 uses ORPHAN_PROTECTION_TIME." ) diff --git a/pulpcore/app/viewsets/custom_filters.py b/pulpcore/app/viewsets/custom_filters.py index 72db8022db7..468802055d7 100644 --- a/pulpcore/app/viewsets/custom_filters.py +++ b/pulpcore/app/viewsets/custom_filters.py @@ -162,6 +162,53 @@ def filter(self, qs, value): """ raise NotImplementedError() + def get_base_version(self, field_name="base_version"): + """ + Resolve the companion ``base_version`` filter, if the request supplied one. + + The added/removed filters use this to diff against an *arbitrary* base repository + version instead of the filtered version's immediate predecessor. + + Args: + field_name (string): The name of the companion base-version filter on the filterset. + + Returns: + pulpcore.app.models.RepositoryVersion or None: The resolved base version, or None + when no base version was supplied. + """ + if self.parent is None: + return None + base_value = self.parent.form.cleaned_data.get(field_name) + if not base_value: + return None + return self.get_repository_version(base_value) + + +class RepositoryVersionBaseFilter(RepoVersionHrefPrnFilter): + """ + Companion filter that designates the base repository version for + ``repository_version_added`` / ``repository_version_removed`` diffs. + + On its own this filter does not alter the queryset. It is consumed by the added/removed + filters to compute the net difference between two arbitrary repository versions, rather than + the single-step difference against the filtered version's immediate predecessor. + """ + + def __init__(self, *args, **kwargs): + kwargs.setdefault( + "help_text", + _( + "Repository Version referenced by HREF/PRN to use as the base for " + "repository_version_added / repository_version_removed. When set, added/removed " + "content is computed relative to this version instead of the immediate predecessor." + ), + ) + super().__init__(*args, **kwargs) + + def filter(self, qs, value): + # No-op on its own; the value is consumed by the added/removed filters. + return qs + class RepositoryVersionFilter(RepoVersionHrefPrnFilter): """ @@ -251,7 +298,7 @@ def filter(self, qs, value): return qs repo_version = self.get_repository_version(value) - return qs.filter(pk__in=repo_version.added()) + return qs.filter(pk__in=repo_version.added(base_version=self.get_base_version())) class ContentRemovedRepositoryVersionFilter(RepoVersionHrefPrnFilter): @@ -273,7 +320,7 @@ def filter(self, qs, value): return qs repo_version = self.get_repository_version(value) - return qs.filter(pk__in=repo_version.removed()) + return qs.filter(pk__in=repo_version.removed(base_version=self.get_base_version())) class CharInFilter(BaseInFilter, CharFilter): diff --git a/pulpcore/tests/functional/api/using_plugin/test_repo_versions.py b/pulpcore/tests/functional/api/using_plugin/test_repo_versions.py index 0ad7d065380..08d639315ab 100644 --- a/pulpcore/tests/functional/api/using_plugin/test_repo_versions.py +++ b/pulpcore/tests/functional/api/using_plugin/test_repo_versions.py @@ -188,6 +188,82 @@ def test_add_remove_content( assert latest_version.content_summary.removed == {} +@pytest.fixture +def file_repo_three_versions( + file_bindings, + file_repository_factory, + file_9_contents, + monitor_task, +): + """Build a repo with three versions whose diffs span more than one step. + + Using the content units "A" through "E", the versions end up as:: + + v1: add A, B, C -> present {A, B, C} + v2: add D, remove A -> present {B, C, D} + v3: add E, remove B -> present {C, D, E} + + So a diff between the non-adjacent versions v1 and v3 differs from the single-step diff at v3. + """ + repo = file_repository_factory() + contents = file_9_contents + + def modify(add=(), remove=()): + body = { + "add_content_units": [contents[name].pulp_href for name in add], + "remove_content_units": [contents[name].pulp_href for name in remove], + } + monitor_task(file_bindings.RepositoriesFileApi.modify(repo.pulp_href, body).task) + return file_bindings.RepositoriesFileApi.read(repo.pulp_href).latest_version_href + + v1 = modify(add=["A", "B", "C"]) + v2 = modify(add=["D"], remove=["A"]) + v3 = modify(add=["E"], remove=["B"]) + return repo, v1, v2, v3 + + +def _relative_paths(list_response): + return {content.relative_path for content in list_response.results} + + +@pytest.mark.parallel +def test_repository_version_added_base_version(file_bindings, file_repo_three_versions): + """``base_version`` turns ``repository_version_added`` into a net diff between two versions.""" + _, v1, v2, v3 = file_repo_three_versions + + # Without base_version: single-step diff against the immediate predecessor (v2 -> v3). + single_step = file_bindings.ContentFilesApi.list(repository_version_added=v3) + assert _relative_paths(single_step) == {"E"} + + # With base_version=v1: net content in v3 but not in v1 => {C, D, E} - {A, B, C}. + net = file_bindings.ContentFilesApi.list(repository_version_added=v3, base_version=v1) + assert _relative_paths(net) == {"D", "E"} + + +@pytest.mark.parallel +def test_repository_version_removed_base_version(file_bindings, file_repo_three_versions): + """``base_version`` turns ``repository_version_removed`` into a net diff across versions.""" + _, v1, v2, v3 = file_repo_three_versions + + # Without base_version: single-step diff against the immediate predecessor (v2 -> v3). + single_step = file_bindings.ContentFilesApi.list(repository_version_removed=v3) + assert _relative_paths(single_step) == {"B"} + + # With base_version=v1: net content in v1 but not in v3 => {A, B, C} - {C, D, E}. + net = file_bindings.ContentFilesApi.list(repository_version_removed=v3, base_version=v1) + assert _relative_paths(net) == {"A", "B"} + + +@pytest.mark.parallel +def test_base_version_ignored_without_added_or_removed(file_bindings, file_repo_three_versions): + """``base_version`` has no effect unless paired with added/removed filters.""" + _, v1, v2, v3 = file_repo_three_versions + + present = file_bindings.ContentFilesApi.list(repository_version=v3) + present_with_base = file_bindings.ContentFilesApi.list(repository_version=v3, base_version=v1) + assert _relative_paths(present_with_base) == _relative_paths(present) == {"C", "D", "E"} + + @pytest.mark.parallel def test_add_remove_repo_version( file_bindings, From d26aa99a517835b2ab5ae7adf346b6cf1e869f33 Mon Sep 17 00:00:00 2001 From: Andrew Cheng Date: Tue, 14 Jul 2026 22:50:48 +0000 Subject: [PATCH 2/2] Mark content_ids as deferred and drop 65535 conditional for subquerying --- pulpcore/app/models/repository.py | 6 +----- pulpcore/app/viewsets/base.py | 13 +++++++++++-- pulpcore/app/viewsets/custom_filters.py | 7 +++++-- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/pulpcore/app/models/repository.py b/pulpcore/app/models/repository.py index 2fcdece61f8..1537449f9a7 100644 --- a/pulpcore/app/models/repository.py +++ b/pulpcore/app/models/repository.py @@ -997,11 +997,7 @@ def get_content(self, content_qs=None): if content_qs is None: content_qs = Content.objects - content_ids = self.content_ids - if len(content_ids) >= 65535: - # Workaround for PostgreSQL's limit on the number of parameters in a query - content_ids = self.content_ids_subquery() - return content_qs.filter(pk__in=content_ids) + return content_qs.filter(pk__in=self.content_ids_subquery()) def content_ids_subquery(self): """ diff --git a/pulpcore/app/viewsets/base.py b/pulpcore/app/viewsets/base.py index 7e09cc151d6..2f814b29ade 100644 --- a/pulpcore/app/viewsets/base.py +++ b/pulpcore/app/viewsets/base.py @@ -157,7 +157,7 @@ def get_resource_model(uri): return match.func.cls.queryset.model @staticmethod - def get_resource(uri, model=None): + def get_resource(uri, model=None, deferred_fields=None): """ Resolve a resource URI/PRN to an instance of the resource. @@ -168,6 +168,9 @@ def get_resource(uri, model=None): uri (str): A resource URI/PRN. model (django.models.Model): A model class. If not provided, the method automatically determines the used model from the resource URI/PRN. + deferred_fields (iterable): Optional field names to defer when loading the resource, so + large columns are not fetched from the database. Field names that do not exist on + the resolved model are ignored. Returns: django.models.Model: The resource fetched from the DB. @@ -207,7 +210,13 @@ def get_resource(uri, model=None): kwargs[key] = value try: - return model.objects.get(**kwargs) + manager = model.objects + if deferred_fields: + model_fields = {field.name for field in model._meta.concrete_fields} + to_defer = [name for name in deferred_fields if name in model_fields] + if to_defer: + manager = manager.defer(*to_defer) + return manager.get(**kwargs) except model.MultipleObjectsReturned: raise DRFValidationError( detail=_("URI {u} matches more than one {m}.").format( diff --git a/pulpcore/app/viewsets/custom_filters.py b/pulpcore/app/viewsets/custom_filters.py index 468802055d7..c8206a0b49e 100644 --- a/pulpcore/app/viewsets/custom_filters.py +++ b/pulpcore/app/viewsets/custom_filters.py @@ -145,9 +145,12 @@ def get_repository_version(value): detail=_("No value supplied for repository version filter") ) - object = NamedModelViewSet.get_resource(value) + # content_ids is only consumed as a database-side subquery (see + # RepositoryVersion.content_ids_subquery), so defer the potentially huge array column here + # to keep it from being loaded into Python when resolving the version. + object = NamedModelViewSet.get_resource(value, deferred_fields=("content_ids",)) if isinstance(object, Repository): - object = object.latest_version() + object = object.versions.complete().defer("content_ids").last() if not isinstance(object, RepositoryVersion): raise serializers.ValidationError( detail=_("URI {u} not found for {m}.").format(u=value, m="repositoryversion")