Repository navigation
Conversation
…ent_ids) The serve-from-repository-version file lookup (handler.py:963) and its pass-through twin (handler.py:897) filtered ContentArtifact with content__in=version.content, which compiles to content_id IN (SELECT ... unnest(content_ids) FROM core_repositoryversion ...) The planner cannot estimate unnest() row counts (hardcoded rows=10), so on every file request it materialized the version's whole content_ids array (hundreds of thousands of UUIDs on prod versions) and probed core_content and core_contentartifact per id, before the selective relative_path filter could apply. Work was O(version size), not O(result size). Compute membership from core_repositorycontent instead (version_added/removed interval, same semantics as _content_relationships), which has real table statistics and real indexes, and skips the redundant core_content existence hop. Generated-by: GLM 5.3 (EnMaaS)
…ISTS Even with the repositorycontent membership subquery, content_id IN (SELECT ...) still forces the planner to evaluate the membership side of the semi-join up front (O(version size)) before the LIMIT 1 file lookup can be satisfied, instead of starting from the selective relative_path index on core_contentartifact. Rewrite both file-lookup call sites as a correlated EXISTS on content_id: the planner can now scan core_contentartifact by relative_path (typically a handful of rows across the whole table) and probe membership per candidate row through the unique (repository_id, content_id, ...) index on core_repositorycontent. Work becomes O(matching contentartifacts), not O(version size). Generated-by: GLM 5.3 (EnMaaS)
…tent_ids) Replace the content__in=repo_version.content filter (per-request unnest of the whole content_ids array + core_content hop + rows=10 planner misestimate) with a correlated EXISTS over _content_relationships(), same pattern as the file-lookup fix (d516451, c1337df). Lets the planner drive the scan from the relative_path filter and probe repositoryversion membership per candidate row instead of materializing version membership up front. Generated-by: GLM 5.3 (EnMaaS)
A plain btree cannot serve LIKE 'prefix%' under a non-C collation (RDS default en_US.UTF-8), so directory-listing queries (relative_path__startswith) could not be index-driven on the relative_path filter. text_pattern_ops also supports equality, so it covers both the listing and the file-lookup path. Built CONCURRENTLY (25M+ rows on prod), following the 0148 precedent. Generated-by: GLM 5.3 (EnMaaS)
Two fixes for list_directory() on large repository versions: - Bound the number of entries rendered to DIRECTORY_LISTING_LIMIT (default 10000, 0 disables). Root listings of huge versions previously fetched and rendered every content artifact of the version; the queryset now scans at most limit+1 rows and a '…' marker entry signals truncation. - The last-modified dates loop iterated the entire version membership in Python to find the handful of listed contents; filter _content_relationships() by the listed content ids instead, so the query is bounded by the listing size, not the version size. Generated-by: GLM 5.3 (EnMaaS)
django.contrib.postgres.indexes exposes BTreeIndex (capital T), not BtreeIndex; the wrong name broke django.setup() at app startup. Generated-by: GLM 5.3 (EnMaaS)
PostgreSQL identifiers are capped at 32 characters for Django index names; core_contentartifact_relative_path_like (40) failed the system check. Rename to core_conten_relativ_like_idx (28), following Django's truncated-name convention. Generated-by: GLM 5.3 (EnMaaS)
The index.html existence probe runs before every serve-from-repo-version request. For file URLs like ".../a-1.0.0.jar" the probe key is ".../a-1.0.0.jar/index.html", which can never exist, so on the hot file path the probe is a pure per-request DB round trip (prod: 2673 execs, 2.14 s avg against the read-only replica). Skip the probe when the request path has no trailing slash and its last segment contains a dot (file-ish). Directory URLs still probe, so the serve-index.html-instead-of-listing-directory behaviour is unchanged. A dotted directory name without a slash now takes the trailing-slash redirect from list_directory() and serves index.html on the redirected request - one extra redirect, same content. Generated-by: GLM 5.3 (EnMaaS)
For directory URLs the index.html existence probe still used content__in=repo_version.content, i.e. the unnest(content_ids) subquery: per-request materialization of the whole content_ids array and a hash semi-join against core_contentartifact, O(version size) on big repo versions. Rewrite as a correlated EXISTS on _content_relationships() (same pattern as the file-lookup call sites): the planner scans core_contentartifact by the relative_path index for "<dir>/index.html" and probes core_repositorycontent membership per candidate row. Generated-by: GLM 5.3 (EnMaaS)
Adjust directory metadata tests for repository membership querysets. Assisted-by: pi coding agent
Assisted-by: pi coding agent
Content membership for the listing query is derived via unnest(content_ids). That SRF has a hardcoded rows=10 estimate, so the planner picks nested-loop index probes that scale with the full version content array instead of the result set. Add content_pks_subquery() which derives content UUIDs from the core_repositorycontent (version_added, version_removed) interval via the existing _content_relationships(). get_content() uses it when USE_INTERVAL_CONTENT_QUERY is set; the default keeps the legacy unnest subquery so behavior is unchanged unless opted in. content_ids_subquery() is retained for added()/removed()/contains(). Generated-by: Qwen-3.8
Replace the USE_INTERVAL_CONTENT_QUERY opt-in switch with the standard A/B experiment pattern: a shared run_experiment() dispatcher picks one variant per call by configured probability, times it, and logs one structured line (event, exp_id, variant, duration_ms, correlation_id). get_content() dispatches variant A (legacy unnest(content_ids) subquery) vs variant B (core_repositorycontent interval join) when EXPERIMENT_CONTENT_QUERY_P_CANDIDATE (env PULP_EXPERIMENT_CONTENT_QUERY_P_CANDIDATE) is set; default 0 keeps A for every call. Both subqueries carry /* pexp=PULP-XXXX v=A|B */ marker comments so the variants are separable in RDS Performance Insights Top SQL; application timing logs remain the source of truth. Add a unit test asserting variants A and B select identical content UUID sets across versions with adds and removes, plus a dispatch wiring test for probabilities 0 and 1. Generated-by: Qwen-3.8
Requested review coverage for the content query A/B variants: - interval query only returns membership for its own repository when another repository holds shared content - both variants return no content for an empty repository version Generated-by: Qwen-3.8
Per review: drop the EXPERIMENT_CONTENT_QUERY_P_CANDIDATE setting and dispatch through run_experiment() unconditionally with the named experiment id and p_candidate=0.5, so both variants are always benchmarked side by side per call. Rename the SQL marker comments from pexp=PULP-XXXX to pexp=PULP-1996-CONTENT-MEMBERSHIP so both variants stay separable in RDS Performance Insights Top SQL. Update the dispatch test: force each variant via monkeypatched random.random instead of the removed setting. Generated-by: Qwen-3.8
Generated-by: Qwen-3.8
…th_pattern_idx (PULP-2346) This combined branch carries the query changes only. The text_pattern_ops index on core_contentartifact.relative_path is created by pulp-service pulp#1642 (migration 0024, core_ca_relpath_pattern_idx), which merges first. Remove the model index and migration 0162 from the combined PRs, and add a changelog entry. The new query shapes need that index. Without it the directory-listing query scans all of core_contentartifact. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
1 of 7 tasks
This branch has not been deployed
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.
Do not merge: draft to run pulpcore CI
This depends on pulp-service #1642 (the index) merging first. The queries here only work as intended with the
core_ca_relpath_pattern_idxindex created by that PR. See "Dependency" below.Ticket: PULP-2346 (slow HEAD requests on the lightwell Java repositories), PULP-1996.
What this is
One branch that combines the pending read-only content query work, so CI and the unit tests run on all of it together:
get_content()interval-table A/B experiment).Per team policy the shipped fix is a patch in
pulp-service; this PR exists to get CI signal. The matching pulp-service PR carries the same changes as patches0080and0081.Dependency: the index comes from pulp-service #1642
core_ca_relpath_pattern_idx, atext_pattern_opsbtree oncore_contentartifact.relative_path, withCREATE INDEX CONCURRENTLY IF NOT EXISTS.core_conten_relativ_like_idx, migration0162) was removed so the index is created in one place, under one name.core_contentartifact(about 16.8 s on prod, measured by Elijah). Do not deploy this before Fix a formatting typo in the changelog #1642 is merged and its index build has finished on prod.What changed
index.htmlprobe for file paths; otherwise check membership with a correlatedEXISTScontent/handler.pyrelative_pathfirst, membership per match, capped atDIRECTORY_LISTING_LIMIT(10,000)content/handler.py,app/settings.pyEXISTSplusrelative_pathequalitycontent/handler.pycontent/handler.pyRepositoryVersion.get_content()runs the interval-table variant as an A/B experimentapp/models/repository.py,app/experiments.pytests/unit/...CHANGES/Problem
For a file path, the content handler asks the database three questions, each shaped as "find rows for this path among the repository version's items": Q1 is there an
index.html, Q2 does anything exist under the folderx.jar/, Q3 fetch the file. All three usedcontent__in=<version content>, which walks every item of the version.Measured on prod (read-only
EXPLAIN ANALYZE,java/validated, 1,592,450 items, path that does not exist): Q1 13.5 s, Q2 14.0 s, Q3 13.0 s. Together that is over Akamai's 30 s limit.Evidence so far (perf, Q2 only)
From #8249, which tested the same path-first query shape for the directory listing on a test database with repositories sized like the ticket's. Database time of one lookup, lower is better. "Before" is today's code; "Query change + index" is the shape in this branch with the index from #1642.
java/validated(1.59M items)org/codehaus/)Perf is smaller than prod (artifact table 3.5 GB against 22 GB, smaller instance), so this shows the effect, not prod timings. Full tables for the other repositories are in #8249.
Not tested yet
relative_pathstatistics differ from prod's, so its planner currently avoids the slow plan).get_content()experiment interacts with the handler changes.CREATE INDEX CONCURRENTLYduration and timeout behavior on prod's table (belongs to Fix a formatting typo in the changelog #1642).