fix(billing): migrate the ProcessedStripeEvent TTL index to its dead-letter-preserving spec - #4007
Conversation
…letter-preserving spec
The schema's 30-day TTL index on processedAt gained
partialFilterExpression { deadLetter: { $eq: false } } with no
accompanying migration. An already-deployed database still holding the
plain processedAt_1 TTL index hits IndexOptionsConflict (code 85) at
every boot — tolerated since #4004, but the live index keeps purging
dead-letter documents (permanent rejections that must never re-enter
replay) until it is swapped.
The migration also backfills deadLetter: false onto documents written
before the field existed: a partial filter's $eq: false never matches a
missing field, so those documents fell outside the TTL entirely and
would never expire.
Same-name swap (fast-path no-op on converged databases), modeled on the
sibling index migrations. No unique constraint involved, so no
duplicate-key window exists; the drop/recreate window only pauses TTL
expiry briefly. Reachable on legacy databases only with the #4004 boot
tolerance deployed — that fix must merge first.
Closes #4006
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4007 +/- ##
=======================================
Coverage 93.50% 93.50%
=======================================
Files 170 170
Lines 5727 5727
Branches 1839 1839
=======================================
Hits 5355 5355
Misses 302 302
Partials 70 70
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
1 participant
Summary
modules/billing/migrations/20260729120000-fix-processed-stripe-event-ttl-index-partial-filter.js) that repairs theProcessedStripeEvent30-day TTL index (processedAt_1) to match the schema's current spec, plus a 7-case integration test file modeled on the weekKey sibling.Scope
billing(migrations only — no schema/model change, the schema already declares the target spec)nonemedium(touches a live TTL index on a collection that may already hold data; mitigated by fast-path skip, ambiguity pre-check, and idempotent re-run — see below)The defects
1. Schema/migration drift (the crash/silent-wrong-behavior bug). The schema's TTL index on
processedAtgainedpartialFilterExpression: { deadLetter: { $eq: false } }with no accompanying migration. On an already-deployed database still holding the plainprocessedAt_1TTL index, autoIndex hitsIndexOptionsConflict(code 85 — same name, different options) on every boot. Left unrepaired, the live index keeps its old semantics forever: it goes on purging dead-letter documents (permanent rejections — Stripe was told to stop retrying) after 30 days, reopening the exact risk the partial filter exists to close — a later manual replay would re-process a purged event as brand new, causing double credit grants / double subscription resets.2. Latent backfill gap. Documents written before the
deadLetterfield existed carry nodeadLetterat all. Partial-filter matching uses query semantics, and{ deadLetter: { $eq: false } }does not match a missing field — those pre-mechanism documents would fall outside the TTL's scope entirely and never expire. The migration backfillsdeadLetter: falseonto them before recreating the index; the schema default already covers every write since the mechanism landed.3. Ambiguity pre-check (no guessing). Before backfilling, the migration does a zero-write pre-check: a document missing
deadLetterbut carrying dead-letter-era evidence (attempts > 0or a non-nulllastError) is impossible via any code path (those fields shipped together with the mechanism, and the schema default has materializeddeadLetteron every write since) — but if manual surgery or a partial restore ever produced one, silently defaulting it tofalsecould mark a genuine permanent rejection as purgeable. The migration aborts loud instead, naming sample ids, and leaves classification to a human. This is the same "abort loud on the impossible state" posture as the sibling index migrations.Why same-name swap, not the weekKey rename technique
20260728120000-fix-usage-weekkey-index-partial.jsrecreates its index under a new name to sidestep a conflict window. This migration deliberately does not do that: the schema already ships this index's target spec under its default name (processedAt_1), so renaming now would force every already-converged database to drop a correct index and install an identical twin under a different name — pure churn, just to spare legacy databases a conflict that PR #4004's boot tolerance already makes survivable. Ordering here is fast-path skip (already-exact spec + nothing to backfill → no-op) → ambiguity pre-check → backfill → drop same-key/same-name index(es) → recreate exact spec. No unique constraint is involved anywhere in this index, so there is no duplicate-key/E11000 window like the unique-index siblings — the only cost of the drop→recreate step is TTL expiry pausing briefly, which is harmless.Merge-order dependency
#4005 (boot tolerance for same-name index conflicts, closing #4004) must merge before this PR reaches a legacy (already-deployed) database. On such a database the live index still differs from the schema declaration, so
awaitIndexBuilds()hitsIndexOptionsConflictat boot; only with #4005's tolerance does boot survive long enough for the migration runner — which runs right after — to reach and execute this migration. Fresh databases and databases where the index already autoIndex'd correctly are unaffected either way (this migration's fast path is a no-op for them).Accepted residual
Same class of residual the sibling index migrations document: a narrow rolling-deploy TOCTOU window between the ambiguity pre-check (a read) and the backfill (a write) — a document could theoretically transition into the ambiguous shape between the two if multiple instances deploy concurrently mid-rollout. Not closed here, consistent with the siblings. Practically unreachable: the ambiguous shape requires dead-letter-era evidence, and Stripe's own retry window (3 days) is two orders of magnitude shorter than this index's 30-day TTL, leaving effectively no realistic scenario where that race matters before the window that would exploit it closes on its own.
Validation
npm run lintnpm testbilling.usage.weekKeyIndexPartialFilter.migration.integration.tests.js: legacy plain-TTL convergence, missing-field backfill, dead-letter documents untouched, ambiguity-abort, idempotent re-run, fast-path skip,syncIndexes()twin-identityGuardrails check
.env*,secrets/**, keys, tokens)Notes for reviewers