Repository navigation
Run the read-only content queries as a PULP-2346 A/B experiment - #1655
Merged
Merged
Conversation
Re-add patch 0080 as an A/B experiment instead of a direct patch, without the PULP-1996 get_content() experiment (patch 0081). The content handler's four lookups (directory listing, index.html probe, exact file lookup and the pass-through file lookup) now go through content_query_experiment(): variant A is stock pulpcore (content__in=version.content), variant B checks repository membership per matching row with a correlated EXISTS and skips the index.html probe for file paths. The helper reuses run_experiment(), so each call logs duration_ms, variant, query, repository_id and result_count to the pulp.experiment logger, and tags the SQL with /* pexp=PULP-2346 v=A|B */ /* repo=<pk> */ for RDS Performance Insights. It is off by default; enable it with PULP_CONTENT_QUERY_AB_ENABLED and set the share with PULP_CONTENT_QUERY_AB_PROBABILITY (clowdapp.yaml). The 10,000 entry directory listing cap from the earlier patch is not included: both variants must return the same rows. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Contributor
Reviewer's GuideReintroduces the PULP-2346 read-only content query optimization as an always-on 50/50 A/B experiment, comparing pulpcore's stock membership query with a correlated EXISTS strategy while preserving results and adding detailed logs, SQL markers, image integration, and database-backed tests. Sequence diagram for the PULP-2346 content query experimentsequenceDiagram
participant Handler as ContentHandler
participant Experiment as content_query_experiment
participant Runner as run_experiment
participant DB as Database
Handler->>Experiment: content_query_experiment(query, version, queryset, filters, limit)
Experiment->>Runner: run_experiment(PULP-2346, run(A), run(B), p_candidate=0.5)
alt Variant A
Runner->>DB: Execute queryset.filter(content__in=version.content, **filters)
else Variant B
Runner->>DB: Execute queryset.filter(Exists(membership), **filters)
end
DB-->>Runner: Rows
Runner-->>Experiment: Rows and experiment metadata
Experiment-->>Handler: rows, used queryset
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="pulp_service/pulp_service/app/experiments.py" line_range="161" />
<code_context>
+ if variant == "A" or not settings_enabled():
+ return queryset.filter(content__in=version.content, **filters)
+ if skip_candidate:
+ return queryset.none()
+ membership = version._content_relationships().filter(content_id=OuterRef("content_id"))
+ return queryset.filter(Exists(membership), **filters)
</code_context>
<issue_to_address>
**Dotted directories fail to serve index pages**
When variant B is selected for a dotted directory requested without a trailing slash that contains `index.html`, `skip_candidate` makes the candidate query return no rows, so the handler falls through to an exact-file lookup and returns the wrong content or a missing-file response instead of serving the index page.
Keep the candidate index probe unless the path is known to be a file.
</issue_to_address>
### Comment 2
<location path="pulp_service/pulp_service/app/experiments.py" line_range="173" />
<code_context>
+ with connections[used["queryset"].db].execute_wrapper(
+ _marked_content_query(variant, version.repository_id)
+ ):
+ return list(used["queryset"] if limit is None else used["queryset"][:limit])
+
+ return evaluate
</code_context>
<issue_to_address>
**Disabled mode adds query overhead**
When the experiment is disabled, as it is by default, the disabled path still sends the index and file lookups through `sync_to_async(content_query_experiment)`, which evaluates them with `list()` rather than the original async `.aexists()` and `.aget()` calls. Every such lookup pays the sync-dispatch and row-materialization cost even when the experiment is off, adding latency to normal content requests.
Keep the original async `.aexists()`/`.aget()` path for disabled mode and use the instrumented synchronous evaluation only when the experiment is enabled.
</issue_to_address>
### Comment 3
<location path="pulp_service/pulp_service/app/experiments.py" line_range="173" />
<code_context>
+ with connections[used["queryset"].db].execute_wrapper(
+ _marked_content_query(variant, version.repository_id)
+ ):
+ return list(used["queryset"] if limit is None else used["queryset"][:limit])
+
+ return evaluate
</code_context>
<issue_to_address>
**Index probes fetch full rows**
When the PULP-2346 experiment is enabled and the content handler performs an index probe, the handler’s index probe only needs to know whether a matching artifact exists, but `content_query_experiment` evaluates it with `list()`, fetching and constructing a full `ContentArtifact` row. This replaces the prior existence-only query and adds avoidable database and Python work to index probes while the experiment is enabled.
Provide an existence-only evaluation path for index probes so the helper does not fetch and instantiate a full artifact row when only presence is needed.
</issue_to_address>
### Comment 4
<location path="pulp_service/pulp_service/tests/unit/test_content_query_experiment.py" line_range="70" />
<code_context>
+ "test", version, ContentArtifact.objects.select_related("artifact"), filters=filters, limit=limit
+ )
+ results[variant] = sorted(row.relative_path for row in rows)
+ assert results["A"] == results["B"]
+
+
</code_context>
<issue_to_address>
**Membership regressions pass parity checks**
When both variants return the same incorrect rows, or variant B uses the stock query, `test_variants_return_same_rows` only compares the two result lists at line 70, and the SQL-marker test only checks the comment at line 129. If both variants return artifacts outside the repository—or variant B uses the stock query instead of `EXISTS`—these checks still pass, including the non-member file lookup case.
Assert expected rows for each lookup (especially that the non-member lookup is empty) and verify that variant B emits the correlated membership predicate.
Also at `pulp_service/pulp_service/tests/unit/test_content_query_experiment.py:129`.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 4 findings to address first, and when enabled, the candidate query determines which artifacts are served for a repository; an incorrect membership match could expose content that is not in that repository. Disabling or reverting stops further requests, but cannot undo content already accessed.
Blocking findings: pulp_service/pulp_service/app/experiments.py:161, pulp_service/pulp_service/app/experiments.py:173, pulp_service/pulp_service/app/experiments.py:173, pulp_service/pulp_service/tests/unit/test_content_query_experiment.py:70
kdelee
reviewed
Oct 29, 2026
| value: "" | ||
| - name: PULP_CONTENT_QUERY_AB_ENABLED | ||
| description: Enable the PULP-2346 read-only content query A/B experiment (content app). | ||
| value: "false" |
Set PULP_CONTENT_QUERY_AB_ENABLED to "true" in clowdapp.yaml so the experiment runs once deployed (50% variant B). It can still be switched off with PULP_CONTENT_QUERY_AB_ENABLED=false, or ramped with PULP_CONTENT_QUERY_AB_PROBABILITY. Update the changelog fragment to match. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Contributor
Author
How to re-run the A/B timings on prod (read-only)Reproduces the variant A (stock) against variant B (new) table in the description. Everything sent to prod is a The queries are compiled with the ORM, in a pod running the real pulpcore, and then timed on prod. Do not hand-write the SQL: a simplified query once left out the Setupexport GABI_URL="https://<prod gabi route>/query" # not committed anywhere
export PERF_CONTEXT="<oc context of a cluster with a pulp-api pod running pulpcore 3.122.1>"
export PERF_NS="<namespace of that pod>"
export PYTHONPATH="$PWD" # directory holding the scripts below
oc login ... # prod cluster (for GABI) and the pod's cluster1. Find the repositories (SELECTs on prod)python3 make_specs.py lightwell java/validated java/remediated > specs.json2. Compile the exact SQL of every lookup and variant (no execution)Runs the helper from this branch ( POD=$(oc --context=$PERF_CONTEXT get pods -n $PERF_NS -o name | grep pulp-api | head -1)
EXPSRC="$(cat pulp_service/pulp_service/app/experiments.py)" SPECS="$(cat specs.json)" \
oc --context=$PERF_CONTEXT exec -i -n $PERF_NS $POD -c pulp-api -- \
env EXPSRC="$EXPSRC" SPECS="$SPECS" python3 - < compile_sql_pod.py | grep '^JSON:' | sed 's/^JSON://' > sql.json3. Time each variant on prod (read-only)python3 time_sql.py sql.json --passes 2 # all cases, A and B
python3 time_sql.py sql.json --passes 1 --only "file lookup" # one casePrints the last pass per case: time of A, time of B, and the number of rows each returned (they must match). The cases
Notes
gabi_lib.py"""Minimal read-only GABI client. Needs GABI_URL (the .../query route) and an `oc` login to the cluster."""
import json
import os
import subprocess
GABI_URL = os.environ["GABI_URL"]
def gq(sql, timeout=280):
token = subprocess.run(["oc", "whoami", "-t"], capture_output=True, text=True).stdout.strip()
out = subprocess.run(
["curl", "-s", "-m", str(timeout), "-X", "POST", GABI_URL, "-H", f"Authorization: Bearer {token}",
"-H", "Content-Type: application/json", "-d", json.dumps({"query": sql})],
capture_output=True, text=True,
).stdout
try:
return json.loads(out)
except ValueError:
return {"error": out[:200]} # e.g. an HTML login page when the oc token has expiredmake_specs.py"""Usage: make_specs.py DOMAIN BASE_PATH [BASE_PATH ...] > specs.json (SELECTs only)"""
import json
import sys
from gabi_lib import gq
domain, base_paths = sys.argv[1], sys.argv[2:]
specs = []
for base_path in base_paths:
rows = gq(f"SELECT d.repository_id FROM core_distribution d JOIN core_domain dm ON dm.pulp_id = d.pulp_domain_id "
f"WHERE dm.name = '{domain}' AND d.base_path = '{base_path}'")["result"][1:]
rid = rows[0][0]
vid, number, items = gq(f"SELECT pulp_id, number, array_length(content_ids, 1) FROM core_repositoryversion "
f"WHERE repository_id = '{rid}' ORDER BY number DESC LIMIT 1")["result"][1]
member = (f"EXISTS (SELECT 1 FROM core_repositorycontent r JOIN core_repositoryversion a ON r.version_added_id = a.pulp_id "
f"LEFT JOIN core_repositoryversion x ON r.version_removed_id = x.pulp_id WHERE r.content_id = ca.content_id "
f"AND r.repository_id = '{rid}' AND a.number <= {number} AND NOT (x.number <= {number} AND x.number IS NOT NULL))")
sample = None
for like in ("org/jboss/%", "org/%", "%"): # any existing .jar, to build existing-file and existing-folder cases
res = gq(f"SELECT ca.relative_path FROM core_contentartifact ca WHERE {member} AND ca.relative_path LIKE '{like}' "
f"AND ca.relative_path LIKE '%.jar' LIMIT 1")["result"]
if len(res) > 1:
sample = res[1][0]
break
specs.append({"name": base_path.rsplit("/", 1)[-1], "rid": rid, "vid": vid, "n": int(number), "items": items,
"ex_file": sample, "ex_folder": sample.rsplit("/", 1)[0] + "/", "broad": ["org/apache/", "org/eclipse/"]})
json.dump(specs, sys.stdout, indent=1)compile_sql_pod.py"""Run inside a pod with the real pulpcore (see the comment for the command). Prints the exact ORM SQL of every
lookup x variant as JSON, without executing it. Env: EXPSRC (experiments.py text), SPECS (specs.json text)."""
import contextlib
import json
import logging
import os
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "pulpcore.app.settings")
import django
django.setup()
from django.db import connections
from pulpcore.app.models import ContentArtifact, RepositoryVersion
ns = {"__name__": "pulp_service.app.experiments"}
exec(compile(os.environ["EXPSRC"], "experiments.py", "exec"), ns)
ns["logger"].setLevel(logging.CRITICAL)
class Captured(Exception):
def __init__(self, sql, params):
self.sql, self.params = sql, params
def capture(execute, sql, params, many, context):
raise Captured(sql, params) # stop before the statement runs
for conn in connections.all(): # open the connection first, or Django's one-time hstore setup query is captured instead
conn.ensure_connection()
with conn.cursor() as cursor:
cursor.execute("select 1")
def listing(): # the handler's directory listing
return ContentArtifact.objects.select_related("artifact")
def lookup(): # the handler's exact file lookup
return ContentArtifact.objects.select_related("artifact", "artifact__pulp_domain")
def probe(): # the handler's index.html probe only needs to know whether a row exists
return ContentArtifact.objects.values_list("pk", flat=True)
out = []
for s in json.loads(os.environ["SPECS"]):
version = RepositoryVersion(pulp_id=s["vid"], repository_id=s["rid"], number=s["n"])
cases = [
("listing: existing folder", "directory_listing", listing, {"relative_path__startswith": s["ex_folder"]}, None),
("listing: missing folder", "directory_listing", listing, {"relative_path__startswith": "org/nope/missing.jar/"}, None),
]
cases += [(f"listing: broad {b}", "directory_listing", listing, {"relative_path__startswith": b}, None) for b in s["broad"]]
cases += [
("index probe: file path", "index_probe", probe, {"relative_path": "org/nope/missing.jar/index.html"}, 1),
("index probe: folder", "index_probe", probe, {"relative_path": s["ex_folder"] + "index.html"}, 1),
("file lookup: existing", "file_lookup", lookup, {"relative_path": s["ex_file"]}, 2),
("file lookup: missing", "file_lookup", lookup, {"relative_path": "org/nope/missing.jar"}, 2),
]
for label, query, base, filters, limit in cases:
for variant in ("A", "B"):
ns["random"].random = lambda v=variant: 0.1 if v == "B" else 0.9 # < 0.5 selects B
with contextlib.ExitStack() as stack:
for conn in connections.all():
stack.enter_context(conn.execute_wrapper(capture))
entry = {"repo": s["name"], "lookup": label, "variant": variant, "sql": None}
try:
ns["content_query_experiment"](query, version, base(), filters=filters, limit=limit)
except Captured as c:
entry.update(
sql=c.sql,
params=[p if isinstance(p, (int, float)) else str(p) for p in c.params],
marker=f"/* pexp=PULP-2346 v={variant} */ /* repo={s['rid']} */ ",
)
out.append(entry)
print("JSON:" + json.dumps(out))time_sql.py"""Usage: time_sql.py sql.json [--passes N] [--only TEXT]. Times each statement with read-only EXPLAIN (ANALYZE, BUFFERS)."""
import argparse
import json
import re
from gabi_lib import gq
ap = argparse.ArgumentParser()
ap.add_argument("sql_json")
ap.add_argument("--passes", type=int, default=2)
ap.add_argument("--only", default="")
args = ap.parse_args()
entries = [e for e in json.load(open(args.sql_json)) if args.only in e["lookup"] or args.only in e["repo"]]
def literal(value):
return str(value) if isinstance(value, (int, float)) else "'" + str(value).replace("'", "''") + "'"
def explain(entry):
params = iter(entry["params"])
sql = re.sub(r"%s", lambda m: literal(next(params)), entry["sql"].replace("%%", "\0")).replace("\0", "%")
result = gq("EXPLAIN (ANALYZE, BUFFERS) " + entry["marker"] + sql, 120)
if not result.get("result"):
return None, None
lines = [row[0] for row in result["result"][1:]]
millis = next(float(re.search(r"Execution Time: ([\d.]+)", l).group(1)) for l in lines if "Execution Time" in l)
return millis, int(re.search(r"rows=(\d+) loops", lines[0]).group(1)) # rows of the top plan node = result rows
timings = {}
for number in range(args.passes):
for e in entries:
timings[(e["repo"], e["lookup"], e["variant"], number)] = (0.0, 0) if e["sql"] is None else explain(e)
fmt = lambda t: "error" if t[0] is None else (f"{t[0]:.2f} ms" if t[0] < 1000 else f"{t[0] / 1000:.2f} s") # noqa: E731
last = args.passes - 1
for repo, lookup in dict.fromkeys((e["repo"], e["lookup"]) for e in entries):
a, b = timings[(repo, lookup, "A", last)], timings[(repo, lookup, "B", last)]
print(f"{repo:11} {lookup:34} A {fmt(a):>10} | B {fmt(b):>10} | rows A={a[1]} B={b[1]} {'SAME' if a[1] == b[1] else 'DIFFERENT'}") |
Contributor
Exact ORM SQL timing via local PR branch + GABIRan the PULP-2346 helper from commit Repositories:
One timing pass per case.
The result sets match for every completed case. Variant A repeatedly expands |
kdelee
approved these changes
Oct 29, 2026
Drop PULP_CONTENT_QUERY_AB_ENABLED and PULP_CONTENT_QUERY_AB_PROBABILITY from clowdapp.yaml and the helper: as long as patch 0080 is present the experiment runs, with a fixed 50% share for variant B. Remove the tests for the off switch and update the changelog fragment. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Contributor
Follow-up:sourcery review caught that we should not skip the cases that you were trying to because produces some false positives: Request that exposes bug: Final segment: It contains dots, but is directory name. Expected behavior: resolves to: Other validated examples: larrymou9#2 should fix that... After removing the dotted-path Captured exact ORM SQL from the local PR branch and timed it through production GABI:
Both statements use the The local regression suite passes: |
kdelee
suggested changes
Oct 29, 2026
kdelee
left a comment
kdelee
left a comment
Contributor
There was a problem hiding this comment.
I think you need to do something like larrymou9#2 (can merge that and we re-request review from sourcery)
Keep candidate index probes enabled for all paths so dotted directory names with index.html retain the same behavior as control. Add regression coverage for candidate index-probe execution. Assisted-by: pi coding agent
The previous commit removed three lines from the patch file by hand without updating the hunk header, so patch(1) failed with "malformed patch". Regenerate patch 0080 from the patched source so every hunk header is correct. Also address the Sourcery review: - remove the now unused skip_candidate parameter of content_query_experiment() - have the index probe select only primary keys instead of full ContentArtifact rows, since it only needs to know whether a match exists - assert the exact expected rows in the variant tests (including a ContentArtifact whose content is not in the version) and that variant A uses unnest while variant B uses EXISTS Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… skipped Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Contributor
|
@sourcery-ai review |
Contributor
|
Hi @kdelee! 👋 Only authors and team members can run @sourcery-ai commands on public repos. |
kdelee
approved these changes
Oct 30, 2026
Contributor
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="pulp_service/pulp_service/app/experiments.py" line_range="183" />
<code_context>
+ p_candidate=CONTENT_QUERY_P_CANDIDATE,
+ context={"query": query, "repository_id": str(version.repository_id)},
+ )
+ return rows, used["queryset"]
+
+
</code_context>
<issue_to_address>
**Derived listing query loses its marker**
When the directory timestamp experiment is enabled and its candidate path evaluates a source queryset from a directory listing, when the directory timestamp experiment evaluates `cas.values("content_id")`, it builds and executes a new queryset after this helper's `execute_wrapper` has exited. That SQL inherits the selected A/B membership filter but lacks the PULP-2346 marker and is excluded from its timing, so the variant's downstream query is not attributed to the experiment.
Ensure evaluations of derived source querysets are included in the PULP-2346 instrumentation, or avoid returning a lazy queryset whose later evaluation inherits the experiment variant.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and if variant B’s correlated membership check is wrong, a matching artifact outside the requested repository version could be served to a requester. Reverting restores the stock query, but cannot undo content already exposed.
Blocking findings: pulp_service/pulp_service/app/experiments.py:183
Assisted-by: pi coding agent
Preserve PULP-2346 markers on lazy directory queries
The queryset now carries the marker in its SQL (previous merge), so the execute-time prefix added a second copy to every statement that the helper runs. The experiment collector adds a digest's load once for each marker it finds, so those statements were reported twice. Drop the execute_wrapper prefix and assert that each executed statement has exactly one marker. Also document why the S610 suppression on QuerySet.extra() is safe: the marker is built from constants and a UUID-validated repository id. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Contributor
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="pulp_service/pulp_service/app/experiments.py" line_range="160" />
<code_context>
+
+ def build(variant):
+ if variant == "A":
+ selected = queryset.filter(content__in=version.content, **filters)
+ else:
+ membership = version._content_relationships().filter(content_id=OuterRef("content_id"))
</code_context>
<issue_to_address>
**Repository contents vary by variant**
When a repository version has a missing `content_ids` cache while its relationship rows contain members, variant A filters through `version.content`, which reads the cache, while variant B checks relationship rows directly; `build` therefore returns different artifacts depending on the randomly selected variant, so callers see inconsistent content responses.
Make both variants use the same membership source and semantics when the cache is missing.
Also at `pulp_service/pulp_service/app/experiments.py:162`.
</issue_to_address>
### Comment 2
<location path="pulp_service/pulp_service/app/experiments.py" line_range="171" />
<code_context>
+ def run(variant):
+ def evaluate():
+ used["queryset"] = build(variant)
+ return list(used["queryset"] if limit is None else used["queryset"][:limit])
+
+ return evaluate
</code_context>
<issue_to_address>
**Replica retries reuse the failed connection**
When a replica conflict or other database error occurs during one of the wrapped lookups, the content handler invokes this synchronous query through `sync_to_async`, so Django runs it on a worker thread. When it raises a database error, `_retry_content_artifact_query` resets connections in the async caller’s thread; the retry then runs on the worker thread with its failed connection still open and the request errors instead of recovering.
Perform the connection reset in the same synchronous worker thread as the query, or keep the query and retry on the async ORM path.
</issue_to_address>
### Comment 3
<location path="pulp_service/pulp_service/app/experiments.py" line_range="171" />
<code_context>
+ def run(variant):
+ def evaluate():
+ used["queryset"] = build(variant)
+ return list(used["queryset"] if limit is None else used["queryset"][:limit])
+
+ return evaluate
</code_context>
<issue_to_address>
**Large listings retain duplicate row references**
When a directory listing matches a very large number of content artifacts, `list(queryset)` fills Django’s queryset result cache and then copies its references into a second list; the directory handler also retains the queryset in `content_sources` while consuming the returned rows. For very large listings, both collections stay live during processing, increasing worker memory enough to risk exhausting it and interrupting the request.
Evaluate the selected queryset with an iterator so the returned rows do not coexist with a populated queryset result cache.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 3 findings to address first, and if the correlated membership check matches artifacts outside the repository version, variant B could serve those files to callers; any content exposed cannot be taken back. Reverting would stop future variant B queries but would not undo prior exposure.
Blocking findings: pulp_service/pulp_service/app/experiments.py:160, pulp_service/pulp_service/app/experiments.py:171, pulp_service/pulp_service/app/experiments.py:171
Member
|
/ok-to-test |
dkliban
approved these changes
Nov 2, 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.
Summary
Re-adds the PULP-2346 content query optimization (old patch 0080) as an A/B experiment instead of a direct patch. Jan's
get_content()experiment (0081) is not included.The content handler's four lookups (directory listing,
index.htmlprobe, exact file lookup, pass-through file lookup) now go throughcontent_query_experiment():content__in=version.content).EXISTS.It reuses
run_experiment(), so each call logsduration_ms, variant, query name,repository_idand result count (pulp.experimentlogger), and the SQL is tagged/* pexp=PULP-2346 v=A|B */ /* repo=<pk> */for RDS Performance Insights. Note that the tokenized Performance Insights view merges statements that differ only in the repo comment; the rawdb.sqlview keeps them apart.Always on: as long as the patch is present the experiment runs, with variant B on 50% of calls. There are no settings or
clowdapp.yamlparameters.The 10,000-entry directory listing cap from the old patch is not included, because both variants must return the same rows. The
index.htmlprobe is not skipped for dotted paths: the handler adds the trailing slash, so…/12.1.12probes…/12.1.12/index.html, which can exist. The probe selects only primary keys. Relies on thecore_ca_relpath_pattern_idxindex from #1642 (already live).Prod timings, variant A against variant B
Database time of one query, from read-only
EXPLAIN ANALYZEon prod through GABI, using the exact SQL the helper builds (compiled with the ORM, not hand-written).java/validatedversion 1837 (1.65M items) andjava/remediatedversion 252 (109K items). Second of two passes. Result rows were identical for all 16 lookups. A's times vary with load on the prod database (an earlier run was 25-30% lower); B was stable. How to re-run: see the comment below.validatedAvalidatedBremediatedAremediatedBindex.htmlprobe, file pathindex.htmlprobe, folderorg/apache/org/eclipse/A missing-file HEAD on
validatedruns three of these lookups: about 33 s with A (24 s in an earlier run) and under 0.2 ms with B. Broad folders still take seconds with B, andremediatedbroad folders do not improve.Testing
SELECTonly): both variants returned identical rows for 18 lookups on the three scale test repositories, with both SQL markers and the log fields present.ruff checkandruff format --checkpass.test_content_query_experiment.py) assert the exact expected rows for both variants (including an artifact whose content is not in the version), that A usesunnestand B usesEXISTS, the log fields and the SQL markers. Not run: they need a Pulp test database.Summary by Sourcery
Run read-only content queries through the always-on PULP-2346 A/B experiment to measure and compare optimized repository membership checks.
New Features:
Enhancements:
Build:
Tests: