Repository navigation
pg: survive lost connections and open TLS directly - #689
Merged
Merged
Conversation
When PostgreSQL or the network dropped a connection in the middle of a transaction, the whole process died with "null is not an object (evaluating 'socket.write')". postgres.js 3.4.9 answers the failed statement of sql.begin with a ROLLBACK written to the closed socket from a setImmediate, outside any promise. reserve().release() has the same flaw: it hands a dead connection back to the pool, and the next query sent on it crashes the same way. runPgTransaction now holds a reserved connection and sends BEGIN, COMMIT and ROLLBACK itself. Once the connection is lost, nothing more is sent on it and it is never released: the server has already rolled the transaction back, and the pool replaces a closed connection on its own. The transaction rejects with the connection error and the process keeps running. Migrations, which already drove their own transaction on a reserved connection, use the same helpers. isConnectionLost replaces the unused isConnectionError: porsager's closed or destroyed codes, socket resets, and any FATAL or PANIC server error, after which PostgreSQL always ends the session. A COMMIT that PostgreSQL answers with a ROLLBACK tag now fails. That happens when a statement failed and the callback caught its error, and it used to be reported as a success whose writes were gone. The fix for the same bug in postgres.js is porsager/postgres#1209. Until it ships, a connection lost between two statements still crashes, because nothing reports the loss before the next deferred write.
When PostgreSQL's shared lock table fills up, it aborts the transaction with "out of shared memory". The message reads like the server ran out of RAM; only the hint says which table filled up. A long serializable transaction hits this easily: it keeps the predicate locks of every serializable transaction that commits while it is open, and the table holds max_pred_locks_per_transaction × (max_connections + max_prepared_transactions) entries for all sessions. runPgTransaction now replaces that error with a [SixbPg] one that names the table, the setting that sizes it, and what to do about it, keeping the original as its cause. A 53200 whose hint names no lock setting is a genuine out-of-memory and passes through unchanged.
porsager negotiates TLS the classic way: an SSLRequest on a plain TCP
socket, then tls.connect({ socket }) to upgrade it. On Bun 1.4.0-1.4.2
such an upgraded socket keeps every byte it receives in memory for
good, so a process that reads a lot from a TLS database grows until it
is killed. Measured here: +537 MB for 384 MB read, against a flat
~100 MB when TLS is opened directly.
A connection string with sslnegotiation=direct, the libpq parameter
for PostgreSQL 17+, now makes @sixb/pg open the TLS connection itself
with tls.connect({ host, port }) and the "postgresql" ALPN protocol.
That avoids the leaking path and saves the SSLRequest round trip on
every connection. sslmode keeps its libpq meaning: require encrypts,
verify-ca checks the chain, verify-full also checks the host name.
It is opt-in, like in libpq, rather than tried first with a fallback:
a fallback would change the negotiation for every deployment, including
those behind a pooler or proxy, to work around a runtime bug that Bun
1.4.3 fixes. Misconfigurations fail when the client is created: a
sslmode that could fall back to plaintext, several hosts, or a Unix
socket. A server that does not accept direct TLS gets a [SixbPg]
explanation instead of a bare connection reset.
The socket factory returns at once and never throws: with postgres.js
3.4.9, a factory that fails leaves its pool slot stuck for good, and a
socket that is already dead stalls the slot's next attempt.
The test database now runs with TLS on; plain connections are still
accepted.
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
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
Three failures that took down long-running workers talking to a managed PostgreSQL over TLS:
pg_terminate_backend, network)null is not an object (evaluating 'socket.write'), thrown outside any promise53200)out of shared memory, which reads like the server ran out of RAM[SixbPg]error naming the table, the setting that sizes it, and what to dosslnegotiation=direct) keeps memory flat1. Transactions survive a lost connection
postgres.js 3.4.9 writes to a connection without checking that its socket is still open:
sql.beginanswers a failed statement by sendingROLLBACKon the closed socket, and the process crashes;reserve().release()hands a dead connection back to the pool, and the next query crashes the same way.runPgTransactionnow drives the transaction itself on a reserved connection:FATAL/PANICserver error (PostgreSQL always ends the session after those).release()loses nothing.COMMITwith aROLLBACKtag, not an error. We used to report that as a success with the writes discarded; it now throws.Known limit: a connection that drops between two statements still crashes, because nothing reports the loss before the next write. The upstream fix is porsager/postgres#1209.
2. Explicit error for a full lock table
The table is identified from PostgreSQL's hint. A
53200without a lock hint is a real out-of-memory error and passes through unchanged.3. Direct TLS, opt-in
sslnegotiation=directSSLRequeston plain TCP, thentls.connect({ socket })upgradetls.connect({ host, port })with ALPNpostgresqlsslmodekeeps its libpq meaning:requireencrypts,verify-cachecks the chain,verify-fullalso checks the host name.sslmodethat could fall back to plaintext, several hosts, or a Unix socket.[SixbPg]explanation, printed once, instead of a bare connection reset.Why opt-in rather than "try direct, fall back": it matches libpq, and it leaves every existing deployment unchanged, including those behind a pooler or proxy. A fallback would add a probe and a second code path to work around a runtime bug that Bun 1.4.3 fixes.
Why the socket factory returns at once: with postgres.js 3.4.9, a factory that throws leaves its pool slot stuck forever, and a socket that is already dead stalls the slot's next attempt. Failures are reported through the socket's own
error/closeevents, which porsager already handles.Linked issue
None.
Scope
@sixb/pgonly. No public contract changes beyond the documented connection-string parameter.storage.unavailablefailure. Nothing would read it yet; it comes with the run-lifecycle work, where attempts and last errors become visible.engineswill move to>=1.4.3once it is released.Validation
pg-connection-loss:pg_terminate_backendmid-transaction, run in a child process so that a crash cannot take down the runner. Proven by reverting: bothsql.beginand releasing a lost connection bring back the exact crash.pg-direct-tls:verify-fullrejects a self-signed certificate;COMMIT/ROLLBACKguard,53200, and the client configuration errors.bun run test:e2einstorage/pg(411 pass),bun run typecheck, andbun run check.Notes
25P02(current transaction is aborted).sql.beginused to rethrow the first error at the end instead.57P01onto the next query of the reconnected slot. One extra query fails (classified as a lost connection), then the pool is healthy. Settle startup queries when their connection keeps closing - fixes #1223, fixes #1193 porsager/postgres#1241 fixes only the startup variant.expect(sqlQuery).rejectsnever runs a porsager query, because the query only starts when itsthenis called, and Bun's matcher then waits forever. The new tests catch such errors explicitly.