Skip to content

TML-3108: serialize queries per pinned pg client in the Postgres runtime driver - #29839

Merged
wmadden-electric merged 5 commits into
mainfrom
tml-3108-pinned-pg-client-serialization
Jul 30, 2026
Merged

TML-3108: serialize queries per pinned pg client in the Postgres runtime driver#29839
wmadden-electric merged 5 commits into
mainfrom
tml-3108-pinned-pg-client-serialization

Conversation

@wmadden-electric

@wmadden-electric wmadden-electric commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Consider a Cloudflare Worker handler that fans out two independent reads on one serverless handle:

const db = postgres(context).connect({ url });
const [users, posts] = await Promise.all([
  db.sql.execute(userPlan),
  db.sql.execute(postPlan),
]);

Both reads share one pg.Client. Today this works because pg@8 silently queues the second query behind the first — a deprecated internal that pg@9 removes: a query sent while another is in flight will throw (maintainer-confirmed in node-postgres discussion #3598). pg only warns when a third call overlaps, so the common two-wide Promise.all shape gives no warning at all today and becomes a hard error on the pg@9 upgrade.

This PR makes the driver own that flow control instead: a per-physical-client FIFO mutex (a WeakMap keyed by the underlying client, reusing the existing AsyncMutex) serializes statements on every pinned-client path — direct { kind: 'pgClient' } drivers, pinned connections from acquireConnection(), and transaction handles, which covers the serverless facade and the Supabase runtime that inherits it. Pool-level calls are untouched: each checks out its own PoolClient, so keying by physical client means they never contend.

The lock is held for exactly as long as the client is protocol-busy, and no longer:

  • Buffered execution (the production path — both shipped runtimes disable cursors) locks only the awaited client.query(). Row iteration is lock-free, so a nested query on the same client inside for await still works. An earlier draft held the lock across the yields; review caught that it hands lock lifetime to the consumer and deadlocks that nested shape, confirmed by execution before narrowing.
  • Cursor streams hold the lock for the whole BEGIN…COMMIT span, because the cursor genuinely occupies the connection for its lifetime. This subsumes the previous streamSpanLocks (deleted): the same mutex now guarantees concurrent streams cannot interleave their transaction wraps.
  • Transaction boundaries (BEGIN/COMMIT/ROLLBACK) each take the lock around their own statement — previously they bypassed all locking and could interleave into a concurrent statement.

Semantics are unchanged for working code: FIFO order at statement granularity is exactly what pg@8's queue provided, including its one deadlock shape (awaiting a query inside iteration of a cursor stream on the same client), which remains and is now documented on PostgresBinding. Statement-granularity locking deliberately does not fence other callers out of an open transaction on a shared client — that also matches pg@8, and no test claims otherwise.

Tests were written red-first: max-in-flight spies read 3/4/2/3/2 overlapping queries across the direct driver, mixed methods, pinned connection, transaction, and lease-overlap cases before the fix (all now 1), and the integration harness (real pg.Client over a PGlite socket server) captured the pg deprecation warning verbatim before and none after. Guard tests pin the other direction: pool-level queries must stay concurrent, and four timeout-guarded regressions pin the nested-query/abandoned-generator/commit-during-stream shapes that the narrowed lock must not break. The warning assertion lives in a single dedicated test because util.deprecate fires once per process.

Changes are confined to @prisma-next/driver-postgres (runtime driver + two test files). The CLI's control-plane driver needed no change — investigation on the linked ticket verified it already runs strictly sequentially (max 1 query in flight across a full migrate journey).

Refs: TML-3108

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Reworked PostgreSQL driver concurrency to serialize all operations that share the same underlying physical client, preserving strict FIFO execution and preventing interleaving.
    • Ensured cursor streaming and transaction control statements (BEGIN/COMMIT/ROLLBACK) remain consistent and don’t break during concurrent activity.
    • Kept parallelism for separate pooled connections.
  • Tests
    • Added a pinned-client FIFO/serialization test suite covering queries, prepared/explain, transactions, streaming (including abandoned streams), and proper unlock behavior.
    • Updated integration teardown and tightened warning assertions to reliably target the expected pg overlap warning.

Overlapping calls on a pinned pg.Client (direct driver, pinned connection,
transaction handle) currently rely on pg@8 silently queueing them, which
pg@9 replaces with a throw. Spy-based max-in-flight assertions and a
process-warning capture document the required behavior: max 1 query in
flight per physical client, FIFO order, pool-level paths unaffected.

Signed-off-by: willbot <w.a.madden+machine@gmail.com>
Signed-off-by: Will Madden <madden@prisma.io>
A WeakMap<client, AsyncMutex> serializes every unit of work on a physical
client: the whole runQuery stream span, buffered query()/explain(), and
BEGIN/COMMIT/ROLLBACK transaction boundaries. The per-client query lock is
held across the entire BEGIN-to-COMMIT stream span, which subsumes the old
streamSpanLocks; that second lock is deleted. Distinct PoolClients keep
their own locks, so pool-level calls stay concurrent. Removes the reliance
on pg@8's deprecated internal query queue, which pg@9 replaces with a
throw. Documents the concurrency contract on PostgresBinding.

Signed-off-by: willbot <w.a.madden+machine@gmail.com>
Signed-off-by: Will Madden <madden@prisma.io>
Holding the per-client lock across buffered-path row yields handed the
lock lifetime to the consumer, deadlocking a nested query inside for-await
on a cursor-disabled pinned client — the production path, and a shape that
works under pg@8. The buffered path now locks only the awaited
client.query; the cursor path keeps the wide stream-span lock (the cursor
occupies the connection for its lifetime) and releases it before the
buffered fallback. Consolidates the warning assertion into one dedicated
test (util.deprecate fires once per process), renames the lease-overlap
test to claim serialization rather than isolation, and documents lock
lifetime on PostgresBinding.

Signed-off-by: willbot <w.a.madden+machine@gmail.com>
Signed-off-by: Will Madden <madden@prisma.io>
@wmadden-electric
wmadden-electric requested a review from a team as a code owner July 29, 2026 10:21
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: 3322a08b-e185-47d8-a41f-16f10df9d825

📥 Commits

Reviewing files that changed from the base of the PR and between 9c366ba and c90c1b3.

📒 Files selected for processing (1)
  • packages/3-targets/7-drivers/postgres/test/driver.pinned-client-serialization.integration.test.ts

📝 Walkthrough

Walkthrough

The PostgreSQL driver now serializes operations per physical client using a shared mutex. Buffered queries lock only during execution, while cursor streams retain the lock through their lifecycle. Unit and integration tests validate ordering, transaction boundaries, and pool-client independence.

Changes

PostgreSQL client serialization

Layer / File(s) Summary
Per-client lock contract
packages/3-targets/7-drivers/postgres/src/postgres-driver.ts
Adds a per-physical-client mutex and documents FIFO ordering plus buffered and cursor lock lifetimes.
Driver operation serialization
packages/3-targets/7-drivers/postgres/src/postgres-driver.ts
Serializes queries, EXPLAIN, buffered execution, cursor streams, and BEGIN/COMMIT/ROLLBACK statements with operation-specific release timing.
Serialization test coverage
packages/3-targets/7-drivers/postgres/test/driver.pinned-client-serialization.test.ts
Tests direct, pinned, connection, transaction, streaming, abandoned-stream, nested-query, and multi-client behavior.
Wire-level validation
packages/3-targets/7-drivers/postgres/test/driver.pinned-client-serialization.integration.test.ts
Uses PGlite and a real pg client to verify sequential execution, warning capture, and stream transaction ordering.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Driver
  participant ClientQueryLock
  participant PhysicalClient
  Driver->>ClientQueryLock: Acquire lock for physical client
  ClientQueryLock->>PhysicalClient: Execute one query operation
  PhysicalClient-->>ClientQueryLock: Complete query
  ClientQueryLock-->>Driver: Release lock
Loading

Suggested labels: lgtm

Suggested reviewers: aqrln, sevinf, tensordreams

🚥 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 matches the main change: per-pinned-client query serialization in the Postgres runtime driver.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch tml-3108-pinned-pg-client-serialization

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.

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

size-limit report 📦

Path Size
postgres / no-emit 170.21 KB (0%)
postgres / emit 150.84 KB (0%)
mongo / no-emit 100.72 KB (0%)
mongo / emit 90.4 KB (0%)
cf-worker / no-emit 195.86 KB (0%)
cf-worker / emit 174.69 KB (0%)

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/3-targets/7-drivers/postgres/test/driver.pinned-client-serialization.integration.test.ts (1)

88-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Narrow the deprecation-warning filter to pg’s overlap warning.

captureProcessWarnings() currently stores every process DeprecationWarning emitted during the window, so unrelated deprecation warnings surface as this assertion failure. Use pg’s overlap-warning text in the filter, e.g. a case-insensitive pattern for Calling client.query when the client is already executing a query is deprecated and will be removed in pg@9.0. Use async/await or an external async flow control mechanism instead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/3-targets/7-drivers/postgres/test/driver.pinned-client-serialization.integration.test.ts`
around lines 88 - 102, Update captureProcessWarnings to retain only
DeprecationWarning instances whose message matches pg’s overlap warning text,
using a case-insensitive pattern for the specified message; continue ignoring
unrelated warnings while preserving the existing warnings collection and stop
behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@packages/3-targets/7-drivers/postgres/test/driver.pinned-client-serialization.integration.test.ts`:
- Around line 56-86: Update createHarness to register the Harness with the
shared cleanup path before client.connect() and driver.connect() can fail, while
ensuring its close callback can access the initialized resources. Extend
Harness.close to close the pg Client after driver and server cleanup, preserving
cleanup when setup rejects and avoiding socket leaks.

---

Nitpick comments:
In
`@packages/3-targets/7-drivers/postgres/test/driver.pinned-client-serialization.integration.test.ts`:
- Around line 88-102: Update captureProcessWarnings to retain only
DeprecationWarning instances whose message matches pg’s overlap warning text,
using a case-insensitive pattern for the specified message; continue ignoring
unrelated warnings while preserving the existing warnings collection and stop
behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: 45af038b-a0fa-47d0-b521-3ff4a6688dca

📥 Commits

Reviewing files that changed from the base of the PR and between a50a762 and 9c366ba.

📒 Files selected for processing (3)
  • packages/3-targets/7-drivers/postgres/src/postgres-driver.ts
  • packages/3-targets/7-drivers/postgres/test/driver.pinned-client-serialization.integration.test.ts
  • packages/3-targets/7-drivers/postgres/test/driver.pinned-client-serialization.test.ts

Register harness cleanup before the fallible connect steps so a rejection
mid-setup still tears down the PGlite instance, socket server, and pg
client, and end the pg client explicitly after driver/server shutdown
(no-op-safe when the driver already ended it). Narrow the warning capture
to pg's "already executing a query" DeprecationWarning so unrelated
deprecations cannot flake the empty-warnings assertion.

Signed-off-by: willbot <w.a.madden+machine@gmail.com>
Signed-off-by: Will Madden <madden@prisma.io>
@wmadden-electric

Copy link
Copy Markdown
Contributor Author

Both review items addressed in c90c1b3: harness cleanup now registers before the fallible connect steps and ends the pg client on close (socket-leak fix), and the warning capture is narrowed to pg's "already executing a query" DeprecationWarning so unrelated deprecations cannot flake the assertion.

@wmadden-electric
wmadden-electric added this pull request to the merge queue Jul 30, 2026
Merged via the queue into main with commit e7059de Jul 30, 2026
6 of 7 checks passed
@wmadden-electric
wmadden-electric deleted the tml-3108-pinned-pg-client-serialization branch July 30, 2026 11:30
wmadden-electric added a commit that referenced this pull request Aug 7, 2026
Verified every in-flight and not-started task against the codebase and
Linear, and updated both roadmap surfaces (ROADMAP.md and ROADMAP.html)
to match reality:

- @db.* attribute deletion marked landed (TML-2988, #1054)
- Repo move, v7 branch, and publishing pipeline statuses corrected
  (moved to prisma/prisma July 27-28; history graft dropped)
- Scoreboard rewritten to the in-repo scorecard state (593 rows:
  416/488/12/244); test-port count updated to 1,308 of 6,304
- Aggregate codec decoding marked resolved (TML-3064, #29867)
- pg deprecation warning marked landed (TML-3108, #29839; TML-2628
  closed)
- Editor-support item rewritten to reflect the shipped language server
- Added raw query support and deprecate-old-repo tasks; deleted the
  regenerate-examples task (regeneration happens per PR)
- Recounted headers: 44 tasks, 14 done / 9 in flight / 21 not started

Signed-off-by: willbot <w.a.madden+machine@gmail.com>
Signed-off-by: Will Madden <madden@prisma.io>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

3 participants