feat: stop sending jit and search_path as startup parameters - #24
Merged
Conversation
`BasePostgresConfig.jit` defaults to None and is put into asyncpg
`server_settings` only when set explicitly. `db_schema` no longer goes
as a `search_path` startup parameter: `create_async_session_manager`
hands it to the manager, which attaches a `begin` listener running
`set_config('search_path', ..., true)` -- SET LOCAL -- as the first
statement of every transaction. That is the one scope PgBouncer in
transaction mode honours: it refused every connection on the default
`jit` (`unsupported startup parameter: jit`) and, with
`ignore_startup_parameters`, silently dropped the schema.
New: `AsyncSessionManager(search_path=...)`,
`AsyncSessionManagerBuilder.with_search_path()`,
`session.manager.attach_search_path(engine, search_path)`.
Two defaults change for direct-to-PostgreSQL users: JIT follows the
server setting unless `jit` is set, and the schema is per transaction
rather than per connection (a statement under AUTOCOMMIT is not
covered). The configuration guide's PgBouncer section is rewritten
around the measurements in the issue; the agents page follows.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
BasePostgresConfig.jitdefaulted to"off", andcreate_async_session_managersent it — anddb_schemaassearch_path— in asyncpgserver_settings, which are PostgreSQL startup parameters. PgBouncer in transaction mode refuses any it does not track, so with the defaults every connection failed withunsupported startup parameter: jit; withignore_startup_parametersit connected and dropped both, sodb_schemawas accepted by the library and ignored by the database. The configuration guide saidjit="off" # Required for pgbouncer.What changes.
jitdefaults toNoneand goes intoserver_settingsonly when set; it is still a startup parameter then, and the docs say what that means through a pooler (track_extra_parameters, orALTER ROLE … SET jit).db_schemanever goes as a startup parameter again:create_async_session_managerhands it to the manager assearch_path, and the manager attaches abeginlistener to the engine that runsSELECT set_config('search_path', $1, true)— the function form ofSET LOCAL, value as a bind parameter — as the first statement of every transaction. Under the asyncpg adapterBEGINis sent lazily with the first statement, so this lands inside the transaction, after PgBouncer has pinned the server connection. New surface:AsyncSessionManager(search_path=None),AsyncSessionManagerBuilder.with_search_path(), andattach_search_path(engine, search_path)besideattach_metricsinsession.manager(not re-exported). Nothing is attached when no schema is configured; with one it costs one round-trip per transaction. Statement caches at 0 andAsyncCConnectionstay, for PgBouncer before 1.22.Why the engine and not the unit of work.
SET LOCALinopen_session()covers only the three UoW blocks:manager.get_session(),get_transaction(), a rawengine.connect()and the statements after asession.commit()insidemanaged_session()would all run inpublic, anddb_schemawould have to be threaded into both DI integrations' UoW providers. Thebeginevent covers all of those from one place, including the transaction that follows a commit in the same session, and fires once per transaction rather than per savepoint. Aconnect-eventSET search_pathwas rejected because it is per server connection, which through a transaction pooler is exactly the leak the lab's last case shows. Docs-only was rejected because it leaves a setting the library accepts and the database ignores. The one shape the listener cannot cover is a statement underisolation_level="AUTOCOMMIT"— no transaction, nothing to scope to — which is equally true of any per-connection state through a pooler; the docs say so.Direct-to-PostgreSQL users see two changed defaults: JIT now follows the server setting unless
jitis set, and the schema is applied per transaction rather than per connection, which is the same effective result for anything that runs in a transaction. That is why this isfeat:and a minor bump rather than a patch. The agents page follows the code: the new argument and builder method,jit=Nonein the config table, rule 16, a row in the fixed-since table, a common-mistakes pair and an errors row; the configuration guide gets a PgBouncer section built around the numbers below.Tests. Unit: the default config yields
server_settings == {"application_name": …}and nosearch_path; an explicitjit="off"is still inserver_settings;db_schemareaches the manager, not the startup packet;attach_search_pathregisters abeginlistener that executesset_configwith the value; builder and manager wiring. Integration on PostgreSQL 17:search_pathinget_session(),get_transaction()with and without an isolation level,engine.connect(), after acommit()in the same session, an unqualified table resolving in the schema, the reporter'sBasePostgresConfig(db_schema="app")throughcreate_async_session_manager,uow.transaction(),uow.query(),uow.managed_session()across a commit, inside and after a savepoint, and a control with no schema reading"$user", public. Run against master in a separate worktree: 6 unit tests fail,test_manager.pyfails to import, 12 integration tests error on the unknownsearch_pathargument; the explicit-jittest and the no-schema control pass on both, as they should.Gate:
The reporter's two scripts, unmodified, from a scratch venv against 0.2.1 from PyPI and then against this branch (
uv pip install -e ".[settings]" asyncpg "testcontainers[postgres]"), PostgreSQL 17 and PgBouncer 1.25.2 in Docker.pgbouncer_lab.pyon 0.2.1:pgbouncer_lab.pyon this branch:jit_probe.pyon 0.2.1:jit_probe.pyon this branch:The probe's direct row now reads
jit='on'because nothing is sent and the server default applies; the PgBouncer row readssearch_path='app'because the schema is set inside the transaction the probe'sSHOWautobegins, soignore_startup_parametershas nothing left to drop. The plain-SQLAlchemy rows are the same in both runs, as they should be. The kit rows' timings are Docker noise: a rerun on this branch gave 0.5s for both.Closes #23