fix: cache terminal local content misses by decko · Pull Request #8080 · pulp/pulpcore

decko · 2026-09-09T16:33:19Z

Summary

Cache eligible terminal local 404s in the existing content cache to avoid repeated content-resolution work. Fixes the repeated-miss latency described in #8078.

Changes

Cache only terminal local PathNotResolved failures after authorization.
Reuse the existing cache key, TTL, and lifecycle invalidation.
Preserve uncached remote, unmatched, guarded-denied, and unmarked 404 behavior.
Add unit coverage and a changelog entry.

Testing

Unit suite: 444 passed, 2 skipped.
Maven load test: 100,000 artifacts; 200 requests at concurrency 32; warm 404 p50/p95/p99/max: 80/139/143/143 ms.

Closes #8078

decko · 2026-09-09T20:27:50Z

Maven benchmark

Compared baseline d78a8ba25 with this PR in isolated oci-env agent environments. Redis was enabled and PostgreSQL statistics were refreshed.

100,000 Maven artifacts
100 repository versions
Repository-backed Maven distribution
No remote or publication
200 parallel curl requests, concurrency 32, for the same missing Maven JAR

Version	Result	p50	p95	p99	Max
Baseline	404	24.19 s	34.70 s	36.66 s	36.90 s
This PR	404	0.165 s	0.259 s	0.315 s	0.318 s

The first candidate request returned 404 with X-PULP-CACHE: MISS; subsequent requests returned X-PULP-CACHE: HIT.

For invalidation, I added the missing Maven artifact, created a new repository version, and moved the distribution to it. The next request immediately returned 200 with X-PULP-CACHE: MISS.

pg_stat_statements was unavailable, so query-level reduction was not measured. The agents and synthetic data were destroyed after testing.

Assisted-by: GPT 5.6 Luna
Assisted-by: GPT 5.6 Terra

decko · 2026-09-09T20:48:23Z

Thanks for the suggestion. I agree that broader 404 caching deserves discussion, including:

Remote 404s: could reduce repeated work, but may hide content when an upstream recovers or publishes it.
Unmatched distributions: useful for invalid-path scans, but would require a separate discovery cache and broader invalidation.
Guarded requests: authorization must still run first; custom handlers may return identity- or header-dependent 404s.
Generic 404 caching: risks caching plugin-specific, remote, or transient failures.

The current design caches only terminal local misses because they are the safest, incident-relevant case. Separate string keys provide independent TTLs and avoid changing positive-cache expiry.

A simpler option is to remove generation invalidation and accept that newly available content may remain cached as 404 until the TTL expires. That reduces implementation complexity but does not meet the immediate-availability requirement.

The main points for discussion are:

Which 404 categories are semantically safe to cache?
Is a bounded stale window acceptable?
Do we require immediate visibility after repository changes?
Should request coalescing be addressed separately from negative caching?

Assisted-by: GPT 5.6 Luna/Terra

decko · 2026-09-10T15:28:58Z

Load test

Isolated oci-env agent with 100,000 Maven artifacts. Repository-backed distribution; no remote or publication.

200 parallel curl requests at concurrency 32 for one missing Maven JAR:

Result	p50	p95	p99	Max
404 cache hit	80 ms	139 ms	143 ms	143 ms

The first request was 404, X-PULP-CACHE: MISS, in 45 ms. Five follow-ups were 404, X-PULP-CACHE: HIT, in 3–4 ms.

All agents and synthetic data were removed.

Assisted-by: GPT 5.6 Luna/Terra

gerrod3

Pretty sure this isn't working the way you expect it to. The HTTPNotFound shouldn't even be placed in the cache because there is no elif isinstance(response, HTTPNotFound) check in make_entry. So I'm not sure how your unit test is passing -- it is making me sus of our other cached unit tests...

gerrod3 · 2026-09-10T21:30:38Z

@decko You didn't finish adding my changes, please do so before requesting another review.

decko · 2026-09-11T13:54:48Z

@decko You didn't finish adding my changes, please do so before requesting another review.

My bad. Few things went missing, and the agent got stuck with oci-env.
I believe we implemented all your requested changes.

Can you take another look?

decko · 2026-09-11T16:38:55Z

Maven load test

Compared the PR with its direct parent (d78a8ba25) using fresh agents, 100,000 Maven artifacts, and a repository-version-backed distribution. The exact missing-JAR content URL was requested 200 times at concurrency 32.

The PR agent Redis cache was cleared, then primed with 404/MISS followed by 404/HIT. All PR benchmark requests were cache hits. The baseline returned no cache header.

Version	Result	p50	p95	p99	Max	RPS
Parent	404, uncached	23.22 s	32.08 s	34.26 s	34.76 s	1.43
PR	404, cache hit	25 ms	54 ms	61 ms	71 ms	92.2

PR result: 200/200 responses were 404 with X-PULP-CACHE: HIT.

gerrod3

Figured out why the code was working before, tests were correct, just my understanding was wrong.

gerrod3 · 2026-09-11T16:42:56Z

+            else:
+                entry["text"] = getattr(body, "_value", body).decode("utf-8")
+            entry["type"] = "HTTPNotFound"
        elif isinstance(response, (Response, HTTPSuccessful)):


After examining this code I have come to the realization that Response is the base class of all HttpException classes, therefore the elif isinstance(response, HTTPFound) is dead code. Now the code still works since we store all the information to create the redirect and HTTPFound is ultimately just a fancy constructor to make a Response.

So now my thinking is that instead of creating a new block for HTTPNotFound we just move it into the Response block. Add an inside check that the response is cacheable and fall through to the reset of the code.

So now my thinking is that instead of creating a new block for HTTPNotFound we just move it into the Response block. Add an inside check that the response is cacheable and fall through to the reset of the code.

It's possible, but IMO, it will make the code less clear about its intentions. I would keep the HTTPNotFound where it is, move the HTTPFound block up in the tree, and leave the elif isinstance(response, (Response, HTTPSuccessful) as a fallback option before returning what is non-treatable anyway.

It's up to you.

That's also fine, being clear is always good. Just note that The HTTPFound will also need to pop the status.

decko · 2026-09-11T17:12:18Z

Figured out why the code was working before, tests were correct, just my understanding was wrong.

The previous test was definitely wrong. Mocked a handler and forced a raise. The current functional test is testing the real code flow now.

Closes: PULP-2367 Closes pulp#8078 Assisted-by: GPT 5.6 Luna Assisted-by: GPT 5.6 Terra Assisted-by: Claude Opus 4.6 Assisted-by: GPT 5 Mini Assisted-by: OpenAI GPT-5.6-terra

gerrod3

Almost there.

gerrod3 · 2026-09-11T21:25:34Z

+    first = get_from_url(url)
+    second = get_from_url(url)
+    assert first.status == second.status == 404
+    assert "X-PULP-CACHE" not in first.headers
+    assert "X-PULP-CACHE" not in second.headers


Can't we use the helper method assert_cache_status(url, [(404, None), (404, None)])

gerrod3 · 2026-09-11T21:27:25Z

+    remote = file_remote_factory(manifest_path=basic_manifest_path, policy="immediate")
+    body = FileRepositorySyncURL(remote=remote.pulp_href)
+    monitor_task(
+        file_bindings.RepositoriesFileApi.sync(file_repo_with_auto_publish.pulp_href, body).task
+    )


This remote and sync are not used, remove them.

github-actions Bot added the no-issue label Sep 9, 2026

github-actions Bot added multi-commit no-changelog labels Sep 9, 2026

decko force-pushed the pulp-2367-negative-404-cache branch from 37e3e8e to 75c507c Compare September 10, 2026 14:47

github-actions Bot removed multi-commit no-changelog labels Sep 10, 2026

decko marked this pull request as ready for review September 10, 2026 15:31

decko force-pushed the pulp-2367-negative-404-cache branch 2 times, most recently from b4b4fb9 to b7acc76 Compare September 10, 2026 16:49

gerrod3 requested changes Sep 10, 2026

View reviewed changes

decko force-pushed the pulp-2367-negative-404-cache branch from b7acc76 to 1c86b52 Compare September 10, 2026 20:58

decko requested a review from gerrod3 September 10, 2026 20:58

decko force-pushed the pulp-2367-negative-404-cache branch 2 times, most recently from 66cceae to 9307275 Compare September 11, 2026 12:50

gerrod3 requested changes Sep 11, 2026

View reviewed changes

decko force-pushed the pulp-2367-negative-404-cache branch from 9307275 to 6c91850 Compare September 11, 2026 17:23

gerrod3 requested changes Sep 11, 2026

View reviewed changes

Comment thread pulpcore/cache/cache.py

Comment thread pulpcore/cache/cache.py Outdated

fix: cache terminal local content misses

bbf26d9

Closes: PULP-2367 Closes pulp#8078 Assisted-by: GPT 5.6 Luna Assisted-by: GPT 5.6 Terra Assisted-by: Claude Opus 4.6 Assisted-by: GPT 5 Mini Assisted-by: OpenAI GPT-5.6-terra

decko force-pushed the pulp-2367-negative-404-cache branch from 6c91850 to bbf26d9 Compare September 11, 2026 19:53

gerrod3 requested changes Sep 11, 2026

View reviewed changes

Uh oh!

Conversation

decko commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Testing

Uh oh!

decko commented Sep 9, 2026

Maven benchmark

Uh oh!

decko commented Sep 9, 2026

Uh oh!

decko commented Sep 10, 2026

Load test

Uh oh!

gerrod3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gerrod3 commented Sep 10, 2026

Uh oh!

decko commented Sep 11, 2026

Uh oh!

decko commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Maven load test

Uh oh!

gerrod3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gerrod3 Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

decko Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

gerrod3 Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

decko commented Sep 11, 2026

Uh oh!

Uh oh!

Uh oh!

gerrod3 left a comment

Choose a reason for hiding this comment

Uh oh!

gerrod3 Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

gerrod3 Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

decko commented Sep 9, 2026 •

edited

Loading

decko commented Sep 11, 2026 •

edited

Loading