fix(retention): dedupe contributor-decision-pack to latest-only, not a preserved series - #9459
Merged
Merged
Conversation
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Contributor
|
Important 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏳ LoopOver is waiting…LoopOver has seen this pull request and is waiting on CI checks to finish before reviewing it. This comment will update once the review runs. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟨 Waiting |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9459 +/- ##
==========================================
- Coverage 89.55% 88.65% -0.91%
==========================================
Files 843 843
Lines 110073 110073
Branches 26194 26194
==========================================
- Hits 98573 97582 -991
- Misses 10238 11520 +1282
+ Partials 1262 971 -291
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…a preserved series contributor-decision-pack was excluded from LATEST_ONLY_SIGNAL_SNAPSHOT_TYPES on the theory that it's a bounded trend/change series read by history, with volume "a fraction of" its three contributor-* neighbors that already dedupe. Both claims were wrong: - src/services/decision-pack.ts's only reader calls listSignalSnapshots(...)[0] -- the exact same latest-only contract as contributor-evidence-graph/outcome-history/strategy, which already dedupe correctly. - Measured against a live export of the hosted D1: 18,549 rows, 6.3 GB, ~350 KB/row -- 71% of the database's entire 8.96 GB file size, and roughly 750x the combined size of the three types it was assumed to dwarf. Every row is still inside the 90-day RETENTION_POLICY age window (the database itself is only ~65 days old), so age-based pruning had not removed a single one, and the type has accumulated ~700-1,300 rows/day since 2026-07-06 -- large enough to refill the 10 GB cap from empty in 3-4 weeks on its own, even with #9415's five-table fix applied (that PR addressed other tables totaling well under 1 GB combined). queue-health is the one signal type genuinely read as a historical series (maintainer-slop-duplicate-trend.ts shapes multiple weeks into a trend card) and stays excluded, unchanged. Refs #9435
JSONbored
force-pushed
the
fix/contributor-decision-pack-latest-only
branch
from
July 27, 2026 20:49
c39a4d5 to
ae17e14
Compare
JSONbored
added a commit
that referenced
this pull request
Jul 27, 2026
…loopover database (#9502) The previous database (b2c79dd6-7771-4b1f-aa6b-085d5f3e9528, internally still named "gittensory" from before the 2026-07-14 rename) had grown to 8.96 GB of its 10 GB cap. D1 never runs VACUUM (cloudflare/workerd#1618), so the batch of now-pruned contributor-decision-pack rows (#9459 -- 18,336 rows / ~6.3 GB deleted) freed pages that get reused, not returned: the file was never going to shrink back down on its own. Cuts DB over to a freshly created, correctly-named "loopover" database (da7537cc-ab54-4dc1-8c38-2713f03f1130), populated from a verified export of the pruned source database: - Every table's row count matches the source exactly, except ordinary live-write drift on actively-written tables (webhook/audit/usage logs, which never stop changing between an export and a later comparison) -- confirmed via a full 103-table sweep. - d1_migrations matches exactly (202/202 rows, same max id). - signal_snapshots' contributor-decision-pack (the table #9459 fixed) is 213/213, exactly one row per contributor, confirming the dedup carried over correctly. - 206 rows whose payload_json exceeded D1's ~100KB single-statement text limit (SQLITE_TOOBIG) were re-imported via a chunked INSERT + payload_json ||= UPDATE sequence, verified byte-identical against the source before import. No other file in the repo references the old database_id. cf-typegen is unaffected (binding name/shape unchanged, only the id). Refs #9435
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The actual dominant cause of the recurring D1 crisis
While investigating #9432/#9435, I exported the live hosted D1 (8.96 GB / 10 GB cap) and measured per-table volume directly rather than trusting row counts alone.
signal_snapshotscame back at 6.35 GB — and within it, one signal type,contributor-decision-pack, accounts for 6.3 GB across 18,549 rows (~350 KB/row). That's 71% of the entire database's file size, and it long predates and dwarfs everything #9415 (merged) fixed — that PR bounded five other tables totaling well under 1 GB combined.Root cause: a wrong doc comment
LATEST_ONLY_SIGNAL_SNAPSHOT_TYPES(src/db/retention.ts) explicitly excludedcontributor-decision-pack, on the stated theory that it's "a bounded trend/change series by design" with volume "a fraction of" the threecontributor-*types beside it that already dedupe (contributor-evidence-graph,contributor-outcome-history,contributor-strategy).Both claims are false, verified directly:
src/services/decision-pack.ts's only reader callslistSignalSnapshots(env, CONTRIBUTOR_DECISION_PACK_SIGNAL, login))[0]— index[0], the exact same latest-only contract as its three neighbors. Nothing anywhere reads it as history.Because
signal_snapshotshas a 90-day age-based retention rule (RETENTION_POLICY) but the hosted database itself is only ~65 days old, age-based pruning has not removed a single one of these rows yet — every one of the 18,549 rows is still live. The type has been accumulating ~700–1,300 rows/day since 2026-07-06. Left alone, it refills the 10 GB cap from empty in roughly 3–4 weeks on its own, regardless of #9415's fixes.queue-healthis the one signal type that genuinely is read as a series (maintainer-slop-duplicate-trend.tsshapes several weeks of snapshots into a trend card) — it correctly stays excluded, and this PR leaves it untouched.Change
Move
contributor-decision-packintoLATEST_ONLY_SIGNAL_SNAPSHOT_TYPES, correct the now-stale doc comments explaining the (wrong) exclusion, and update the tests that pinned the old (wrong) behavior:test/unit/retention.test.ts: the "preserves bounded history" test now covers onlyqueue-health; a new dedicated test coverscontributor-decision-packdeduping to latest-per-contributor; the contributor-intelligence-types test's decision-pack assertions flip from "series preserved" to "deduped."No schema change, no new migration.
retention.tsis fully covered by the updated tests (statements + branches).What this doesn't fix
The already-accumulated 6.3 GB stays on disk until either (a) the next
dedupeSignalSnapshotsrun prunes it (this fix makes that happen going forward), or (b) the file itself is reclaimed — D1 never runsVACUUM(cloudflare/workerd#1618), so freed pages are reused, not returned to the OS; the file size will plateau rather than shrink. A follow-up (tracked separately) covers reclaiming the already-inflated file via a fresh database + data migration.Refs #9435