Skip to content

Add added_between/removed_between filters for content diffs across arbitrary repository versions#7844

Open
acheng-01 wants to merge 1 commit into
pulp:mainfrom
acheng-01:ac/add-base-version-parameter-for-repo-versions
Open

Add added_between/removed_between filters for content diffs across arbitrary repository versions#7844
acheng-01 wants to merge 1 commit into
pulp:mainfrom
acheng-01:ac/add-base-version-parameter-for-repo-versions

Conversation

@acheng-01

@acheng-01 acheng-01 commented Jul 2, 2026

Copy link
Copy Markdown

This PR adds two range filters to the content viewset, added_between and removed_between, that
compute the net content diff between two arbitrary (non-adjacent) repository versions, rather
than only the single-step diff against the immediate predecessor that repository_version_added
and repository_version_removed provide.

Each filter takes exactly two comma-separated Repository Version (or Repository) HREF/PRNs in
'base,target' order. added_between=base,target returns the content present in target but not in
base; removed_between=base,target returns the content present in base but not in target. The two
are symmetric: added_between=a,b is equivalent to removed_between=b,a.

Benchmarks on a synthetic 200k-unit system with 1000-unit diffs between repo versions show this
approach is ~5-15x faster than running an equivalent complex q query, 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. Planning time also decreases significantly and stays ~0.2 ms.

repository_version_added and repository_version_removed are unchanged and continue to compute the
single-step diff against the immediate predecessor, preserving backward compatibility.

Closes #7831

Assisted by: Claude Opus 4.8

📜 Checklist

  • Commits are cleanly separated with meaningful messages (simple features and bug fixes should be squashed to one commit)
  • A changelog entry or entries has been added for any significant changes
  • Follows the Pulp policy on AI Usage
  • (For new features) - User documentation and test coverage has been added

See: Pull Request Walkthrough

@acheng-01
acheng-01 force-pushed the ac/add-base-version-parameter-for-repo-versions branch from 3b22794 to ca2e6ba Compare July 6, 2026 18:54
@acheng-01
acheng-01 marked this pull request as ready for review July 6, 2026 18:54
@acheng-01
acheng-01 force-pushed the ac/add-base-version-parameter-for-repo-versions branch 2 times, most recently from 3c005b4 to 423500f Compare July 6, 2026 20:13
Comment thread pulpcore/app/models/repository.py
@dkliban

dkliban commented Jul 7, 2026

Copy link
Copy Markdown
Member

We have fixed a couple of DB performance issues by switching to using subqueries. I foresee us doing this more broadly throughout the codebase.

@acheng-01

acheng-01 commented Jul 7, 2026

Copy link
Copy Markdown
Author

Here are some added insights to some of the performance concerns raised. I'm mainly trying to answer these two questions:

  1. How the query shape differs when using q with subqueries vs base_version
  2. What is the performance like for q with subqueries vs base_version

Tl;dr - the q= filter can be optimized to the same performance as a dedicated filter as two end up as the same query, but it has a much larger blast radius and could have regressive consequences if we're not careful.

To review, q= is slow today (in our use case but maybe others too) because for "content in version X but not in base Y", repository_version=X AND NOT repository_version=Y compiles through the expression engine's set operations and becomes:

(X's content) INTERSECT ( (ALL content) EXCEPT (Y's content) )

_NotAction → qs.difference() (EXCEPT) and _AndActionqs.intersection() (INTERSECT). The NOT term's left operand is the whole table, so the plan is a HashSetOp Intersect over a HashSetOp Except that seq-scans the entire content table and sorts/hashes every leg, spilling to disk (temp read/written, external merge sorts). That full-table set-op work — not the pk__in sub-selects — is the cost.

If the existing q expression filter engine composed operands as pk__in subqueries instead of set-ops — i.e. _NotActionexclude(pk__in=…), _AndActionfilter(pk__in=…) — the plan collapses to a nested loop over an indexed anti-join (unnest(content_ids) → pkey Index Scan + NOT (hashed SubPlan)). No full-table seq scan, no SetOp , no disk spill. This is the same plan a dedicated base_version filter (filter(pk__in=X).exclude(pk__in=Y)) produces.

And here are some benchmarks comparing all three scenarios. These are measured on a 200k-unit dataset with 1000-unit diff between repo versions. Results are taken over a 3-run average.

Approach 199k-unit versions 50k-unit versions
q= today with set-ops ~1400 ms ~450 ms
q= with pk__in subqueries ~680 ms ~150 ms
base_version filter ~600 ms ~140 ms

As the results show, optimizing q= gets ~2× faster and lands within noise of a dedicated filter. The remaining time is inherent to the diff (unnesting/deduping ~199k ids and probing the index) rather than anything the query shape can avoid.

The worrying part is where we would implement the fix for q vs adding in base_version for our specific use case. The three evaluate methods (NotAction / _AndAction / _OrAction) are the generic q= engine used by every plugin and content type. Some specific concerns:

• The pk__in rewrite only holds if every operand is a pk-semijoinable queryset on the same model. If any sub-filter adds annotations/joins or filters a different model, .values("pk") wrapping is needed and must be validated — .difference() / .intersection() were chosen likely because they compose arbitrary querysets generically.
• Nested pk__in (… values("pk")) subqueries usually plan as semi/anti-joins, but it's unclear what would happen with deeply nested AND / OR / NOT queries. Pulp's docs say the limit is 8, which would mean a lot of nesting.
union()/intersection()/difference() return querysets with restricted downstream operations; the filter / exclude form actually lifts those restrictions, which is a behavior change (generally an improvement) to test.

As an aside, I've also explored removing the 65535 content unit conditional on get_content() before subquerying happens for the repository version table as a smaller change to all of q's operands. It will improve planning performance but won't affect execution performance because we'll still have the EXCEPT and INTERSECT operations that take up a bulk of the resources.

@acheng-01

acheng-01 commented Jul 7, 2026

Copy link
Copy Markdown
Author

I've opened a draft PR on what subqueries for q would look like. The existing filter tests appear to be passing with the changes. Let me know which direction would be better.

@mdellweg

Copy link
Copy Markdown
Member

copied from #7848; discussion should really happen here.

Given the speedups we see here, I'd say we most definitely want this. Looks like a great piece of work. (I'm almost inclined to keep this in limbo for another week to unlock the next level of significant speedup of the parsing. Not actually!) Too bad, it doesn't sweepingly address your initial issue. Do you believe we could add a dedicated filter receiving two versions returning the set-difference (maybe that would "just" boil down to merging the base_version into the original 'version_added' filter)?
Drafting from the top of my head (and using a terrible name): added_between=/p/a/v3/r/v/3/,/p/a/v3/r/v/6/
Maybe this is helping (probably only as a blueprint, not as an implementation): https://django-filter.readthedocs.io/en/stable/ref/filters.html#django_filters.filters.BaseRangeFilter

Thanks! I'm glad that this turned out well.

Regarding our initial issue, as you mentioned I think what you're proposing is actually what my base_filter PR already does. If we were to go the added_between / removed_between route, it'd really be an alternative packaging of the same logic because both ultimately call RepositoryVersion.added()/removed() with a base version.

I did a bit of research on BaseRangeFilter, and I believe its contribution here is thin: • It just splits "a,b" into two values and validates the count; its default range lookup (SQL BETWEEN) doesn't apply, so we'd override .filter() anyway. • We'd still resolve each href/PRN via the existing get_repository_version() (called twice) and need two new filters.

* Its only positive is that it binds the two versions together, so the current "base alone is a no-op" scenario becomes impossible.

Since #7844 is more or less ready and tested, I would personally lean toward keeping base_version and, if the silent no-op is the concern, I could add a validation error when base_version is passed without repository_version_added / removed. If you prefer the single-param ergonomics then I can add added_between / removed_between to it. Lmk!

Exactly. That's what I meant with blueprint. The implementation of the RangeFilter was never going to help us.
The "single-param egonomics" (nicely phrased!) is my personal preference for the api design (after all, to go full circle, it would allow sophisticated use of the filter in the "q-Language"). But I really want others to weigh in.

@acheng-01

Copy link
Copy Markdown
Author

@gerrod3 thoughts on the comment @mdellweg copied over?

@acheng-01
acheng-01 force-pushed the ac/add-base-version-parameter-for-repo-versions branch from d26aa99 to 8f20834 Compare July 17, 2026 22:03
@gerrod3

gerrod3 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

My preference is always for designs that are more explicit in their behavior and explanation. So in this case I think added_between and removed_between are the more intuitive filters.

@acheng-01

Copy link
Copy Markdown
Author

My preference is always for designs that are more explicit in their behavior and explanation. So in this case I think added_between and removed_between are the more intuitive filters.

Yeah, after I patched in the existing changes I realized that this is actually probably better, because repository_version_added and repository_version_removed basically have the same effect depending on the order of the repository version you feed either of those fields and base_version. Will work on using added_between and removed_between for the next commit.

…bitrary repository versions

This PR adds two range filters to the content viewset, added_between and removed_between, that
compute the net content diff between two arbitrary (non-adjacent) repository versions, rather
than only the single-step diff against the immediate predecessor that repository_version_added
and repository_version_removed provide.

Each filter takes exactly two comma-separated Repository Version (or Repository) HREF/PRNs in
'base,target' order. added_between=base,target returns the content present in target but not in
base; removed_between=base,target returns the content present in base but not in target. The two
are symmetric: added_between=a,b is equivalent to removed_between=b,a.

Benchmarks on a synthetic 200k-unit system with 1000-unit diffs between repo versions show this
approach is ~5-15x faster than running an equivalent complex q query, 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. Planning time also decreases significantly and stays ~0.2 ms.

repository_version_added and repository_version_removed are unchanged and continue to compute the
single-step diff against the immediate predecessor, preserving backward compatibility.

Closes pulp#7831

Assisted by: Claude Opus 4.8
@acheng-01
acheng-01 force-pushed the ac/add-base-version-parameter-for-repo-versions branch from 8f20834 to df3edea Compare July 21, 2026 21:13
@acheng-01 acheng-01 changed the title Add a base_version filter Add added_between/removed_between filters for content diffs across arbitrary repository versions Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Need a performant solution to find content diffs between non-adjacent repo versions

5 participants