Repository navigation
[PULP-2346] Use correlated EXISTS membership lookups and add relative_path index - #8300
Merged
Merged
Conversation
Contributor
|
Looking again at the experiement results in cloudwatch, The outliers we saw on day one have fallen off, now p99 on all repositories is looking much better!!! |
larrymou9
force-pushed
the
lrios/pulp-2346-interval-membership
branch
from
November 11, 2026 14:43
e6f1f3c to
c03c345
Compare
Contributor
|
@larrymou9 if you rebase and repush, the tests will pick up #8332 and should run clean |
Check repository version membership per matching ContentArtifact row with a correlated EXISTS over the RepositoryContent version interval, instead of content__in=repo_version.content (unnest of content_ids). This covers the content handler's directory listing, index.html probe, exact file lookup and pass-through file lookup. The index.html probe is existence-only (pk, LIMIT 1) and always runs, including for dotted final path segments, which can be directories. Add a text_pattern_ops btree index on ContentArtifact.relative_path, created concurrently, for the handler's prefix and equality lookups. The migration reuses an existing index only when it is valid and has the expected definition. Tracked in Jira as PULP-2346. Validated in a production A/B test. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
larrymou9
force-pushed
the
lrios/pulp-2346-interval-membership
branch
from
November 12, 2026 18:24
c03c345 to
507495a
Compare
dkliban
approved these changes
Nov 12, 2026
This was referenced Nov 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
This PR adds an index on
ContentArtifact.relative_pathand changes the content handler's lookups to use it (PULP-2346).What changes
core_ca_relpath_pattern_idx, atext_pattern_opsbtree onrelative_path. Our database usesen_US.UTF-8, where a default index can't serveLIKE 'prefix%'. Production already has this index from a downstream migration, so the migration reuses it when it is valid and has the expected definition.index.htmlprobe and file lookups now check membership with a correlatedEXISTSoverRepositoryContent. They no longer usecontent__in=repo_version.content, which unnests the wholecontent_idsarray.Why: We ran this as an A/B experiment in production. B averaged 4.44 against 12.46 for A, and p99 was 8.96 against 260.9 (kdelee's Nov 4 numbers below).
Where I'd like review: The migration's handling of an already-existing index, and the test coverage. I used database-backed unit tests, because
pulp_filedistributions serve from publications and never reach these handler paths.Summary
Makes the validated PULP-2346 Variant B behavior permanent and adds the index it relies on.
index.htmlprobe, exact file lookup and pass-through file lookup check membership per matchingContentArtifactrow with a correlatedEXISTSover theRepositoryContentinterval, rather thancontent__in=repo_version.content.index.htmlprobe is existence-only (values_list("pk", flat=True)[:1]) and always runs, including for dotted final path segments such as12.1.12, which can be directories.0162addscore_ca_relpath_pattern_idx(text_pattern_opsoncore_contentartifact.relative_path) withCREATE INDEX CONCURRENTLY IF NOT EXISTS. It reuses an existing index only if it is valid and has exactly the expected definition (btree onrelative_pathwithtext_pattern_ops); an invalid or differently defined index of the same name makes the migration fail instead of being reused or dropped, and the reverse migration only drops an index it created (tracked via an index comment). Model state declares the same index, somakemigrations --checkstays clean.Why
text_pattern_opsrelative_path LIKE 'prefix%'(Djangostartswith), and exact lookups use=.en_US.UTF-8, not theClocale, so Postgres can't turnLIKE 'prefix%'into a range scan on a default index.text_pattern_opscompares strictly character by character, so a prefix becomes one contiguous range andLIKE 'prefix%'becomes an index range scan. Equality can use it too.<,<=,>and>=cannot, but the existing default index still covers those. See the PostgreSQL docs.ILIKE, a leading wildcard or unanchored patterns. Our lookups are case-sensitive and anchored, so it fits.Checked on the production read replica via gabi:
datcollate = en_US.UTF-8,datctype = en_US.UTF-8, encodingUTF8. The existingcore_ca_relpath_pattern_idxis valid and is 2032 MB (2,130,673,664 bytes).Indexes on
core_contentartifacton the production read replica, largest first (partial list, as returned by gabi):core_contentartifact_artifact_id_f5d9a66bcore_contentartifact_content_id_04d8a3d5core_contentartifact_pkeycore_conten_relativ_ca1ae5_idxcore_ca_relpath_pattern_idx(this PR)Production A/B results
Variant A is the previous code, variant B is this change, dispatched 50/50 per call in production. Window: 2026-11-01 20:29:38 to 2026-11-02 20:29:38 UTC (24 hours). Values are exactly as emitted by our CloudWatch collector (the output does not state a unit); only the columns and the repository ordering are unchanged.
Overall
B is better at p50, p95 and p99 overall.
PULP-2346, repository by repository
Four repositories with meaningful sample sizes improved on average by 57-92%. Three show B slower on average (+122%, +122%, +4.9%); they are discussed below.
Update from the 2026-11-04 results (kdelee's PR comment)
Later CloudWatch results from the same experiment, with many more samples. The comment does not state the time window. The Nov 2 tables above are kept unchanged. The PULP-2505 row in that comment is a separate experiment and is omitted here.
Overall:
Repository by repository:
Reading the table: B has the lower average and the lower p99 on all six repositories. B's p50 is slightly higher on two (
019fd363: 4.5008 vs 4.0041, and019f42bb: 4.412 vs 3.543, which has only 4 samples per variant). Repository019f3d82(1 A / 1 B on Nov 2) no longer appears in the table.Outliers and caveats
Three repositories show B slower on average, all with very small samples:
019f3818(13 A / 10 B): avg 74.3 -> 165.1, p95/p99 308 -> 1596. One slow B call accounts for about 97% of B's total; the other nine average roughly 6.1, and the medians are flat (4.44 vs 4.355). We reran the same queries many times and could not reproduce it. We suspect an unrelated circumstance but have not proven it.019f3d82(1 A / 1 B) and019f42bb(3 A / 1 B): too few samples to conclude anything.p99 on small per-repository samples is fragile in both directions: A also has p99 outliers (838 and 606) that B does not.
Update (Nov 4): with more samples,
019f3818has 279 A / 273 B calls, B averages 4.9693 against 26.6893 for A, and p99 is 11.221 against 297.7197. See the Nov 4 tables above. The Nov 2 reading above is kept for transparency.Changelog
CHANGES/+optimize-content-membership-queries.bugfixCo-Authored-By: Claude Sonnet 5