diff --git a/docs/agents.md b/docs/agents.md index ab18448..af6b65a 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -138,8 +138,8 @@ asyncio.run(main()) With the `settings` extra, `create_async_session_manager(config)` builds the same manager from a `PostgresSettingsProtocol` and fills in the PgBouncer-safe defaults for you: -`AsyncCConnection`, both statement caches at 0, `application_name` as the only startup -parameter, and `db_schema` applied per transaction as `search_path` (rule 16). +both statement caches at 0, `application_name` as the only startup parameter, and +`db_schema` applied per transaction as `search_path` (rule 16). ## The API @@ -193,7 +193,8 @@ reusable: `build()` does not consume it. `create_async_session_manager(postgres_config, application_name=None, metrics=None, on_engine_created=None, connection_class=None, extra_server_settings=None, extra_connect_args=None, **kwargs)` takes a `PostgresSettingsProtocol` and returns a -manager. It defaults `connection_class` to `AsyncCConnection`, sends `application_name` — +manager. It defaults `connection_class` to `AsyncCConnection` (uuid statement names; it +plays no part in PgBouncer compatibility, rule 16), sends `application_name` — and `jit`, only when the config sets it — as `server_settings`, hands `db_schema` to the manager as `search_path`, and passes both statement cache sizes through. Your keys in `extra_server_settings` / `extra_connect_args` win over the library's; `server_settings` @@ -448,11 +449,18 @@ decorator. transaction. To have the schema without the round-trip use `ALTER ROLE … SET search_path`, `ALTER DATABASE … SET`, or schema-qualified metadata. A plain `SET` on a connection leaks to the next client through a pooler; never issue one in a `connect` - listener. Statement caches stay at 0 and `AsyncCConnection` stays, for PgBouncer before - 1.22 (`max_prepared_statements=0`); 1.22+ tracks prepared statements itself. + listener. With `statement_cache_size=0` (the default) any PgBouncer works: asyncpg + sends unnamed statements and parses them on every execution. `statement_cache_size > 0` + needs PgBouncer 1.21+ with `max_prepared_statements > 0`, which is the default from 1.24 + (1.21–1.23 ship it at 0). The connection class plays no part. `pre_ping=True` (the + default) costs a second PgBouncer transaction on every reused checkout and puts + PgBouncer's queue inside the checkout, outside `pool_timeout`; behind a transaction-mode + PgBouncer prefer `PoolSettings(pre_ping=False, recycle=)`. 17. **The wait for a connection and the time it was held are different metrics.** Alert on `postgres_db_connection_checkout_wait_seconds` — a rising wait is a pool about to run - out, and `postgres_db_connection_timeouts_total` is what it turns into. The held + out, and `postgres_db_connection_timeouts_total` is what the pool's own queue turns + into at `pool_timeout` (PgBouncer's queue, reached through the pre-ping, is not + bounded by it). The held duration, `postgres_db_connection_held_duration_seconds`, is query latency seen from the pool: it rises when the database slows down, whether or not the pool is under pressure. `postgres_db_connection_checkout_duration_seconds` is the held duration under diff --git a/docs/guide/advanced.md b/docs/guide/advanced.md index e6f8c36..ea7bcbc 100644 --- a/docs/guide/advanced.md +++ b/docs/guide/advanced.md @@ -314,7 +314,10 @@ one return behind. `pool.connect()` before a connection is handed out: the queue wait, plus the pre-ping and the connect handshake when the pool has to grow. It is the metric that predicts a pool outage — it climbs while the pool saturates, and turns into -`connection_timeouts_total` when it crosses `pool_timeout`. The held duration is the time +`connection_timeouts_total` when the wait in the pool's own queue crosses `pool_timeout`. +Behind a transaction-mode PgBouncer with `pre_ping=True` it also holds PgBouncer's queue, +which `pool_timeout` does not bound (see +[Configuration → PgBouncer](configuration.md#pgbouncer)). The held duration is the time between the `checkout` and `checkin` events, which is query duration seen from the pool: it climbs when the database slows down, whether or not the pool is under any pressure. A saturated pool moves both, and only the wait tells you which one caused the other. @@ -689,12 +692,15 @@ await wait_for_database(session_manager) ### PgBouncer Compatibility `create_async_session_manager` is safe behind PgBouncer in transaction mode as it stands: -both statement caches default to 0 and the connection class is `AsyncCConnection`, whose -statement names are unique per connection (both for PgBouncer before 1.22); `jit` is not -sent unless you set it; and `db_schema` is applied with `SET LOCAL` at the start of every -transaction rather than as a startup parameter. The one thing left to size is the pool: -PgBouncer owns the server connections, so `size` is how many client connections this -process may hold open. +both statement caches default to 0, so asyncpg sends unnamed statements that any PgBouncer +carries (raising `statement_cache_size` needs 1.21+ with `max_prepared_statements > 0`, the +default from 1.24); `jit` is not sent unless you set it; and `db_schema` is applied with +`SET LOCAL` at the start of every transaction rather than as a startup parameter. Two +things are left to you. Size the pool: PgBouncer owns the server connections, so `size` is +how many client connections this process may hold open. And consider `pre_ping=False`: +the ping is a PgBouncer transaction of its own on every reused checkout, and it is where +PgBouncer's queue lands, outside `pool_timeout`. A `recycle` below PgBouncer's +`client_idle_timeout` replaces what the ping protected against. ```python from sqlalchemy_foundation_kit.contrib.settings import ( @@ -705,7 +711,7 @@ from sqlalchemy_foundation_kit.contrib.settings import ( config = BasePostgresConfig( connection=ConnectionSettings(host="pgbouncer", port=6432, ...), - pool=PoolSettings(size=20, max_overflow=0), + pool=PoolSettings(size=20, max_overflow=0, pre_ping=False, recycle=300), application_name="my-service", db_schema="app", ) diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index 5632ad0..923abdd 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -174,7 +174,7 @@ ConnectionSettings( | `kind` | `PoolClassStr` | `"async_adapted_queue"` | Pool implementation (`async_adapted_queue`, `static`, `null`) | | `size` | `int` | `10` | Number of connections the pool keeps open (minimum 1); queue pools only | | `max_overflow` | `int` | `20` | Additional connections when pool is exhausted (0 or more); queue pools only | -| `pre_ping` | `bool` | `True` | Check connection health before use (recommended) | +| `pre_ping` | `bool` | `True` | Check connection health before use (recommended on a direct connection; behind PgBouncer see [PgBouncer](#pgbouncer)) | | `recycle` | `int` | `3600` | Recycle connections after N seconds (-1 = never) | | `timeout` | `float` | `30.0` | Seconds to wait for connection before raising error; queue pools only | @@ -434,12 +434,13 @@ run twice, so every cache fills. The scripts are on |---|---|---| | plain SQLAlchemy + asyncpg, driver defaults | PostgreSQL directly | 800 of 800 ok | | plain SQLAlchemy + asyncpg, driver defaults | PgBouncer, defaults (`max_prepared_statements=200`) | 800 of 800 ok | -| plain SQLAlchemy + asyncpg, driver defaults | PgBouncer, `max_prepared_statements=0` (the default before 1.22) | 485 of 800 ok; 315 × `prepared statement "__asyncpg_stmt_…" does not exist` | +| plain SQLAlchemy + asyncpg, driver defaults | PgBouncer, `max_prepared_statements=0` (the default before 1.24) | 485 of 800 ok; 315 × `prepared statement "__asyncpg_stmt_…" does not exist` | | `create_async_session_manager(config)`, 0.2.1 | PgBouncer, defaults | 0 of 800: every connection refused with `unsupported startup parameter: jit` | | `create_async_session_manager(config)`, 0.2.1 | PgBouncer, `ignore_startup_parameters=jit,search_path` | 800 of 800 ok — and `SHOW jit` is `on`, `SHOW search_path` is `"$user", public`: both parameters silently dropped | | `create_async_session_manager(config)`, current | PgBouncer, defaults | 800 of 800 ok; `SHOW search_path` is the configured `db_schema` | -Three things follow, and the library is built around each. +Four things follow. The library is built around the first three; the fourth is a setting +you choose. **Startup parameters do not pass through.** asyncpg's `server_settings` are parameters of the PostgreSQL startup packet. PgBouncer forwards only the ones it tracks — @@ -462,22 +463,50 @@ the start of every transaction, and why the alternatives are `ALTER ROLE … SET `ALTER DATABASE … SET`, `track_extra_parameters`, or schema-qualified metadata — never a `SET` in a `connect` listener. -**Prepared statements are fine on a current PgBouncer, and still not on an old one.** -Since 1.22 PgBouncer tracks protocol-level prepared statements itself -(`max_prepared_statements=200` by default), so the driver's default statement cache runs -clean. Before 1.22, or with `max_prepared_statements=0`, a cached statement name is unknown -to the server connection the next transaction lands on and the classic failure comes back. -`QuerySettings` defaults both caches to 0 and `create_async_session_manager` uses -`AsyncCConnection`, whose statement names are unique per connection, so the library is -safe on either; on 1.22+ you may raise the caches to win the round-trip back. +**With the statement caches at 0, any PgBouncer works.** With `statement_cache_size=0` +(the default) asyncpg sends unnamed statements and parses them again on every execution, +so no statement depends on what a server connection remembers. `statement_cache_size > 0` +names statements and reuses them in later transactions, which may land on a different +server connection. That needs PgBouncer to track prepared statements itself: 1.21 or later +with `max_prepared_statements > 0`. The setting was added in 1.21 at 0 (off) and has +defaulted to 200 since 1.24 — measured, a stock 1.21.0, 1.22.1 and 1.23.1 report +`max_prepared_statements=0`, a stock 1.24.1 and 1.25.2 report 200. Without tracking, every +cache combination with `statement_cache_size > 0` fails with +`prepared statement "__asyncpg_stmt_…" does not exist`. The connection class plays no part: +`AsyncCConnection` and a plain `asyncpg.Connection` passed and failed in exactly the same +runs ([issue #60](https://github.com/bedrock-python/sqlalchemy-foundation-kit/issues/60)). +On a tracking PgBouncer you may raise the caches to win the parse round-trip back. + +**Pre-ping costs a PgBouncer transaction, and moves PgBouncer's queue out of +`pool_timeout`.** `pre_ping=True` (the default) pings every connection the pool reuses, +and SQLAlchemy runs that ping on asyncpg as its own `BEGIN` / `;` / `ROLLBACK`. Through a +transaction-mode pooler that is a separate server assignment before the request's own +transaction: ten requests cost twenty PgBouncer transactions (measured, +[issue #58](https://github.com/bedrock-python/sqlalchemy-foundation-kit/issues/58)). And +when all of PgBouncer's server connections are busy, the request waits in PgBouncer's +queue inside the ping, that is inside `pool.connect()`: the wait is recorded as +`connection_checkout_wait_seconds`, `pool_timeout` does not bound it (a pool with +`timeout=0.2` recorded a 1.0 s checkout wait and no timeout), and only PgBouncer's +`query_wait_timeout` (120 s by default) ends it. Behind a transaction-mode PgBouncer, +`pre_ping=False` is usually the better trade. What the ping buys back is a connection +closed while idle: PgBouncer closes a client connection after `client_idle_timeout`, and +without the ping the next request on it fails (`InterfaceError: connection is closed`) +before SQLAlchemy discards it. Set `recycle` below `client_idle_timeout`, and below any +load balancer's idle timeout, and the pool retires the connection first — measured with +`client_idle_timeout=2`: `pre_ping=False, recycle=3600` failed the request after 3 s idle, +`pre_ping=False, recycle=1` did not. ```python -# Behind PgBouncer in transaction mode this is all it takes +# Behind PgBouncer in transaction mode config = BasePostgresConfig( connection=ConnectionSettings(host="pgbouncer", port=6432, ...), application_name="my-service", db_schema="app", # SET LOCAL at the start of every transaction: arrives # jit stays None # nothing sent: the connection is accepted + pool=PoolSettings( + pre_ping=False, # no extra PgBouncer transaction per checkout + recycle=300, # below PgBouncer's client_idle_timeout and any LB idle timeout + ), ) manager = create_async_session_manager(config) ``` @@ -547,7 +576,8 @@ PoolSettings(size=2, max_overflow=0) The defaults already do the right thing — statement caches at 0, `jit` unset, `db_schema` applied per transaction. Do not put `jit`, `search_path` or any other untracked parameter -into `extra_server_settings`; see [PgBouncer](#pgbouncer). +into `extra_server_settings`. Consider `pre_ping=False` with a `recycle` below +PgBouncer's `client_idle_timeout`; see [PgBouncer](#pgbouncer). ### 6. Enable Metrics in Production @@ -559,9 +589,11 @@ BasePostgresConfig( ``` The series to watch is `postgres_db_connection_checkout_wait_seconds` — how long callers -wait for a connection. It climbs before anything fails, and when it reaches the `timeout` -above, callers start getting `sqlalchemy.exc.TimeoutError` and -`postgres_db_connection_timeouts_total` starts moving. A rising +wait for a connection. It climbs before anything fails, and when the wait in the pool's own +queue reaches the `timeout` above, callers start getting `sqlalchemy.exc.TimeoutError` and +`postgres_db_connection_timeouts_total` starts moving. Behind PgBouncer with +`pre_ping=True` the wait also includes PgBouncer's queue, which `timeout` does not bound +(see [PgBouncer](#pgbouncer)). A rising `postgres_db_connection_held_duration_seconds` with a flat wait is a slower database, not a small pool; a rising wait with a flat held duration is a pool that needs more connections. See [Advanced → Prometheus Metrics](advanced.md#prometheus-metrics) for the full list. diff --git a/sqlalchemy_foundation_kit/session/connection.py b/sqlalchemy_foundation_kit/session/connection.py index 9ab1d3d..4c9926d 100644 --- a/sqlalchemy_foundation_kit/session/connection.py +++ b/sqlalchemy_foundation_kit/session/connection.py @@ -1,4 +1,4 @@ -"""Custom connection class for pgbouncer compatibility (async).""" +"""asyncpg connection class with uuid prepared-statement names.""" import uuid @@ -6,14 +6,21 @@ class AsyncCConnection(asyncpg.Connection): - """Custom async connection class for pgbouncer magic. - - This subclass overrides only the private method _get_unique_id so that - prepared statement identifiers are unique per connection. That is required - when using pgbouncer in transaction mode, where the same server connection - may be reused for different logical connections. - - See: https://github.com/sqlalchemy/sqlalchemy/issues/6467 + """asyncpg connection whose prepared statements are named with a uuid. + + Overrides only the private ``_get_unique_id``, so statement names are unique across + every connection rather than counted per connection. The only failure that prevents + is two clients colliding on a name on a shared server connection + (https://github.com/sqlalchemy/sqlalchemy/issues/6467). That needs a transaction-mode + PgBouncer that does not track prepared statements, where a named statement fails + anyway with ``prepared statement ... does not exist``. + + It is not what makes the library work behind PgBouncer. With + ``statement_cache_size=0`` (the default) asyncpg sends unnamed statements and asks for + no name at all; with ``statement_cache_size > 0`` PgBouncer has to track prepared + statements itself (1.21+ with ``max_prepared_statements > 0``, the default from 1.24), + and then rewrites the names per server connection. Kept as the default connection + class of :func:`create_async_session_manager` because it is harmless. """ def _get_unique_id(self, prefix: str) -> str: diff --git a/sqlalchemy_foundation_kit/session/factories.py b/sqlalchemy_foundation_kit/session/factories.py index 589d742..b288767 100644 --- a/sqlalchemy_foundation_kit/session/factories.py +++ b/sqlalchemy_foundation_kit/session/factories.py @@ -44,8 +44,11 @@ def create_async_session_manager( metrics: Optional metrics collector for connection pool monitoring. on_engine_created: Optional callback invoked with the AsyncEngine right after creation. Use it to attach OpenTelemetry instrumentation, custom listeners, etc. - connection_class: Custom asyncpg Connection subclass. Defaults to ``AsyncCConnection`` - which provides pgbouncer transaction-mode compatibility. + connection_class: Custom asyncpg Connection subclass. Defaults to ``AsyncCConnection``, + which names statements with a uuid. PgBouncer compatibility does not depend on + it: with ``statement_cache_size=0`` (the default) asyncpg sends unnamed + statements that any PgBouncer carries, and ``statement_cache_size > 0`` needs + PgBouncer 1.21+ with ``max_prepared_statements > 0`` whatever the class. extra_server_settings: Additional PostgreSQL ``server_settings`` to merge with defaults (e.g., ``{"statement_timeout": "30000", "timezone": "UTC"}``). User-provided keys override library defaults. These are startup parameters: through PgBouncer only