fix(server): bound retention work under rowid reuse#65
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reached
Next review available in: 50 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: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Summary
scannedCountLIMIT scannedCountrowid subqueryWhy
tokensandaudit_logsuse ordinary hidden rowids, so SQLite may reuse a deleted maximum rowid. The prior numeric-range-only COUNT and DELETE could therefore expand after a scan: a two-row window{1, 2000}could become 2,000 candidates/deletions before the next statement. The new semantic limit makes both properties structural:expiredCount <= scannedCount <= 1000deletedCount <= scannedCount <= 1000Rows inserted or expiring behind the cursor remain next-sweep work. COUNT and DELETE are independent statement snapshots, so their counts are not required to match under concurrency.
Red-first proof
The first commit (
e1b9e139) reproduces the exact race on real SQLite for both tables: scan rowids1,2000, delete rowid2000, insert 1,999 expired rows without explicit rowids so SQLite reuses2..2000. Against the old source both tests failed withscannedCount=2,expiredCount=2000,deletedCount=2000. The fix makes both pass at2/2/2.D1 proof
Before implementation, the approved query shapes were checked with
EXPLAIN QUERY PLANagainst the DEV D1 database. Token and audit COUNT/DELETE used intrinsic rowid range searches; DELETE used a list subquery; the outer COUNT scanned only the limited subquery; no plan contained a table scan orUSE TEMP B-TREE. These were EXPLAIN-only reads (changed_db=false,changes=0,rows_written=0). The final statements use 4 token binds and 7 audit binds, below D1's 100-bind cap.Validation
Observed runtime: Node
v22.22.2, npm10.9.7(matching upstream CI).@relayauth/serversuite: 409/409 PASSdist/engine/retention-gc.{js,d.ts}; built JS contains all bounded COUNT/DELETE statementsgit diff --check: PASSRelease sequencing
Do not hand-bump the manifest in this PR. After merge, the owner-gated release workflow should publish
@relayauth/server@0.2.22; Cloud PR #2805 then pins the real registry artifact and is re-reviewed at its new head.