Serve stale virtual members while they refresh #167

Merged

benvin merged 5 commits from benvin/virtual-serve-stale-member into master 2026-10-25 15:44:16 +11:00

Conversation 3 Commits 5 Files Changed 10

+459 -11

unkin-agent commented 2026-10-25 13:16:06 +11:00

Member

Copy Link

Copy Source

Virtual merges expire after 60s and the next request blocks on every member. One slow member (a large github_rpm remote) makes the re-merge outlast dnf's 30s timeout.

• add serve_stale to remotes (model, v2 API, migration 0008, UI, TUI), default false
• serve a flagged member's last good fetch to rpm/deb/alpine merges at once
• refresh it in the background, single-flighted, detached from the request
• block on the member again once its copy is over an hour old, or the flag is off
• prune copies with their merge cache; drop refreshes for evicted entries

Virtual merges expire after 60s and the next request blocks on every member. One slow member (a large github_rpm remote) makes the re-merge outlast dnf's 30s timeout. - add `serve_stale` to remotes (model, v2 API, migration 0008, UI, TUI), default false - serve a flagged member's last good fetch to rpm/deb/alpine merges at once - refresh it in the background, single-flighted, detached from the request - block on the member again once its copy is over an hour old, or the flag is off - prune copies with their merge cache; drop refreshes for evicted entries

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

Serve a serve_stale member's previous index while it refreshes ... 327f09fc6f

Add a serve_stale remote flag. rpm, deb and alpine virtual merges use a
flagged member's last good fetch immediately and refresh it in the
background, single-flighted per member.

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

Author

Member

Copy Link

Copy Source

• internal/virtual/engine.go:542 (staleMember) — !remote.ServeStale falls through to fetch without dropping e.stale[sk], so toggling serve_stale off keeps the parsed index in memory indefinitely, and toggling it back on serves that arbitrarily old copy → delete(e.stale, sk) under e.mu in the not-flagged branch.
• internal/virtual/engine.go:565 — no max age on the stale copy: if the background refresh keeps failing (member removed upstream, bad creds), merges serve the first-good index forever and the only signal is a slog.Warn → record fetch time with the copy and fall back to the blocking fetch (surfacing the error) past a bound.
• internal/virtual/engine.go:548 / :587 — memory bound leaks: a refresh finishing after evictLRU re-inserts e.stale[sk] for the evicted key, and entries are never pruned when a member is removed from a virtual or the virtual/remote is deleted (rpm keys are not evicted at all) → only store when e.gens[key] still exists, and prune on member/virtual removal (or key the map off the gen so it dies with it).
• internal/virtual/engine_stale_test.go — covers rpm only; the generic helper is wired separately for deb (:410) and alpine (:466) with no test → add one deb and one alpine case.
• internal/virtual/engine_stale_test.go — missing: refresh error keeps serving the previous copy, serve_stale toggled off stops serving stale, evictLRU prunes e.stale → add these.
• e2e/management_test.go — no create/update/get round trip of serve_stale through the v2 API and DB (PUT is a full replace, so an update omitting it resets to false; undocumented) → add a round-trip case and note the PUT behaviour in docs/common/caching.md.
• nit: ui/src/api/types.ts:19, ui/src/pages/RemoteDetail.tsx:52, internal/tui/render.go:60 — show stale_on_error but not serve_stale → add the field.

- internal/virtual/engine.go:542 (staleMember) — `!remote.ServeStale` falls through to `fetch` without dropping `e.stale[sk]`, so toggling serve_stale off keeps the parsed index in memory indefinitely, and toggling it back on serves that arbitrarily old copy → `delete(e.stale, sk)` under `e.mu` in the not-flagged branch. - internal/virtual/engine.go:565 — no max age on the stale copy: if the background refresh keeps failing (member removed upstream, bad creds), merges serve the first-good index forever and the only signal is a `slog.Warn` → record fetch time with the copy and fall back to the blocking fetch (surfacing the error) past a bound. - internal/virtual/engine.go:548 / :587 — memory bound leaks: a refresh finishing after `evictLRU` re-inserts `e.stale[sk]` for the evicted key, and entries are never pruned when a member is removed from a virtual or the virtual/remote is deleted (rpm keys are not evicted at all) → only store when `e.gens[key]` still exists, and prune on member/virtual removal (or key the map off the gen so it dies with it). - internal/virtual/engine_stale_test.go — covers rpm only; the generic helper is wired separately for deb (:410) and alpine (:466) with no test → add one deb and one alpine case. - internal/virtual/engine_stale_test.go — missing: refresh error keeps serving the previous copy, serve_stale toggled off stops serving stale, `evictLRU` prunes `e.stale` → add these. - e2e/management_test.go — no create/update/get round trip of `serve_stale` through the v2 API and DB (PUT is a full replace, so an update omitting it resets to false; undocumented) → add a round-trip case and note the PUT behaviour in docs/common/caching.md. - nit: ui/src/api/types.ts:19, ui/src/pages/RemoteDetail.tsx:52, internal/tui/render.go:60 — show `stale_on_error` but not `serve_stale` → add the field.

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

Bound serve_stale member copies by age, toggle and eviction ... 50ef1291fc

- drop a member's copy when serve_stale is off
- block on the member again once its copy is over an hour old
- skip storing a refresh whose entry was evicted or replaced
- show serve_stale in the UI and TUI

unkin-agent commented 2026-10-25 13:33:46 +11:00

Author

Member

Copy Link

Copy Source

• docs/common/caching.md:19 — the blank line plus the PUT replaces the whole remote... paragraph sit inside the options table, so the quarantine_* and mirrorlist rows below it no longer render as table rows → move the paragraph below the table.
• docs/common/caching.md:20 — the PUT paragraph is unrelated to serve_stale (atomicity) → drop it from this PR, or split into a docs-only PR.
• internal/virtual/engine.go:565 — the cold-fetch path (prev == nil) re-inserts under e.stale[sk] == prev, which is nil == nil after eviction, a toggle-off or a toggle-off/on cycle; a cold fetch that outlives its eviction re-adds a copy for an evicted key → track a per-key generation (or check e.gens[key] still exists and the flag is still set) before storing.
• nit: internal/virtual/engine.go:555 — expired copies are only pruned inside staleMember, so a deleted virtual, a removed member, or an rpm key (never LRU-evicted) holds its index in memory until some other member fetch happens → also prune from evictLRU/a merge-cache sweep, or accept and document the ceiling.

- docs/common/caching.md:19 — the blank line plus the `PUT replaces the whole remote...` paragraph sit inside the options table, so the `quarantine_*` and `mirrorlist` rows below it no longer render as table rows → move the paragraph below the table. - docs/common/caching.md:20 — the PUT paragraph is unrelated to serve_stale (atomicity) → drop it from this PR, or split into a docs-only PR. - internal/virtual/engine.go:565 — the cold-fetch path (`prev == nil`) re-inserts under `e.stale[sk] == prev`, which is `nil == nil` after eviction, a toggle-off or a toggle-off/on cycle; a cold fetch that outlives its eviction re-adds a copy for an evicted key → track a per-key generation (or check `e.gens[key]` still exists and the flag is still set) before storing. - nit: internal/virtual/engine.go:555 — expired copies are only pruned inside `staleMember`, so a deleted virtual, a removed member, or an rpm key (never LRU-evicted) holds its index in memory until some other member fetch happens → also prune from `evictLRU`/a merge-cache sweep, or accept and document the ceiling.

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

Guard serve_stale cold fetches against eviction and prune copies on every merge 5b4f904a17

unkin-agent commented 2026-10-25 13:54:03 +11:00

Author

Member

Copy Link

Copy Source

• nit: internal/virtual/engine_stale_test.go:83 — calls.Load() != 2 is asserted right after the 3 merges, but the refresh runs in a singleflight goroutine that may not have reached the member stub yet → call waitRefresh-style sync (or poll until calls==2 with a deadline) before asserting; a missed increment reads as 1.
• nit: internal/virtual/engine.go:553 — a transient getRemote error deletes the stale copy before falling through to a blocking fetch → on error keep the copy and only delete when getRemote succeeds with ServeStale false.

- nit: internal/virtual/engine_stale_test.go:83 — `calls.Load() != 2` is asserted right after the 3 merges, but the refresh runs in a singleflight goroutine that may not have reached the member stub yet → call `waitRefresh`-style sync (or poll until calls==2 with a deadline) before asserting; a missed increment reads as 1. - nit: internal/virtual/engine.go:553 — a transient `getRemote` error deletes the stale copy before falling through to a blocking fetch → on error keep the copy and only delete when `getRemote` succeeds with `ServeStale` false.

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

Merge master 1ad9b32542

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

Merge master 24c4253345

benvin merged commit e5313342ba into master 2026-10-25 15:44:16 +11:00

benvin deleted branch benvin/virtual-serve-stale-member 2026-10-25 15:44:16 +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#167