Add cache_repodata toggle for github_rpm remotes #168

Merged

benvin merged 5 commits from benvin/github-rpm-repodata-cache into master 2026-10-25 20:21:19 +11:00

Conversation 3 Commits 5 Files Changed 13

+648 -26

unkin-agent commented 2026-10-25 13:17:02 +11:00

Member

Copy Link

Copy Source

github_rpm remotes regenerate repomd and all three gz files from every metadata row on each request, which is expensive for large repos under repeated dnf traffic.

• add a per-remote cache_repodata toggle (column, model, v2 API, docs)
• cache built repodata per replica, rebuilt via singleflight when the count(*)+max(id) row fingerprint moves
• store a build only if no newer build has landed; evict idle entries hourly
• map cached-path DB errors like the uncached path: 503 + Retry-After on cancel/deadline, generic 500 otherwise
• check cacheEmpty via the fingerprint for all github_rpm remotes, not only cached ones

github_rpm remotes regenerate repomd and all three gz files from every metadata row on each request, which is expensive for large repos under repeated dnf traffic. - add a per-remote `cache_repodata` toggle (column, model, v2 API, docs) - cache built repodata per replica, rebuilt via singleflight when the `count(*)`+`max(id)` row fingerprint moves - store a build only if no newer build has landed; evict idle entries hourly - map cached-path DB errors like the uncached path: 503 + Retry-After on cancel/deadline, generic 500 otherwise - check `cacheEmpty` via the fingerprint for all github_rpm remotes, not only cached ones

unkin-agent added 1 commit 2026-10-25 13:17:02 +11:00

Add cache_repodata toggle for github_rpm remotes ... 5d50eda58a

Cache generated repodata per replica and rebuild only when the remote's
rpm_metadata fingerprint (count, max id) changes.

unkin-agent commented 2026-10-25 13:19:34 +11:00

Author

Member

Copy Link

Copy Source

• docs/rpm/remote-github.md:56 — the new "Repodata cache" section is inserted between the multi-replica bullets and the HCL example, so the example (and its dnf paragraph) now reads as part of the cache section → move the section below the example/dnf paragraph, and add a cache_repodata = true line to the example.
• internal/provider/rpm/github_repodata_cache_test.go:20 — fpStore.InsertRPMMetadata bumps seq even on a conflicting insert, unlike Postgres (ON CONFLICT DO NOTHING leaves max(id) unchanged), and nothing covers the digest-change/prune delete+reinsert path where count is unchanged and only max id moves → make the fake mirror the DB, and add a test that delete+insert of one row rebuilds.
• internal/provider/rpm/github.go:327 — dropRepodata only runs when a repodata request for that remote reaches the uncached branch on that replica; a deleted remote (or toggled off and never requested again) keeps its cached index in memory forever → nit: also drop when the remote is gone, or accept and note the ceiling.

- docs/rpm/remote-github.md:56 — the new "Repodata cache" section is inserted between the multi-replica bullets and the HCL example, so the example (and its dnf paragraph) now reads as part of the cache section → move the section below the example/dnf paragraph, and add a `cache_repodata = true` line to the example. - internal/provider/rpm/github_repodata_cache_test.go:20 — `fpStore.InsertRPMMetadata` bumps `seq` even on a conflicting insert, unlike Postgres (ON CONFLICT DO NOTHING leaves max(id) unchanged), and nothing covers the digest-change/prune delete+reinsert path where count is unchanged and only max id moves → make the fake mirror the DB, and add a test that delete+insert of one row rebuilds. - internal/provider/rpm/github.go:327 — `dropRepodata` only runs when a repodata request for that remote reaches the uncached branch on that replica; a deleted remote (or toggled off and never requested again) keeps its cached index in memory forever → nit: also drop when the remote is gone, or accept and note the ceiling.

unkin-agent added 1 commit 2026-10-25 13:26:39 +11:00

Evict idle repodata cache entries and tighten fingerprint tests 817b246eaf

unkin-agent commented 2026-10-25 13:29:47 +11:00

Author

Member

Copy Link

Copy Source

• internal/provider/rpm/github.go:317 — singleflight closure uses the leader request ctx; if the leader client disconnects, every coalesced waiter gets a spurious 500 "context canceled" → run the build with a detached ctx (context.WithoutCancel) or have waiters retry on ctx error.
• internal/provider/rpm/github.go:272 — cached path returns 500 + err.Error() on any failure, losing the existing canceled→503 mapping in readMetadataEntries and leaking DB error text → reuse readMetadataEntries semantics (503 on ctx cancel, generic 500 body).
• internal/provider/rpm/github.go:296 — unknown repodata tails (e.g. repodata/foo) trigger fingerprint query + a full rebuild on a cold cache before the 404 → check tail first, 404 before building.
• internal/provider/rpm/github.go:317 — a slower older build can overwrite a newer cache entry (stores its own count/maxID unconditionally) → only store if the existing entry is not newer, or accept and drop; at minimum cover with a test.
• internal/api/v2/remotes.go — PR body claims "v2 API" plumbing but no v2 handler/test changes; field round-trips only via models.Remote JSON → add a v2 create/update/get test asserting cache_repodata persists (and that PUT omitting it resets to false is intended).
• internal/provider/rpm/github_repodata_cache_test.go — no test for singleflight (concurrent requests -> one list), fingerprint/list error paths, or cacheEmpty via fingerprint → add them.
• nit: github.go:200 — dropRepodata takes the global mutex and sweeps on every uncached request → only call it when the remote is in the map, or sweep elsewhere.
• nit: github_repodata_cache_test.go:TestCachedRepodataHitSkipsRegeneration — the first two serve calls are repeated by the loop → drop the redundancy.
• nit: cacheEmpty now uses the fingerprint even with cache_repodata=false (not "unchanged with toggle off", though equivalent) → state it in the PR body or split it out.

- internal/provider/rpm/github.go:317 — singleflight closure uses the leader request ctx; if the leader client disconnects, every coalesced waiter gets a spurious 500 "context canceled" → run the build with a detached ctx (context.WithoutCancel) or have waiters retry on ctx error. - internal/provider/rpm/github.go:272 — cached path returns 500 + err.Error() on any failure, losing the existing canceled→503 mapping in readMetadataEntries and leaking DB error text → reuse readMetadataEntries semantics (503 on ctx cancel, generic 500 body). - internal/provider/rpm/github.go:296 — unknown repodata tails (e.g. repodata/foo) trigger fingerprint query + a full rebuild on a cold cache before the 404 → check tail first, 404 before building. - internal/provider/rpm/github.go:317 — a slower older build can overwrite a newer cache entry (stores its own count/maxID unconditionally) → only store if the existing entry is not newer, or accept and drop; at minimum cover with a test. - internal/api/v2/remotes.go — PR body claims "v2 API" plumbing but no v2 handler/test changes; field round-trips only via models.Remote JSON → add a v2 create/update/get test asserting cache_repodata persists (and that PUT omitting it resets to false is intended). - internal/provider/rpm/github_repodata_cache_test.go — no test for singleflight (concurrent requests -> one list), fingerprint/list error paths, or cacheEmpty via fingerprint → add them. - nit: github.go:200 — dropRepodata takes the global mutex and sweeps on every uncached request → only call it when the remote is in the map, or sweep elsewhere. - nit: github_repodata_cache_test.go:TestCachedRepodataHitSkipsRegeneration — the first two serve calls are repeated by the loop → drop the redundancy. - nit: cacheEmpty now uses the fingerprint even with cache_repodata=false (not "unchanged with toggle off", though equivalent) → state it in the PR body or split it out.

unkin-agent added 1 commit 2026-10-25 13:37:25 +11:00

Harden github_rpm repodata cache error, 404 and store ordering e6375011dc

unkin-agent commented 2026-10-25 13:43:23 +11:00

Author

Member

Copy Link

Copy Source

• internal/provider/rpm/github.go:317 — (count, max id) can collide when a concurrent scan's insert holds a lower BIGSERIAL id that commits after a higher one: cached at (N, M) while id<M is in flight, then that row commits and another row is deleted → same (N, M), different rows, stale repodata served indefinitely → nit: acceptable if scans are serialised per remote across replicas (state it in the comment on RPMMetadataFingerprint), otherwise add a cheap third term (e.g. sum(id)).
• internal/provider/rpm/github_repodata_cache_test.go:171 — waitFor ends with time.Sleep(50ms) to "let goroutines reach singleflight"; a goroutine stalled past that under -race misses the in-flight call after close(gate) and triggers a second list → flaky Coalesces/LeaderCancel tests → nit: gate on a deterministic signal (e.g. count fingerprint calls after ListRPMMetadataEntries is entered, or a hook invoked just before Do) instead of sleeping.
• internal/provider/rpm/github.go:333 — cacheEmpty and readMetadataEntries/writeMetadataError changes alter behaviour of all github_rpm/rpm remotes (not only cache_repodata) → nit: fine to keep as disclosed, but consider splitting the shared error-mapping change into its own PR.

- internal/provider/rpm/github.go:317 — (count, max id) can collide when a concurrent scan's insert holds a lower BIGSERIAL id that commits after a higher one: cached at (N, M) while id<M is in flight, then that row commits and another row is deleted → same (N, M), different rows, stale repodata served indefinitely → nit: acceptable if scans are serialised per remote across replicas (state it in the comment on RPMMetadataFingerprint), otherwise add a cheap third term (e.g. sum(id)). - internal/provider/rpm/github_repodata_cache_test.go:171 — waitFor ends with time.Sleep(50ms) to "let goroutines reach singleflight"; a goroutine stalled past that under -race misses the in-flight call after close(gate) and triggers a second list → flaky Coalesces/LeaderCancel tests → nit: gate on a deterministic signal (e.g. count fingerprint calls after ListRPMMetadataEntries is entered, or a hook invoked just before Do) instead of sleeping. - internal/provider/rpm/github.go:333 — cacheEmpty and readMetadataEntries/writeMetadataError changes alter behaviour of all github_rpm/rpm remotes (not only cache_repodata) → nit: fine to keep as disclosed, but consider splitting the shared error-mapping change into its own PR.

unkin-agent added 1 commit 2026-10-25 15:48:15 +11:00

Merge master e5a50315c3

unkin-agent added 1 commit 2026-10-25 19:08:01 +11:00

Merge master d41902a5fa

benvin merged commit f1b78988e4 into master 2026-10-25 20:21:19 +11:00

benvin deleted branch benvin/github-rpm-repodata-cache 2026-10-25 20:21:22 +11:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

benvin (BenVincent) unkin-agent (Unkin Agent) unkinben (Ben Vincent)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unkin/artifactapi#168