From 817b246eaf31299f9ac5862548084b8a063a0036 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sun, 25 Oct 2026 13:26:21 +1100 Subject: [PATCH] Evict idle repodata cache entries and tighten fingerprint tests --- docs/rpm/remote-github.md | 17 ++++---- internal/provider/rpm/github.go | 20 ++++++++- .../rpm/github_repodata_cache_test.go | 41 ++++++++++++++++++- 3 files changed, 68 insertions(+), 10 deletions(-) diff --git a/docs/rpm/remote-github.md b/docs/rpm/remote-github.md index 3d6eed1..19fe91a 100644 --- a/docs/rpm/remote-github.md +++ b/docs/rpm/remote-github.md @@ -53,14 +53,6 @@ derived metadata current off the client request path: ~once per `mutable_ttl` regardless of replica count, and the shared `etag` lets any replica issue the conditional request. -### Repodata cache - -Repodata is regenerated from the metadata rows on every request by default. Set -`cache_repodata: true` on the remote to keep the generated `repomd.xml` and its -`primary`/`filelists`/`other` files in memory per replica. Each request checks a -cheap row fingerprint (count + newest id) in the database and rebuilds only when -it moved, so every replica picks up a scan's changes on its next request. - ```hcl # Backend that serves the actual .rpm bytes from github.com. resource "artifactapi_remote_generic" "github" { @@ -76,6 +68,7 @@ resource "artifactapi_remote_github_rpm" "acme-tools" { base_url = "https://api.github.com/repos/acme/tools" # the releases API root releases_remote = "github" # backend for downloads mutable_ttl = 3600 # release re-scan interval + cache_repodata = true # keep generated repodata in memory # Optional: restrict which release assets become packages (regex on filename). patterns = [".*\\.x86_64\\.rpm$", ".*\\.noarch\\.rpm$"] @@ -88,3 +81,11 @@ resource "artifactapi_remote_github_rpm" "acme-tools" { `dnf` config: `baseurl=https://artifactapi.example/api/v1/remote/acme-tools`. The repo is multi-arch (no `$basearch` needed) — `dnf` selects matching packages from the synthesized metadata. + +### Repodata cache + +Repodata is regenerated from the metadata rows on every request by default. Set +`cache_repodata = true` on the remote to keep the generated `repomd.xml` and its +`primary`/`filelists`/`other` files in memory per replica. Each request checks a +cheap row fingerprint (count + newest id) in the database and rebuilds only when +it moved, so every replica picks up a scan's changes on its next request. diff --git a/internal/provider/rpm/github.go b/internal/provider/rpm/github.go index b6fcce4..d534e17 100644 --- a/internal/provider/rpm/github.go +++ b/internal/provider/rpm/github.go @@ -53,6 +53,8 @@ const ( // retryable 503. Kept short so a client never hangs on a rate-limited derive // of a large repo; small repos usually prime within this window. defaultColdWait = 8 * time.Second + // repodataIdleTTL evicts cached indexes of remotes deleted or switched off. + repodataIdleTTL = time.Hour ) // GitHubProvider is a metadata-only remote: it scans a GitHub repo's releases @@ -100,6 +102,7 @@ type rpmFingerprinter interface { type cachedRepodata struct { count, maxID int64 + used time.Time *repodata } @@ -295,8 +298,14 @@ func (p *GitHubProvider) cachedRepodata(ctx context.Context, store provider.RPMM if err != nil { return nil, err } + now := time.Now() p.repodataMu.Lock() + p.sweepRepodataLocked(now) c, ok := p.repodataCache[name] + if ok { + c.used = now + p.repodataCache[name] = c + } p.repodataMu.Unlock() if ok && c.count == count && c.maxID == maxID { return c.repodata, nil @@ -311,7 +320,7 @@ func (p *GitHubProvider) cachedRepodata(ctx context.Context, store provider.RPMM // worst case is one extra rebuild on the next request. rd := buildRepodata(metas) p.repodataMu.Lock() - p.repodataCache[name] = cachedRepodata{count: count, maxID: maxID, repodata: rd} + p.repodataCache[name] = cachedRepodata{count: count, maxID: maxID, used: time.Now(), repodata: rd} p.repodataMu.Unlock() return rd, nil }) @@ -324,9 +333,18 @@ func (p *GitHubProvider) cachedRepodata(ctx context.Context, store provider.RPMM func (p *GitHubProvider) dropRepodata(name string) { p.repodataMu.Lock() delete(p.repodataCache, name) + p.sweepRepodataLocked(time.Now()) p.repodataMu.Unlock() } +func (p *GitHubProvider) sweepRepodataLocked(now time.Time) { + for name, c := range p.repodataCache { + if now.Sub(c.used) > repodataIdleTTL { + delete(p.repodataCache, name) + } + } +} + func (p *GitHubProvider) cacheEmpty(ctx context.Context, store provider.RemoteMetadataStore, name string) bool { if fp, ok := store.(rpmFingerprinter); ok { count, _, err := fp.RPMMetadataFingerprint(ctx, name) diff --git a/internal/provider/rpm/github_repodata_cache_test.go b/internal/provider/rpm/github_repodata_cache_test.go index 611af6e..5d19382 100644 --- a/internal/provider/rpm/github_repodata_cache_test.go +++ b/internal/provider/rpm/github_repodata_cache_test.go @@ -27,10 +27,16 @@ func (s *fpStore) ListRPMMetadataEntries(ctx context.Context, name string) ([]pr } func (s *fpStore) InsertRPMMetadata(ctx context.Context, m *provider.RPMMetadata) error { + s.mu.Lock() + _, exists := s.rows[m.FilePath] + s.mu.Unlock() + if err := s.fakeStore.InsertRPMMetadata(ctx, m); err != nil || exists { + return err // ON CONFLICT DO NOTHING leaves max(id) unchanged + } s.mu.Lock() s.seq++ s.mu.Unlock() - return s.fakeStore.InsertRPMMetadata(ctx, m) + return nil } func (s *fpStore) RPMMetadataFingerprint(_ context.Context, _ string) (int64, int64, error) { @@ -145,3 +151,36 @@ func TestUncachedRepodataMatchesCachedAndReadsEveryTime(t *testing.T) { t.Fatal("disabling cache_repodata should drop the cached index") } } + +func TestCachedRepodataRebuildsOnSameCountReinsert(t *testing.T) { + p, store, remote := newCacheFixture(t, true) + addPkg(t, store, "alpha") // conflicting insert: fingerprint must not move + serve(t, p, remote, store, "repodata/repomd.xml") + serve(t, p, remote, store, "repodata/repomd.xml") + if n := store.listCount(); n != 1 { + t.Fatalf("full reads after conflicting insert = %d, want 1", n) + } + + if err := store.DeleteRPMMetadata(context.Background(), remote.Name, "alpha-1-1.noarch.rpm"); err != nil { + t.Fatal(err) + } + addPkg(t, store, "alpha") + serve(t, p, remote, store, "repodata/repomd.xml") + if n := store.listCount(); n != 2 { + t.Fatalf("full reads after same-count reinsert = %d, want 2", n) + } +} + +func TestCachedRepodataEvictsIdleRemotes(t *testing.T) { + p, store, remote := newCacheFixture(t, true) + serve(t, p, remote, store, "repodata/repomd.xml") + p.repodataCache["gone"] = cachedRepodata{used: time.Now().Add(-2 * repodataIdleTTL), repodata: &repodata{}} + + serve(t, p, remote, store, "repodata/repomd.xml") + if _, ok := p.repodataCache["gone"]; ok { + t.Fatal("idle remote's index not evicted") + } + if _, ok := p.repodataCache[remote.Name]; !ok { + t.Fatal("active remote's index evicted") + } +}