Skip to content

fix(retention): bound the five unpruned tables that filled the hosted D1 - #9415

Merged
JSONbored merged 2 commits into
mainfrom
fix/retention-unbounded-tables
Jul 27, 2026
Merged

fix(retention): bound the five unpruned tables that filled the hosted D1#9415
JSONbored merged 2 commits into
mainfrom
fix/retention-unbounded-tables

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

The hosted D1 reached its 10GB ceiling. Every write then failed with D1_ERROR: Exceeded maximum DB size — including recordOrbWebhookEvent's INSERT, so /v1/orb/webhook returned 500 to GitHub and inbound webhook delivery stopped fleet-wide until rows were pruned by hand. Reviews only kept moving because the internal regate sweep does not depend on webhooks.

Three compounding causes, all addressed:

1. Five high-growth tables had no retention rule at all and grew without bound. Measured during the incident:

table rows payload
check_summaries 100,459 0.30 GB
pull_request_files 54,532 0.20 GB
repo_github_totals_snapshots 198,552
recent_merged_pull_requests 29,634
orb_pr_outcomes 9,148

All five are re-derivable GitHub mirrors or superseded snapshots, re-fetched on the next sync of the PR that needs them — never a contributor's evidentiary trail (decision_records, 180d, remains the table carrying that).

2. The per-run delete cap made retention structurally unable to keep up. At 50k/table against a measured ~4.3M rows written/day, a daily prune could delete at most ~1.25M rows/day across the whole policy — a permanent ~3.5x deficit. The cap, not the cutoff, was the binding constraint, so no window tightening alone could close it. Raised to 250k.

3. Cadence. prune-retention moves from daily to hourly, so each pass stays small and the drain rate exceeds the write rate. Cheap when idle: each rule is an indexed range delete matching zero rows once a table is inside its window.

Both webhook idempotency logs also drop 90d -> 14d — the dedup lookups they serve are minutes old at most, and at fleet write rates a 90-day window on a pure idempotency log was the single largest avoidable contributor.

Test plan

  • New test pins all five previously-unbounded tables with their exact retention column names — a wrong column silently makes a rule a permanent no-op, so this matters as much as the window
  • New test pins the tightened 14-day window on both webhook logs
  • New functional prune test for check_summaries (largest single consumer) exercising the real policy entry rather than an ad-hoc override
  • Updated the orb_webhook_events test for the intentional 90d -> 14d change
  • Fixed a latent time-sensitivity in the shared seed() helper: its "recent" webhook row was anchored to a fixed NOW while processJob/the preview route evaluate against real wall-clock time, so that row survived the old 90-day window only by accident. Now anchored to real now, keeping each test's "recent rows are kept" intent independent of window width.
  • npx vitest run test/unit/retention.test.ts — 36 passed
  • selfhost-pg-retention, selfhost-cron-alignment, selfhost-queue-common, selfhost-pg-dialect — 119 passed
  • npm run typecheck clean

The hosted D1 reached its 10GB ceiling, after which every write failed with
`D1_ERROR: Exceeded maximum DB size` -- including recordOrbWebhookEvent's
INSERT, so /v1/orb/webhook returned 500 to GitHub and inbound webhook
delivery stopped fleet-wide until rows were pruned by hand.

Three compounding causes, all fixed here:

- check_summaries, pull_request_files, repo_github_totals_snapshots,
  recent_merged_pull_requests and orb_pr_outcomes had no retention rule at
  all and grew without bound. check_summaries (100,459 rows / 0.30GB of
  payload_json) and pull_request_files (54,532 rows / 0.20GB) were the two
  largest single consumers in the database.
- MAX_DELETED_PER_TABLE was 50k against a measured ~4.3M rows written per
  day, so a daily prune could delete at most ~1.25M rows/day across the
  whole policy -- a permanent ~3.5x deficit in which the cap, not the
  cutoff, was the binding constraint. Raised to 250k.
- prune-retention ran once a day. It now runs hourly, so each pass stays
  small and the drain rate exceeds the write rate.

Both webhook idempotency logs also drop 90d -> 14d; the dedup lookups they
serve are minutes old at most.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb

loopover-orb Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-27 18:06:42 UTC

4 files · 1 AI reviewer · 2 blockers · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
This PR adds five new 30/90-day retention rules for previously-unbounded GitHub-mirror tables, tightens the two webhook idempotency logs from 90d to 14d, raises the per-table delete cap from 50k to 250k, and moves prune-retention from a daily 03:00 tick to every hour — directly addressing the stated D1-ceiling incident with a well-reasoned batch-cap/cadence tradeoff (250k × 24 runs/day comfortably exceeds the claimed ~4.3M rows/day write rate). The check_summaries rule gets a genuine execution-level test that inserts rows and asserts the delete count, which would catch a wrong column name at runtime, but the other four new tables (pull_request_files, repo_github_totals_snapshots, recent_merged_pull_requests, orb_pr_outcomes) are only asserted via `toContainEqual` against the policy array, which cannot catch a nonexistent column — exactly the silent-no-op failure mode the PR's own commentary warns about.

Nits — 5 non-blocking
  • src/db/retention.ts: only check_summaries among the five new rules gets an execution-level test against a real table; pull_request_files/repo_github_totals_snapshots/recent_merged_pull_requests/orb_pr_outcomes are only checked via `toContainEqual`, which can't catch a wrong/nonexistent column name (the exact silent-no-op risk the PR's own comment calls out) — consider adding at least one seeded-insert test per remaining table, or confirm column names against schema.ts directly.
  • src/db/retention.ts:98-112 (RETENTION_PK_COLUMN): the five newly-added high-growth tables (check_summaries, pull_request_files, repo_github_totals_snapshots, recent_merged_pull_requests, orb_pr_outcomes) are absent from the PK map and fall back to rowid — the adjacent comment says that fallback is 'acceptable for the lower row counts of the tables that fall back today,' but these are precisely the tables measured at 30k–200k rows during the outage, so consider adding real single-column PKs here to match the comment's own stated criterion.
  • The new day-count literals (14, 30, 90) on the five new rules and the two tightened webhook rules could be named constants (e.g. `IDEMPOTENCY_LOG_RETENTION_DAYS`, `GITHUB_MIRROR_RETENTION_DAYS`) for readability, though the inline comments already explain the rationale well.
  • The two `validate`/`validate-tests` CI failures are undetailed and this branch is 6 commits behind the default branch — the failure cause could not be verified from what's given here; rebasing onto the latest default branch before merge would rule out a divergence-caused failure.
  • Add a seeded-row execution test (like the check_summaries one) for at least one more of the four remaining new tables to reduce the silent-no-op risk on a column-name typo.

Concerns raised — review before merging

  • No linked issue detected: No closing reference or linked issue number was found in the PR metadata/body. — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue: This repo's maintainer focus manifest requires every PR to reference a tracked issue. — Link the relevant issue (for example Closes #123) before opening the PR.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. No linked issue detected: No closing reference or linked issue number was found in the PR metadata/body. — If this PR is intended to solve an issue, link it explicitly in the PR body.

2. Maintainer requires a linked issue: This repo's maintainer focus manifest requires every PR to reference a tracked issue. — Link the relevant issue (for example `Closes #123`) before opening the PR.

Decision drivers

  • ❌ Code review — 2 blockers (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 13 registered-repo PR(s), 13 merged, 345 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 13 PR(s), 345 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 13 PR(s), 345 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Then work through the remaining 2 steps in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

Decision record
  • action: hold · clause: missing_linked_issue
  • config: 5e18b46bedc1f42028ad14a2baccb6d1591b4a2131200a79c515389d94991598 · pack: oss-anti-slop · ci: passed
  • record: 701e0ce7c3684f90741d71d0f846771b0e9e2f2271f91cfd0958631d054c4140 (schema v5, head ac99657)

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@JSONbored JSONbored self-assigned this Jul 27, 2026
@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 27, 2026
@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.59%. Comparing base (0a93e29) to head (ac99657).
⚠️ Report is 6 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9415      +/-   ##
==========================================
+ Coverage   89.52%   93.59%   +4.07%     
==========================================
  Files         840      738     -102     
  Lines      109777    60119   -49658     
  Branches    26147    21249    -4898     
==========================================
- Hits        98275    56268   -42007     
+ Misses      10239     2900    -7339     
+ Partials     1263      951     -312     
Flag Coverage Δ
backend 93.59% <100.00%> (-1.65%) ⬇️
control-plane ?
engine ?
rees ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/db/retention.ts 100.00% <100.00%> (ø)
src/index.ts 98.21% <100.00%> (+1.78%) ⬆️

... and 275 files with indirect coverage changes

The three scheduled-enqueue assertions in index.test.ts pin the exact job
list per tick, so moving prune-retention from a 03:00-only slot to every
hourly tick correctly failed them. Add it to each list at its dispatch
position (after rollup-product-usage).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant