Skip to content

perf(task-board): cache the assembled PR card, and never wait on GitHub for what the DB has - #6966

Merged
pedrofrxncx merged 1 commit into
mainfrom
perf/pr-card-level-cache
Sep 4, 2026
Merged

perf(task-board): cache the assembled PR card, and never wait on GitHub for what the DB has#6966
pedrofrxncx merged 1 commit into
mainfrom
perf/pr-card-level-cache

Conversation

@pedrofrxncx

@pedrofrxncx pedrofrxncx commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Rebased on origin/main.

The measurement

pr_read_cache_fetches_total in prod, last hour:

outcome count
miss (blocks on GitHub) 922
stale (served instantly) 1273
hit 81
error 26

A miss blocks on four GitHub round-trips. That is the ~2s on opening a task — including on a page refresh well inside the 30-minute window, where it should have been a hit.

Fix 1 — cache the card, not the raw reads

The read cache stores raw GitHub payloads, and get_comments on a busy PR runs past the 512KB maxValueSize. The put is rejected, the rejection was swallowed by a bare catch {}, and that PR then misses on every read forever — silently, and indistinguishably from a cold cache. The busier the PR, the more certain it never caches.

A TaskBoardItemPr[] is a few hundred bytes, so it always stores, and one KV get replaces the four-to-six this made per PR. maxStaleMs is a day, not half an hour: past it the card blocks again, and a card rendered once should never make someone wait on GitHub twice. Stale is still refreshed on every poll, so this trades nothing but the worst case.

Fix 2 — never block for what the database already has

The repo, the number and the link are in task_board_item_prs the whole time; only the title, checks and preview need GitHub. fetchOrPlaceholder returns the DB row immediately on a cold card and runs the GitHub read in the background, so the enrichment lands in KV for the next poll instead of holding the response.

Two details that matter:

  • The reconciles are skipped on the placeholder. All-null means "we have not asked yet", not "open, unmerged, no checks" — prReadyForReview would read it as a candidate and hand the card to the reviewer on the strength of a database row. They run on the next poll, against real data.
  • A stale card is still served as live, not replaced by the placeholder. Falling back there would blank a fully-rendered card back to a bare link.

While any card is unenriched the dialog polls every 2s instead of 60s, so the fill-in is a moment rather than a minute.

Make the failure visible

Both caches now run on one generalized JetStreamKVPrCache (namespace + key, separate buckets, a cache label on the metric), and a rejected put increments store_rejected instead of vanishing. That counter is what would have surfaced fix 1 in an hour rather than a week. placeholder is a new outcome too, so the cold-card rate is visible.

The read cache stays — it still serves the archive/review sweeps, which read many distinct PRs and benefit from payload-level entries.

Testing

pr-cache.test.ts (renamed from pr-read-cache.test.ts; the file no longer only caches reads) — 9 tests:

  • the original 3 SWR cases (serve-then-revalidate, keep serving on a failed refresh, invalidate)
  • card-cache config invariants: longer max-stale than the read cache, sub-poll revalidate, distinct buckets
  • a bucket that rejects every put, pinning the "answers correctly but never caches" mode
  • fetchOrPlaceholder: returns the placeholder without waiting then serves the real value; a stale value stays live; past max-stale it falls back rather than blocking

bun run --cwd=apps/api check, bun run lint, knip clean. Pre-existing on main and unrelated: apps/web check has a duplicate-prosemirror-version error, and the 10 *.integration.test.ts in task-board/ need Postgres.

What to watch after deploy

sum by (cache, outcome) (increase(pr_read_cache_fetches_total[1h])). Expect cache="cards" to be overwhelmingly hit/stale, with placeholder only on genuinely cold cards. A non-zero store_rejected on cards means a check-run summary is blowing the cap and should be truncated before storing.

Follow-up, not in scope

The card cache is storage: Memory, so a NATS restart empties it and every card goes through the placeholder path once. Persisting the last-known card on the task_board_item_prs row would survive that — a migration plus a write path.

…ub for what the DB has

Prod, last hour: 922 misses to 81 hits on pr_read_cache. A miss blocks on
GitHub, which is the ~2s on opening a task — including on a refresh well inside
the 30-minute window, where it should have been a hit.

Two fixes.

**Cache the card, not the reads.** The read cache stores raw GitHub payloads,
and `get_comments` on a busy PR runs past the 512KB value cap; the put was
rejected, the rejection swallowed, and that PR then missed on EVERY read
forever — silently, and indistinguishably from a cold cache. A card is a few
hundred bytes, so it always stores, and one KV get replaces the four-to-six
this made per PR. `maxStaleMs` is a day, not half an hour: past it the card
blocks again, and a card rendered once should never make someone wait on GitHub
twice.

**Never block on a cold card.** The database already holds the repo, the number
and the link. `fetchOrPlaceholder` returns those immediately and runs the GitHub
read in the background, so the enrichment lands in KV for the next poll instead
of holding the response. The reconciles are skipped on the placeholder: all-null
means "we have not asked", not "open, unmerged, no checks", and acting on it
would hand a card to the reviewer on the strength of a database row. While a
card is unenriched the dialog polls every 2s instead of 60s.

Both caches now run on one generalized `JetStreamKVPrCache` (namespace + key,
separate buckets, a `cache` label on the metric), and a rejected put is counted
as `store_rejected` instead of vanishing — the counter that would have shown
this in an hour rather than a week. Cards are invalidated org-wide on a merge.
@pedrofrxncx
pedrofrxncx force-pushed the perf/pr-card-level-cache branch from 4cbf5ba to 1b7a444 Compare September 4, 2026 00:11
@pedrofrxncx pedrofrxncx changed the title perf(task-board): cache the assembled PR card, not the raw GitHub reads perf(task-board): cache the assembled PR card, and never wait on GitHub for what the DB has Sep 4, 2026
@pedrofrxncx
pedrofrxncx enabled auto-merge (squash) September 4, 2026 00:13
@pedrofrxncx
pedrofrxncx merged commit f8c26dc into main Sep 4, 2026
34 checks passed
@pedrofrxncx
pedrofrxncx deleted the perf/pr-card-level-cache branch September 4, 2026 00:20
decocms Bot pushed a commit that referenced this pull request Sep 4, 2026
PR: #6966 perf(task-board): cache the assembled PR card, and never wait on GitHub for what the DB has
Bump type: patch

- decocms (apps/api/package.json): 4.328.5 -> 4.328.6
- @decocms/native (apps/native/package.json): 4.328.5 -> 4.328.6

Deploy-Scope: both
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant