Skip to content

Commit 8f20834

Browse files
author
Andrew Cheng
committed
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
1 parent b56b2a4 commit 8f20834

6 files changed

Lines changed: 173 additions & 19 deletions

File tree

CHANGES/7831.feature

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Added an optional ``base_version`` filter to the content list endpoints. When combined with
2+
``repository_version_added`` or ``repository_version_removed``, it returns the net set of content
3+
added or removed between two arbitrary repository versions instead of only the single-step
4+
difference against the filtered version's immediate predecessor.

pulpcore/app/models/repository.py

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -997,15 +997,24 @@ def get_content(self, content_qs=None):
997997
if content_qs is None:
998998
content_qs = Content.objects
999999

1000-
content_ids = self.content_ids
1001-
if len(content_ids) >= 65535:
1002-
# Workaround for PostgreSQL's limit on the number of parameters in a query
1003-
content_ids = (
1004-
RepositoryVersion.objects.filter(pk=self.pk)
1005-
.annotate(cids=Func(F("content_ids"), function="unnest"))
1006-
.values_list("cids", flat=True)
1007-
)
1008-
return content_qs.filter(pk__in=content_ids)
1000+
return content_qs.filter(pk__in=self.content_ids_subquery())
1001+
1002+
def content_ids_subquery(self):
1003+
"""
1004+
Return this version's ``content_ids`` as a database-side ``unnest`` subquery.
1005+
1006+
Using a subquery keeps the content unit UUIDs inside PostgreSQL instead of loading the
1007+
whole array into Python and passing each UUID as a bound query parameter. This avoids the
1008+
per-query parameter limit and the memory/serialization cost for large repository versions.
1009+
1010+
Returns:
1011+
django.db.models.QuerySet: A values queryset yielding the content unit UUIDs.
1012+
"""
1013+
return (
1014+
RepositoryVersion.objects.filter(pk=self.pk)
1015+
.annotate(cids=Func(F("content_ids"), function="unnest"))
1016+
.values_list("cids", flat=True)
1017+
)
10091018

10101019
@property
10111020
def content(self):
@@ -1119,8 +1128,8 @@ def added(self, base_version=None):
11191128
if not base_version:
11201129
return Content.objects.filter(version_memberships__version_added=self)
11211130

1122-
return Content.objects.filter(pk__in=self.content_ids).exclude(
1123-
pk__in=base_version.content_ids
1131+
return Content.objects.filter(pk__in=self.content_ids_subquery()).exclude(
1132+
pk__in=base_version.content_ids_subquery()
11241133
)
11251134

11261135
def removed(self, base_version=None):
@@ -1134,8 +1143,8 @@ def removed(self, base_version=None):
11341143
if not base_version:
11351144
return Content.objects.filter(version_memberships__version_removed=self)
11361145

1137-
return Content.objects.filter(pk__in=base_version.content_ids).exclude(
1138-
pk__in=self.content_ids
1146+
return Content.objects.filter(pk__in=base_version.content_ids_subquery()).exclude(
1147+
pk__in=self.content_ids_subquery()
11391148
)
11401149

11411150
def contains(self, content):

pulpcore/app/viewsets/base.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ def get_resource_model(uri):
157157
return match.func.cls.queryset.model
158158

159159
@staticmethod
160-
def get_resource(uri, model=None):
160+
def get_resource(uri, model=None, deferred_fields=None):
161161
"""
162162
Resolve a resource URI/PRN to an instance of the resource.
163163
@@ -168,6 +168,9 @@ def get_resource(uri, model=None):
168168
uri (str): A resource URI/PRN.
169169
model (django.models.Model): A model class. If not provided, the method automatically
170170
determines the used model from the resource URI/PRN.
171+
deferred_fields (iterable): Optional field names to defer when loading the resource, so
172+
large columns are not fetched from the database. Field names that do not exist on
173+
the resolved model are ignored.
171174
172175
Returns:
173176
django.models.Model: The resource fetched from the DB.
@@ -207,7 +210,13 @@ def get_resource(uri, model=None):
207210
kwargs[key] = value
208211

209212
try:
210-
return model.objects.get(**kwargs)
213+
manager = model.objects
214+
if deferred_fields:
215+
model_fields = {field.name for field in model._meta.concrete_fields}
216+
to_defer = [name for name in deferred_fields if name in model_fields]
217+
if to_defer:
218+
manager = manager.defer(*to_defer)
219+
return manager.get(**kwargs)
211220
except model.MultipleObjectsReturned:
212221
raise DRFValidationError(
213222
detail=_("URI {u} matches more than one {m}.").format(

pulpcore/app/viewsets/content.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
ContentAddedRepositoryVersionFilter,
2424
ContentRemovedRepositoryVersionFilter,
2525
ContentRepositoryVersionFilter,
26+
RepositoryVersionBaseFilter,
2627
)
2728

2829

@@ -125,6 +126,10 @@ class ContentFilter(BaseFilterSet):
125126
Return Content which was added in this repository version.
126127
repository_version_removed:
127128
Return Content which was removed from this repository version.
129+
base_version:
130+
When combined with repository_version_added / repository_version_removed, compute the
131+
net difference relative to this base repository version instead of the filtered
132+
version's immediate predecessor. Has no effect on its own.
128133
orphaned_for:
129134
Return Content which has been orphaned for a given number of minutes;
130135
-1 uses ORPHAN_PROTECTION_TIME value.
@@ -135,6 +140,7 @@ class ContentFilter(BaseFilterSet):
135140
repository_version = ContentRepositoryVersionFilter()
136141
repository_version_added = ContentAddedRepositoryVersionFilter()
137142
repository_version_removed = ContentRemovedRepositoryVersionFilter()
143+
base_version = RepositoryVersionBaseFilter()
138144
orphaned_for = OrphanedFilter(
139145
help_text="Minutes Content has been orphaned for. -1 uses ORPHAN_PROTECTION_TIME."
140146
)

pulpcore/app/viewsets/custom_filters.py

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,12 @@ def get_repository_version(value):
145145
detail=_("No value supplied for repository version filter")
146146
)
147147

148-
object = NamedModelViewSet.get_resource(value)
148+
# content_ids is only consumed as a database-side subquery (see
149+
# RepositoryVersion.content_ids_subquery), so defer the potentially huge array column here
150+
# to keep it from being loaded into Python when resolving the version.
151+
object = NamedModelViewSet.get_resource(value, deferred_fields=("content_ids",))
149152
if isinstance(object, Repository):
150-
object = object.latest_version()
153+
object = object.versions.complete().defer("content_ids").last()
151154
if not isinstance(object, RepositoryVersion):
152155
raise serializers.ValidationError(
153156
detail=_("URI {u} not found for {m}.").format(u=value, m="repositoryversion")
@@ -162,6 +165,53 @@ def filter(self, qs, value):
162165
"""
163166
raise NotImplementedError()
164167

168+
def get_base_version(self, field_name="base_version"):
169+
"""
170+
Resolve the companion ``base_version`` filter, if the request supplied one.
171+
172+
The added/removed filters use this to diff against an *arbitrary* base repository
173+
version instead of the filtered version's immediate predecessor.
174+
175+
Args:
176+
field_name (string): The name of the companion base-version filter on the filterset.
177+
178+
Returns:
179+
pulpcore.app.models.RepositoryVersion or None: The resolved base version, or None
180+
when no base version was supplied.
181+
"""
182+
if self.parent is None:
183+
return None
184+
base_value = self.parent.form.cleaned_data.get(field_name)
185+
if not base_value:
186+
return None
187+
return self.get_repository_version(base_value)
188+
189+
190+
class RepositoryVersionBaseFilter(RepoVersionHrefPrnFilter):
191+
"""
192+
Companion filter that designates the base repository version for
193+
``repository_version_added`` / ``repository_version_removed`` diffs.
194+
195+
On its own this filter does not alter the queryset. It is consumed by the added/removed
196+
filters to compute the net difference between two arbitrary repository versions, rather than
197+
the single-step difference against the filtered version's immediate predecessor.
198+
"""
199+
200+
def __init__(self, *args, **kwargs):
201+
kwargs.setdefault(
202+
"help_text",
203+
_(
204+
"Repository Version referenced by HREF/PRN to use as the base for "
205+
"repository_version_added / repository_version_removed. When set, added/removed "
206+
"content is computed relative to this version instead of the immediate predecessor."
207+
),
208+
)
209+
super().__init__(*args, **kwargs)
210+
211+
def filter(self, qs, value):
212+
# No-op on its own; the value is consumed by the added/removed filters.
213+
return qs
214+
165215

166216
class RepositoryVersionFilter(RepoVersionHrefPrnFilter):
167217
"""
@@ -251,7 +301,7 @@ def filter(self, qs, value):
251301
return qs
252302

253303
repo_version = self.get_repository_version(value)
254-
return qs.filter(pk__in=repo_version.added())
304+
return qs.filter(pk__in=repo_version.added(base_version=self.get_base_version()))
255305

256306

257307
class ContentRemovedRepositoryVersionFilter(RepoVersionHrefPrnFilter):
@@ -273,7 +323,7 @@ def filter(self, qs, value):
273323
return qs
274324

275325
repo_version = self.get_repository_version(value)
276-
return qs.filter(pk__in=repo_version.removed())
326+
return qs.filter(pk__in=repo_version.removed(base_version=self.get_base_version()))
277327

278328

279329
class CharInFilter(BaseInFilter, CharFilter):

pulpcore/tests/functional/api/using_plugin/test_repo_versions.py

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,82 @@ def test_add_remove_content(
188188
assert latest_version.content_summary.removed == {}
189189

190190

191+
@pytest.fixture
192+
def file_repo_three_versions(
193+
file_bindings,
194+
file_repository_factory,
195+
file_9_contents,
196+
monitor_task,
197+
):
198+
"""Build a repo with three versions whose diffs span more than one step.
199+
200+
Using the content units "A" through "E", the versions end up as::
201+
202+
v1: add A, B, C -> present {A, B, C}
203+
v2: add D, remove A -> present {B, C, D}
204+
v3: add E, remove B -> present {C, D, E}
205+
206+
So a diff between the non-adjacent versions v1 and v3 differs from the single-step diff at v3.
207+
"""
208+
repo = file_repository_factory()
209+
contents = file_9_contents
210+
211+
def modify(add=(), remove=()):
212+
body = {
213+
"add_content_units": [contents[name].pulp_href for name in add],
214+
"remove_content_units": [contents[name].pulp_href for name in remove],
215+
}
216+
monitor_task(file_bindings.RepositoriesFileApi.modify(repo.pulp_href, body).task)
217+
return file_bindings.RepositoriesFileApi.read(repo.pulp_href).latest_version_href
218+
219+
v1 = modify(add=["A", "B", "C"])
220+
v2 = modify(add=["D"], remove=["A"])
221+
v3 = modify(add=["E"], remove=["B"])
222+
return repo, v1, v2, v3
223+
224+
225+
def _relative_paths(list_response):
226+
return {content.relative_path for content in list_response.results}
227+
228+
229+
@pytest.mark.parallel
230+
def test_repository_version_added_base_version(file_bindings, file_repo_three_versions):
231+
"""``base_version`` turns ``repository_version_added`` into a net diff between two versions."""
232+
_, v1, v2, v3 = file_repo_three_versions
233+
234+
# Without base_version: single-step diff against the immediate predecessor (v2 -> v3).
235+
single_step = file_bindings.ContentFilesApi.list(repository_version_added=v3)
236+
assert _relative_paths(single_step) == {"E"}
237+
238+
# With base_version=v1: net content in v3 but not in v1 => {C, D, E} - {A, B, C}.
239+
net = file_bindings.ContentFilesApi.list(repository_version_added=v3, base_version=v1)
240+
assert _relative_paths(net) == {"D", "E"}
241+
242+
243+
@pytest.mark.parallel
244+
def test_repository_version_removed_base_version(file_bindings, file_repo_three_versions):
245+
"""``base_version`` turns ``repository_version_removed`` into a net diff across versions."""
246+
_, v1, v2, v3 = file_repo_three_versions
247+
248+
# Without base_version: single-step diff against the immediate predecessor (v2 -> v3).
249+
single_step = file_bindings.ContentFilesApi.list(repository_version_removed=v3)
250+
assert _relative_paths(single_step) == {"B"}
251+
252+
# With base_version=v1: net content in v1 but not in v3 => {A, B, C} - {C, D, E}.
253+
net = file_bindings.ContentFilesApi.list(repository_version_removed=v3, base_version=v1)
254+
assert _relative_paths(net) == {"A", "B"}
255+
256+
257+
@pytest.mark.parallel
258+
def test_base_version_ignored_without_added_or_removed(file_bindings, file_repo_three_versions):
259+
"""``base_version`` has no effect unless paired with added/removed filters."""
260+
_, v1, v2, v3 = file_repo_three_versions
261+
262+
present = file_bindings.ContentFilesApi.list(repository_version=v3)
263+
present_with_base = file_bindings.ContentFilesApi.list(repository_version=v3, base_version=v1)
264+
assert _relative_paths(present_with_base) == _relative_paths(present) == {"C", "D", "E"}
265+
266+
191267
@pytest.mark.parallel
192268
def test_add_remove_repo_version(
193269
file_bindings,

0 commit comments

Comments
 (0)