Skip to content

fix(errors): chunk the error tick's bulk statements under the bind-parameter cap - #1271

Merged
JeremyFunk merged 2 commits into
mainfrom
fix/error-tick-chunk-bulk-writes
Oct 6, 2026
Merged

JeremyFunk merged 2 commits into
mainfrom
fix/error-tick-chunk-bulk-writes

Conversation

@JeremyFunk

@JeremyFunk JeremyFunk commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Why

Postgres caps one statement at 32,767 bind parameters. persistErrorTickWindow wrote each window as one statement per table, so a window carrying a few thousand never-seen fingerprints failed on the candidate upsert (12 parameters per row):

Parse parameter type count exceeds 32767: 36840 [while: insert into "error_fingerprint_candidates" ...]

The transaction rolls back, the cursor (error_tick_states.processed_through) stays put, and the next cron claims the same window and fails the same way. Until the window can commit, that org gets no new issues, incidents, regressions, auto-resolves or error notifications. The existing row cap (TICK_MAX_WINDOW_ROWS = 20_000) does not help: it is far above what a single statement can bind.

What changed

  • Every bulk read and write in persistErrorTickWindow runs in chunks of BULK_CHUNK_ROWS = 500 inside the same transaction: candidate upsert and delete, issue prefetch and upsert, regression flip, state prefetch, incident/state refresh, incident claim and insert, stale-incident resolve, events, notification outbox.
  • 500 keeps the widest statement (the issue upsert, 26 parameters per row) at 13,000 parameters.
  • Scan rows are already deduplicated by fingerprint, so chunks never share a conflict target and the per-row ON CONFLICT semantics are unchanged.
  • A steady-state window (well under 500 fingerprints) still issues one statement per write.

Most of the diff in error-tick-persistence.ts is re-indentation from wrapping statements; hiding whitespace shows the real change.

Reviewer notes

  • A stuck org catches up on its own after deploy: up to 5 minutes of event time per one-minute tick. Notifications for incidents in the backlog go out as the backlog is applied.
  • A very large window now costs more round trips (about one set of statements per 500 fingerprints) but stays one transaction under the cursor row lock.

Testing

  • New test: one window of 3,100 new fingerprints opens 3,100 issues and incidents, the next tick refreshes them, and the quiet window 30 minutes later resolves all of them. It fails on main (the tick writes nothing) and passes with 1,000 fingerprints there, which is under the cap.
  • vitest run src/services/errors/ in packages/backend: 263 passing.
  • Typecheck scoped to error-tick-persistence.ts; oxfmt and oxlint on the changed files.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Devin Review

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of large bursts of errors, helping ensure issues and incidents are recorded without database-limit failures.
    • Improved incident updates and resolution for high-volume error periods, including accurate notifications when incidents resolve.

…rameter cap

Postgres caps a statement at 32,767 bind parameters. The error tick wrote each
window as one statement per table, so a window with a few thousand new
fingerprints failed on the candidate upsert (12 parameters per row), rolled
back, and was retried at the same width every minute: the org's cursor stopped
advancing and it produced no issues, incidents or notifications.

Every bulk read and write in persistErrorTickWindow now runs in chunks of 500
rows inside the same transaction. A steady-state window is still one statement
per write.
@maple-review-bot

maple-review-bot Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Note

A newer push replaced 22d6ebb before its review finished. The latest commit is reviewed in a new comment.

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3e0c27c9-edfc-4ef1-952b-1fbb93ecaf99
📥 Commits

Reviewing files that changed from the base of the PR and between 016ce2e and 0b92935.

📒 Files selected for processing (2)
  • packages/backend/src/services/errors/ErrorsService.test.ts
  • packages/backend/src/services/errors/error-tick-persistence.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

Error tick persistence now divides database reads and writes into chunks of 500 rows. The integration test covers issue and incident creation for 3,100 fingerprints, repeated observations, and incident resolution after a quiet period.

Changes

Error Tick Persistence

Layer / File(s) Summary
Batch candidate and issue persistence
packages/backend/src/services/errors/error-tick-persistence.ts
Candidate operations, issue lookups and upserts, and regression updates run in chunks. Upserts retain existing counters, timestamps, and version data; regression reopening uses a separate targeted update.
Refresh state and open incidents
packages/backend/src/services/errors/error-tick-persistence.ts
Incident and issue-state refreshes, conditional issue-state claims, and incident inserts run in chunks.
Resolve incidents and validate burst handling
packages/backend/src/services/errors/error-tick-persistence.ts, packages/backend/src/services/errors/ErrorsService.test.ts
Incident resolution, issue-state cleanup, event inserts, and notification-delivery inserts run in chunks. The test checks a 3,100-fingerprint burst, later observations, and resolution after 33 minutes.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Suggested reviewers: makisuo

Merge Risk: ⚪ Minimal · up to 0b929

No confirmed issue blocks merging. Reappearing issues that qualify as regressions still receive the targeted state update.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: chunking bulk error-tick statements to stay under PostgreSQL’s bind-parameter limit.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@maple-review-bot

maple-review-bot Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 4/5 · likely safe to merge
The chunked writes rely on Effect.forEach running raw drizzle query values, which is what the new 3,100-row test verifies end to end; otherwise the transformation is mechanical.
quality 100/100 · no findings · tests covered · risk medium

persistErrorTickWindow now runs every bulk read and write in 500-row chunks inside the same transaction, so a window past the 32,767 bind-parameter cap commits and the cursor advances instead of failing forever. The transformation is mechanical and I found no defect.

  • inChunks/chunks wrap each bulk statement in persistErrorTickWindow (candidate, issue, state, incident, event, outbox writes)
  • Duplicate conflict targets stay impossible per statement because mergeScanRows deduplicates by fingerprint first
  • incidentsResolved now counts distinct flipped ids from the chunked RETURNING
What was checked
  • Parameter budget per chunked statement: widest is the 26-column issue upsert at 13,000 params (error-tick-persistence.ts:531)
  • Every statement still filters OrgId, including the chunked state-clear and stale-issue read (error-tick-persistence.ts:891)
  • Raw SQL in the incident/state refresh is unchanged apart from indentation (git diff -w on the file)

0b92935 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@JeremyFunk
JeremyFunk merged commit d32f93c into main Oct 6, 2026
58 of 60 checks passed
@JeremyFunk
JeremyFunk deleted the fix/error-tick-chunk-bulk-writes branch October 6, 2026 17:30
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