Repository navigation
Reuse the TLS connector across connection attempts - #4242
Merged
Merged
Conversation
goffrie
force-pushed
the
tls-cache
branch
3 times, most recently
from
April 21, 2026 22:44
9db9a93 to
e4b068a
Compare
TheSentinel454
added a commit
to block/buzz
that referenced
this pull request
Nov 6, 2026
## Problem sqlx 0.9.0 counts a transaction only after Postgres acknowledges its `BEGIN`. If the future is dropped while `BEGIN` is still in flight (a cancelled REQ history read, a client that disconnects, a timeout), sqlx doesn't queue a `ROLLBACK`. The pool's release ping then reads the late `BEGIN` reply, and the writer pool hands the connection to the next borrower while it is still inside that transaction. The reader pool already closes such connections in its release check (`Db::read_pool_options`). The writer pool has no release check. Load tests showed what happens next: - `there is already a transaction in progress` (25001) on the next event read's `BEGIN`. - Sometimes a burst of `current transaction is aborted` (25P02). The leftover from the event read is `READ ONLY`, so the next write fails and aborts it, and every later statement on that connection fails until the connection is closed. During a burst, ban checks failed closed with 503 and window reads returned 500. - A read-write leftover would also leave autocommit writes uncommitted, and the next read's `ROLLBACK` would discard them. ## Change - Pin every sqlx crate to transact-rs/sqlx#4394 (merged, not yet released). That commit takes the transaction count before the `BEGIN` round trip, so a cancelled begin queues its `ROLLBACK`. The crates share types, so they have to move together. The pin also picks up the other 23 commits between v0.9.0 and that fix, including the libsqlite3-sys fix `sqlx-sqlite` was already pinned for. - Add a regression test, `writer_pool_postgres_tests::begin_cancelled_in_flight_leaves_no_writer_transaction_open`. It holds `BEGIN` in flight behind a paused proxy and times it out, for both the event read's `BEGIN ... READ ONLY` and a plain `begin()`. It then asserts that the next borrower's autocommit write commits and that its backend is `idle`, not inside a transaction. Other upstream changes in the pin that reach the Postgres path: `TCP_NODELAY` on sockets (transact-rs/sqlx#4336), reuse of the TLS connector across connection attempts (transact-rs/sqlx#4242), a fix for a memory leak when the statement cache is disabled (transact-rs/sqlx#4337), a describe-time generic-plan change used only by the macros (transact-rs/sqlx#4286), and the `whoami` default-user fix (transact-rs/sqlx#4303). The rest are SQLite, MySQL, CLI, or docs changes. Remove the pin once sqlx 0.9.1 ships. ## What got simpler The lockfile loses its second `sqlx-core` copy, which the sqlx-sqlite-only pin used to pull in. No runtime code changes. A pool-level release check would have fixed the same bug, but it costs a round trip on every writer release. Comments that described the 0.9.0 behavior as current are refreshed: the reader pool's release check (`Db::read_pool_options`) and the lease heartbeat (`EntityDeletionStore::heartbeat`). The reader test `proof_timeout_mid_begin_leaves_no_reader_transaction_open` asserted that the release check closed the connection left mid-`BEGIN`. With the pin, sqlx rolls that transaction back itself, so the test now asserts the check finds nothing to close. That also catches a sqlx regression the check would otherwise hide. Identified debt (not in this PR): the release check's open-transaction arm, and the reader-session `default_transaction_isolation` pin it depends on, are now only a backstop against a sqlx regression. They could be removed in a follow-up. ## Testing - Against sqlx 0.9.0 (main's `Cargo.toml`/`Cargo.lock` at 962b3fd), the new writer test fails for each `BEGIN` form on its own. The event read fails with `cannot execute INSERT in a read-only transaction`, and after a plain `begin()` the next borrower's backend is not `idle`. With the pin it passes 5/5. - Single-machine load-test A/B against main (962b3fd), with three main runs and two pin runs. Main logged 25001 and 25P02 in every run (41/80, 10/9, 12/18); the pin logged none. Client error rate, latency, and throughput matched within run-to-run noise. - Full suite on the pin: every `just ci` recipe, the PostgreSQL lane, and `just test`. The only failure from the pin was the reader test above. Reds that don't involve sqlx: the desktop Tauri recipes (`alsa-sys` needs ALSA headers on the build host), `mobile-check`/`mobile-test` (a Flutter SDK `ColorScheme` type clash), and two `buzz-acp` `git_bootstrap` tests (no git credentials on the host). Two bridge search-timeout tests failed only under the full parallel lane; run alone they pass on both the pin and 0.9.0. - Two later commits change only tests, comments, and `Cargo.lock` (the lock now moves only the sqlx crates). At those heads: the PostgreSQL lane, the unit lane, and workspace clippy pass. Three tests failed once under a contended full lane, and each passed 3/3 alone on both the pin and 0.9.0. The two transaction tests fail on 0.9.0 and pass on the pin. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Luke Tornquist <[email protected]> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <[email protected]>
endoze
added a commit
to endoze/sqlx
that referenced
this pull request
Nov 9, 2026
…ler-supplied rustls `ClientConfig` Builds on transact-rs#4051 and transact-rs#4242.
endoze
added a commit
to endoze/sqlx
that referenced
this pull request
Nov 9, 2026
…ler-supplied rustls `ClientConfig` Builds on transact-rs#4051 and transact-rs#4242.
endoze
added a commit
to endoze/sqlx
that referenced
this pull request
Nov 10, 2026
…ler-supplied rustls `ClientConfig` Builds on transact-rs#4051 and transact-rs#4242.
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.
Creating a TLS connector (native_tls::TlsConnector or rustls::ClientConfig) is often expensive, primarily because it needs to load and parse the trusted roots. However, TLS settings are rarely changed during a pool's lifecycle, so the cost can be amortized by reusing the connector.
This PR:
Arc<OnceLock>so that clones of the same ConnectOptions will share TLS connectors. The cache is cleared if any relevant settings are changed.To avoid behaviour changes we still create the connector at the moment that it's first needed, and only cache it if the connector was successfully created.
Does your PR solve an issue?
no
Is this a breaking change?
No; it is only a breaking change in sqlx-core which is not subject to semver