feat(webapp,run-store,database): env-configurable transaction resilience (maxWait + tx-start retry) - #4623
Conversation
…nce (maxWait + tx-start retry)
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (23)
🧰 Additional context used📓 Path-based instructions (11)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.ts📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/app/**/*.{ts,tsx}📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/**/*.ts📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
internal-packages/database/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{test,spec}.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
internal-packages/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (17)📚 Learning: 2026-03-22T13:26:12.060ZApplied to files:
📚 Learning: 2026-03-22T19:24:14.403ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-06-13T19:53:13.759ZApplied to files:
📚 Learning: 2026-06-17T17:13:49.929ZApplied to files:
📚 Learning: 2026-06-23T13:04:21.413ZApplied to files:
📚 Learning: 2026-05-05T09:38:02.512ZApplied to files:
📚 Learning: 2026-05-12T21:04:05.815ZApplied to files:
📚 Learning: 2026-06-25T18:21:51.905ZApplied to files:
📚 Learning: 2026-07-03T17:10:21.498ZApplied to files:
📚 Learning: 2026-05-20T17:21:18.543ZApplied to files:
📚 Learning: 2026-06-01T11:37:08.569ZApplied to files:
📚 Learning: 2026-06-04T18:16:35.386ZApplied to files:
📚 Learning: 2026-06-09T17:58:04.699ZApplied to files:
📚 Learning: 2026-05-18T14:40:02.173ZApplied to files:
📚 Learning: 2026-06-16T09:19:47.637ZApplied to files:
🔇 Additional comments (2)
WalkthroughAdded transaction-start retries for Prisma acquisition-time P2028 errors. The retry system supports bounded jittered backoff, shared token-bucket budgets, callbacks, and separate serialization retry handling. Added transaction wait and retry settings to 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
…s pool resilience
…m serialization retry, clamp maxWait, narrow release note
…holesale db.server mocks don't break
…-tolerant transaction env vars, and drop test mocks - Gate the acquisition-error exclusion from the maxRetries branch on startRetry actually being active, so disabling the new retry falls back to prior maxRetries behavior for the callers that set it (e.g. dashboardPreferences). - Generic DATABASE_TRANSACTION_* numeric vars fall back to their default on a blank value instead of coercing to 0. - Rewrite transaction.test.ts to use plain counters/closures instead of vi.fn() spies.
A 20-25s PlanetScale reader freeze can outlast 2 attempts (~2x maxWait ~= 20s tolerance). 3 attempts (~30s) covers it. Verified on the pooler-freeze rig: a 25s freeze loses 7/12 tx-starts at maxAttempts=2 but 12/12 survive at 3.
…client resilience - DATABASE_TRANSACTION_START_RETRY_ENABLED uses a blank-tolerant BoolEnvWithDefault(true) so a declared-but-empty value falls back to enabled instead of parsing as false and silently disabling the retry. - Derive each run-store's resilience from its actual client identity (resilienceForClient WeakMap lookup) instead of the routing role, so run-ops clients aliased onto the control-plane pool (split flag off) get the control-plane config, not a run-ops override. Lookup + registration live in transactionResilience.server to keep them off db.server's wholesale-mocked export surface.
… retry on callback entry - isTransactionAcquisitionError now also matches the pg driver adapter's 'timeout exceeded when trying to connect' (pre-BEGIN, no SQL ran). Without this the retry silently did nothing on driver-adapter pools, which prod uses on writers. Verified on the rig: an adapter pool that lost 18/20 tx-starts to a freeze now survives 20/20 via retries. - withTransactionStartRetry takes a canRetry() guard; the $transaction helper and the two PostgresRunStore sites set it to !entered, so an acquisition-shaped error surfacing from a nested transaction after the callback has run does not re-execute a side-effectful body.
What
Makes two transaction-resilience behaviors real and env-var configurable, defaults set to the good values, so we can tune during and after the Aug 15 database patch window without a redeploy:
BEGIN. A restart freeze holds the pool full, and the only thing that errored was transaction starts giving up at 2s.maxWaitit raises P2028 (Unable to start a transaction in the given time) and no SQL ran, so retrying is safe. Scoped narrowly: only that error (never P2024 pool-exhaustion), 2 attempts, jittered backoff, and a token-bucket budget so a mass freeze can't amplify into a retry storm.Env vars (
DATABASE_*convention)Generic defaults:
DATABASE_TRANSACTION_MAX_WAIT_MS10000DATABASE_TRANSACTION_START_RETRY_ENABLEDtrue(kill switch)DATABASE_TRANSACTION_START_RETRY_MAX_ATTEMPTS2DATABASE_TRANSACTION_START_RETRY_BACKOFF_MIN_MS50DATABASE_TRANSACTION_START_RETRY_BACKOFF_MAX_MS250DATABASE_TRANSACTION_START_RETRY_BUDGET_PER_SEC50DATABASE_TRANSACTION_START_RETRY_BUDGET_BURST100Per-writer-pool overrides, each falling back to the generic when unset (same pattern as the per-client pool/connect-timeout work):
RUN_OPS_DATABASE_TRANSACTION_*andRUN_OPS_LEGACY_DATABASE_TRANSACTION_*(all 7 knobs each). Transactions only open on writer pools, so those are the only pools with their own knobs. Each pool gets its own token bucket, so a storm on one pool can't drain another's retry budget.Design
internal-packages/databaseand never readprocess.env(IoC): a P2028-at-acquisition classifier, aTokenBucketRetryBudget, andwithTransactionStartRetry, folded into the$transactionhelper via a newstartRetryoption. Config is resolved at the app boundary and threaded in.$transactionhelper is the chokepoint (wraps the whole transaction), not the per-statement$allOperationsextension.PostgresRunStore's own.$transaction(...), not the webapp helper, so both the helper and the twoPostgresRunStoresites apply maxWait + retry (sharing the per-pool config). Builds on theoptions?: { timeout, maxWait }seam added in fix(run-store): stop run-create failing on a brief write stall #4514.$transactioncall sites get the defaultmaxWait+ retry injected at one merge point, so no call site needed editing.Evidence
internal-packages/database: reverting the helper wiring turned the acquisition-retry test red (Unable to start a transaction in the given time), re-applying it green. Full package suite 25/25. Covers: classifier (P2028-acq yes, P2024 no, in-tx P2028 no), retry (retry-then-succeed, no-retry P2024, stop at maxAttempts, disabled, budget-exhausted, jitter bounds), token bucket, and$transactionwiring.PostgresRunStoretransaction path). 13 pass; the 2 failures are one documented known-failure and one stale-worker-state flake that passes 2/2 with this change active on a fresh app.Configuration & rollout
Ship inert first (zero behavior change), then flip to the good values live via env — no redeploy needed for either.
Inert — behaves exactly as today
maxWait=2000is what every path used before (Prisma's default; the run-store sites and the helper passed no maxWait).retry=falseshort-circuitswithTransactionStartRetryto a single run and makes the serialization-retry exclusion a no-op. Verified on the pooler-freeze rig: identical fail-fast P2028 at ~2003ms with zero retries — byte-for-byte current behavior, across all pools.Production ("good") — the baked defaults
Rely on defaults (nothing to set) or set explicitly:
Per-pool overrides
RUN_OPS_DATABASE_TRANSACTION_*andRUN_OPS_LEGACY_DATABASE_TRANSACTION_*(all seven knobs each) are optional and fall back to the generic set — not needed for v1; the generic set covers the control-plane, run-ops, and run-ops-legacy writer pools. Readers open no transactions and take nothing.Guardrail: the retry only engages when a pool's
pool_timeout>maxWait. Prod is fine (DATABASE_POOL_TIMEOUT=60>> 10). Do not set any writer pool'spool_timeoutat or undermaxWait, or saturation failures flip from retryable P2028 to non-retryable P2024 and the retry silently stops helping.Rollback
Env flip (set inert) or revert. Retry only fires where no SQL ran, and the per-pool token bucket caps a storm. No migration.
refs TRI-13295, TRI-12982, TRI-12984