fix(run-store): stop run-create failing on a brief write stall - #4514
Merged
matt-aitken merged 1 commit intoAug 5, 2026
Merged
Conversation
|
Contributor
WalkthroughThe run store now accepts transaction options and defines a 15-second write timeout. Dedicated 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
ericallam
force-pushed
the
feature/tri-12968-fix-5s-interactive-transaction-timeout-fails-68-run
branch
from
August 5, 2026 15:10
ee4f7c2 to
975ed18
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
ericallam
marked this pull request as ready for review
August 5, 2026 15:24
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
On the run-ops store, a run create with no associated waitpoint was wrapped in an interactive transaction whose default 5s budget is wall-clock from BEGIN, so a brief write stall could expire it and throw before the create ran, losing the run. Single-write creates now run as an implicit transaction with no app-held budget; the triggerAndWait path (run plus associated waitpoint) keeps an interactive transaction with headroom.
ericallam
force-pushed
the
feature/tri-12968-fix-5s-interactive-transaction-timeout-fails-68-run
branch
from
August 5, 2026 15:35
975ed18 to
075c12b
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
matt-aitken
approved these changes
Aug 5, 2026
matt-aitken
deleted the
feature/tri-12968-fix-5s-interactive-transaction-timeout-fails-68-run
branch
August 5, 2026 16:35
Merged
ericallam
added a commit
that referenced
this pull request
Aug 15, 2026
…nce (maxWait + tx-start retry) (#4623) ## 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: - **maxWait 2s → 10s** (TRI-12982): how long Prisma waits to borrow a connection before it can `BEGIN`. A restart freeze holds the pool full, and the only thing that errored was transaction starts giving up at 2s. - **Retry transaction-start P2028-at-acquisition** (TRI-12984): when Prisma can't borrow a connection within `maxWait` it 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: | var | default | |---|---| | `DATABASE_TRANSACTION_MAX_WAIT_MS` | `10000` | | `DATABASE_TRANSACTION_START_RETRY_ENABLED` | `true` (kill switch) | | `DATABASE_TRANSACTION_START_RETRY_MAX_ATTEMPTS` | `2` | | `DATABASE_TRANSACTION_START_RETRY_BACKOFF_MIN_MS` | `50` | | `DATABASE_TRANSACTION_START_RETRY_BACKOFF_MAX_MS` | `250` | | `DATABASE_TRANSACTION_START_RETRY_BUDGET_PER_SEC` | `50` | | `DATABASE_TRANSACTION_START_RETRY_BUDGET_BURST` | `100` | Per-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_*` and `RUN_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 - The retry primitives live in `internal-packages/database` and never read `process.env` (IoC): a P2028-at-acquisition classifier, a `TokenBucketRetryBudget`, and `withTransactionStartRetry`, folded into the `$transaction` helper via a new `startRetry` option. Config is resolved at the app boundary and threaded in. - The `$transaction` helper is the chokepoint (wraps the whole transaction), not the per-statement `$allOperations` extension. - The run engine's writes go through `PostgresRunStore`'s own `.$transaction(...)`, not the webapp helper, so both the helper and the two `PostgresRunStore` sites apply maxWait + retry (sharing the per-pool config). Builds on the `options?: { timeout, maxWait }` seam added in #4514. - Webapp `$transaction` call sites get the default `maxWait` + retry injected at one merge point, so no call site needed editing. ## Evidence - Unit red/green in `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 `$transaction` wiring. - Typecheck clean: webapp, run-store, run-engine. - Full-stack run: bounded queue-ay pass (15 projects, real dev runs through the run-engine `PostgresRunStore` transaction 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. - Boots cleanly with per-pool overrides set. ## 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 ``` DATABASE_TRANSACTION_MAX_WAIT_MS=2000 # Prisma's built-in default (change defaults to 10000) DATABASE_TRANSACTION_START_RETRY_ENABLED=false # disable the new retry entirely ``` `maxWait=2000` is what every path used before (Prisma's default; the run-store sites and the helper passed no maxWait). `retry=false` short-circuits `withTransactionStartRetry` to 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: ``` DATABASE_TRANSACTION_MAX_WAIT_MS=10000 DATABASE_TRANSACTION_START_RETRY_ENABLED=true DATABASE_TRANSACTION_START_RETRY_MAX_ATTEMPTS=3 # 3 attempts (2 retries); ~30s acquisition tolerance covers a ~20-25s freeze DATABASE_TRANSACTION_START_RETRY_BACKOFF_MIN_MS=50 DATABASE_TRANSACTION_START_RETRY_BACKOFF_MAX_MS=250 DATABASE_TRANSACTION_START_RETRY_BUDGET_PER_SEC=50 DATABASE_TRANSACTION_START_RETRY_BUDGET_BURST=100 ``` Per-pool overrides `RUN_OPS_DATABASE_TRANSACTION_*` and `RUN_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's `pool_timeout` at or under `maxWait`, 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
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
2 participants
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.
Summary
On the run-ops store, creating a run could intermittently fail with a "Transaction already closed" error, and the run would never be created. Single-write run creates no longer run inside an interactive transaction, so a brief database write stall can't blow the transaction budget and drop the run.
Fix
The dedicated run-ops
createRun/createFailedRunwrapped a single nestedtaskRun.createin an interactive$transaction. Its default 5s budget is wall-clock fromBEGIN, so when a write briefly stalls the transaction expires before the create completes and throws, even though the statement itself is fast at the database.A single-write create does not need an interactive transaction: Prisma's implicit nested create is already atomic and holds no app-side budget, so it now runs directly. Only the
triggerAndWaitpath (run plus its associated waitpoint, two writes that must commit together) keeps an interactive transaction, now with headroom over the default.Verified with a red/green test against the real split topology (reproduces the exact expiry on the unchanged code, green after) and an end-to-end run created and completed through the dedicated store.