You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
sql-pg: prepared-statement names are a per-connection counter, so they collide behind a transaction-mode pooler (Hyperdrive, PgBouncer) #8320
effect@4.0.0-rc.115, @effect/sql-pg@4.0.0-rc.115, on Cloudflare Workers (workerd), talking to Postgres through Cloudflare Hyperdrive. Present on main in packages/sql/pg/src/PgConnection.ts (PreparedCache).
What steps can reproduce the bug?
Run any workload with the default prepare (on) through a transaction-mode pooler — Hyperdrive, PgBouncer in transaction mode, Supabase's pooled port — with more than one connection in play:
PreparedCache is per connection, so the counter is too, and effect5 means "the fifth distinct statement this connection prepared". But a prepared statement name is scoped to the backend session, not to the client connection. A transaction-mode pooler multiplexes N client connections onto M backend sessions and returns the session to the pool at the end of each transaction, so two client connections — two entries in one maxConnections: 5 pool, or two Worker isolates — can each hold their own idea of effect5 and be routed to the same session. The second one's Bind lands on the first one's plan.
Nothing exotic is required: concurrent traffic and a pooler are enough. We saw four occurrences across half an hour of normal production load.
What is the expected behavior?
A name minted by one connection can never be mistaken for another connection's statement. The smallest fix is to namespace the counter per connection, e.g. a random component chosen at connection setup:
name: `effect_${this.nonce}_${++this.counter}`// nonce: 8 hex chars, per connection
That costs a few bytes per Parse and removes the collision entirely.
Worth separating two hazards here, because only one of them is loud:
The loud one, which is what we hit: the colliding statements have different parameter counts, so Postgres rejects the Bind and you get the error above. Noisy and confusing, but safe.
The quiet one: if the two statements happen to have the same arity and compatible parameter types, there is nothing for Postgres to reject. The Bind succeeds against the other statement's plan and the backend executes the wrong SQL with your parameters. I have not reproduced this — our collisions all had mismatched arity — but it follows from the same mechanism, and it is the reason I think unique naming is worth doing rather than leaving prepare: false as the answer.
To be clear about scope: unique names would not make named prepared statements fully safe behind a transaction-mode pooler. A client can still believe a statement is prepared on a session that no longer holds it, which surfaces as 26000 prepared statement "…" does not exist; the existing evict / stale handling in PreparedCache looks like it already covers that case. Unique naming addresses specifically the case where one connection's name resolves to another connection's statement.
What do you see instead?
Intermittent bind message supplies N parameters, but prepared statement "effectX" requires M on unrelated queries, under load, with no application-level cause — the query named in the error is innocent, and which query draws the short straw varies between occurrences.
The workaround is prepare: false, which the config documents ("Disable it for poolers that cannot preserve named statements between queries") and which works. The cost is real but smaller than it sounds: the driver still sends Parse/Bind/Describe/Execute/Sync as a single socket write against the unnamed statement, so it is the backend's cached plan that is lost, not a round trip. Measured on our workload across the change, p50 request latency moved 188 ms → 209 ms and p90 244 ms → 253 ms.
Additional information
Cloudflare's How Hyperdrive works says it "supports named prepared statements as implemented in the postgres.js and node-postgres drivers" and that "named prepared statements in other drivers may have worse performance or may not be supported". @effect/sql-pg implements the wire protocol itself, so it falls under the latter — which means today the safe default for every Hyperdrive user on this driver is prepare: false, and they will only discover that after the errors start.
hi, i'd like to work on this. a per-connection counter makes sense on a dedicated backend, and it collides once hyperdrive or pgbouncer is sharing the name space. i'd name each prepare from a hash of the sql (or skip named prepares in transaction mode). actually, do you want unique names always, or only when a pooler is in transaction mode?
What version of Effect is running?
effect@4.0.0-rc.115,@effect/sql-pg@4.0.0-rc.115, on Cloudflare Workers (workerd), talking to Postgres through Cloudflare Hyperdrive. Present onmaininpackages/sql/pg/src/PgConnection.ts(PreparedCache).What steps can reproduce the bug?
Run any workload with the default
prepare(on) through a transaction-mode pooler — Hyperdrive, PgBouncer in transaction mode, Supabase's pooled port — with more than one connection in play:Under ordinary concurrency, queries start failing intermittently with a parameter count that has nothing to do with the query being run:
The statement in the failing query had two parameters. Some other statement, elsewhere in the application, has four.
The cause is that statement names are minted from a counter that restarts on every connection:
PreparedCacheis per connection, so the counter is too, andeffect5means "the fifth distinct statement this connection prepared". But a prepared statement name is scoped to the backend session, not to the client connection. A transaction-mode pooler multiplexes N client connections onto M backend sessions and returns the session to the pool at the end of each transaction, so two client connections — two entries in onemaxConnections: 5pool, or two Worker isolates — can each hold their own idea ofeffect5and be routed to the same session. The second one'sBindlands on the first one's plan.Nothing exotic is required: concurrent traffic and a pooler are enough. We saw four occurrences across half an hour of normal production load.
What is the expected behavior?
A name minted by one connection can never be mistaken for another connection's statement. The smallest fix is to namespace the counter per connection, e.g. a random component chosen at connection setup:
That costs a few bytes per
Parseand removes the collision entirely.Worth separating two hazards here, because only one of them is loud:
Bindand you get the error above. Noisy and confusing, but safe.Bindsucceeds against the other statement's plan and the backend executes the wrong SQL with your parameters. I have not reproduced this — our collisions all had mismatched arity — but it follows from the same mechanism, and it is the reason I think unique naming is worth doing rather than leavingprepare: falseas the answer.To be clear about scope: unique names would not make named prepared statements fully safe behind a transaction-mode pooler. A client can still believe a statement is prepared on a session that no longer holds it, which surfaces as
26000 prepared statement "…" does not exist; the existingevict/ stale handling inPreparedCachelooks like it already covers that case. Unique naming addresses specifically the case where one connection's name resolves to another connection's statement.What do you see instead?
Intermittent
bind message supplies N parameters, but prepared statement "effectX" requires Mon unrelated queries, under load, with no application-level cause — the query named in the error is innocent, and which query draws the short straw varies between occurrences.The workaround is
prepare: false, which the config documents ("Disable it for poolers that cannot preserve named statements between queries") and which works. The cost is real but smaller than it sounds: the driver still sendsParse/Bind/Describe/Execute/Syncas a single socket write against the unnamed statement, so it is the backend's cached plan that is lost, not a round trip. Measured on our workload across the change, p50 request latency moved 188 ms → 209 ms and p90 244 ms → 253 ms.Additional information
postgres.jsandnode-postgresdrivers" and that "named prepared statements in other drivers may have worse performance or may not be supported".@effect/sql-pgimplements the wire protocol itself, so it falls under the latter — which means today the safe default for every Hyperdrive user on this driver isprepare: false, and they will only discover that after the errors start.@effect/sql-mysql2, no way to disable prepared statements behind Hyperdrive). That asked for an escape hatch; here the escape hatch already exists and works, and the request is that the default not collide in the first place.