Repository navigation
Disable psycopg3 auto-prepare by default behind connection poolers - #74581
Merged
Merged
Conversation
Airflow 3.4 makes psycopg3 the default Postgres driver for both the sync and async metadata engines. psycopg3 prepares a statement server-side after prepare_threshold executions, which defaults to 5. Transaction pooling routes consecutive transactions on one client connection to different server connections, so a pooler that does not itself track prepared statements cannot honour that. PgBouncer only gained the ability in 1.24; the chart here pins 1.23.1 and sets pool_mode = transaction, so the deployment the project recommends is one of the affected ones. Under concurrency the scheduler crosses the threshold and tasks start failing with DuplicatePreparedStatement. The driver switch landed in two parts. The async half was validated against a transaction-mode PgBouncer; the sync half mirrored it without re-testing, and neither covered a pooler that declines to track prepared statements. Operators whose pooler does track them, or who run no pooler at all, can set prepare_threshold back through sql_alchemy_connect_args and keep the speedup.
The newsfragment PR-number check rejects edits to 68496.significant.rst from this PR, and rightly so. The stale claim is corrected where it outlives the release notes -- in set-up-database.rst -- and this release note says plainly that it supersedes the earlier one.
PgBouncer has supported prepared statements since 1.21, not 1.24 -- 1.24 only turned them on by default. The distinction matters: an operator on 1.21 to 1.23 can set max_prepared_statements and keep preparation on both sides rather than changing Airflow config, and the text sent them the wrong way. The async half of _get_connect_args had no coverage, and nothing proved the default survived into create_engine; both are tested now, and the whole set fails if the default is removed. Writing the engine-level test surfaced that conf_vars re-runs configure_vars and rebinds SQL_ALCHEMY_CONN, so a test that patched the URL first was asserting against the environment's sqlite conn. config.yml described both connect-args options as defaulting to an empty dict and told operators psycopg3 needed no extra args, which is the claim this change exists to refute. Also names the Celery result backend as out of scope: it builds its own engine from a derived psycopg3 URL and does not read these options.
Airflow docs are spellchecked against a US English dictionary.
test_sql_alchemy_connect_args compared create_engine's connect_args against the module-level dict the config points at, and passed because _get_connect_args handed that very object back. configure_orm then set check_same_thread on it, so the expectation mutated alongside the actual value. Merging driver defaults returns a copy instead, which breaks that coincidence on the sqlite and postgres jobs while mysql still passes. The expectation is now built from the merge, and the copy also stops Airflow permanently mutating a dict defined in someone's airflow_local_settings. Also says what happens when the configured value is not a mapping -- engine creation fails rather than quietly skipping the default -- and records that disabling preparation costs executemany its single prepared statement.
The fallback it describes requires a Celery deployment that never set [celery] result_backend, on 3.4, behind a transaction-mode pooler. Both the Helm chart and most deployments set it, so giving the case its own paragraph in a significant note overstates it and makes the fix read as partial.
kaxil
reviewed
Oct 12, 2026
kaxil
approved these changes
Oct 12, 2026
…facts Resolve the driver with make_url().get_driver_name() instead of matching the URL prefix, so the check agrees with the dialect that actually loads: sql_alchemy_conn_async is taken verbatim from config with no scheme normalization, and a bare postgresql:// resolves to psycopg on SQLAlchemy 2.1. Correct the PgBouncer requirements in the docstring, the docs and the newsfragment. psycopg puts the floor at PgBouncer 1.22 and libpq 17, not the 1.21 taken from PgBouncer's own NEWS, so anyone opting back in from that paragraph could land right back on the failure this fixes. Say explicitly that opting back in needs both the pooler and prepare_threshold, since the default forces it off regardless of what the pooler supports. Lead the newsfragment with the deployments that actually change: nothing released defaults to psycopg3, so it is the ones already setting postgresql+psycopg:// explicitly that silently lose preparation. Tests: point sql_alchemy_connect_args at real module-level dicts rather than patching getimport out, assert the default through configure_orm() and _configure_async_session() instead of hand-wiring create_engine, and cover the unparseable-URL branch.
ashb
reviewed
Oct 12, 2026
Per review, 69469's driver-default note and this one describe the same change for the same unreleased version, so they read better as a single entry than as two. Folds 69469.improvement.rst in and removes it; the PR-number CI gate skips removed files, so this needs no label.
Per review, the psycopg3 driver note belongs where the default was introduced. Towncrier types other than 'significant' are single-line only, so holding the combined body means 69469.improvement.rst becomes 69469.significant.rst. Keeping a foreign newsfragment in the diff trips check-newsfragment-pr-number, so this PR carries the 'skip newsfragment check' label.
vatsrahul1001
added
the
skip newsfragment check
The label landed a second after the previous push, so that run's event payload predated it and the gate evaluated its 'if' against a PR with no label. Re-running replays the same payload, so it needs a fresh push.
Collaborator
|
Thanks for fixing! Valuable lesson about sync/async testing.... |
1 task done
1 task done
28 of 93 tasks
1 task done
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.
Since 3.4 a bare
postgresql://metadata URL is rewritten topostgresql+psycopg://, so psycopg3 is now the default driver for both the sync and async engines. psycopg3 prepares a statement server-side once it has been executedprepare_thresholdtimes, which defaults to 5.Under a transaction-mode pooler the next transaction can land on a different server connection, so unless the pooler tracks prepared statements itself, the scheduler starts failing once it crosses that threshold:
This is not an exotic setup. The Helm chart sets
pool_mode = transaction(chart/templates/_helpers.yaml) and pins PgBouncer 1.23.1 (chart/values.yaml), and PgBouncer only enables prepared statements by default from 1.24. So the deployment we recommend is one of the affected ones.This sets
prepare_threshold = Nonefor psycopg3 URLs in_get_connect_args. Anything in[database] sql_alchemy_connect_argsis merged over the top and wins, so operators whose pooler does track prepared statements — or who run without one — can set it back and keep the optimisation.related: #68496, #69469
Reproducing
Postgres plus PgBouncer in transaction mode with
max_prepared_statements = 0, eight concurrent clients running one parameterised query each:It only shows up under concurrency, which is why a single-threaded check passes. The same harness against PgBouncer 1.26 passes without this change, since
max_prepared_statementsdefaults to 200 there.Why this wasn't caught earlier
The driver switch landed in two parts. #68496 moved the async default and was validated against a transaction-mode PgBouncer. #69469 moved the sync default and describes itself as mirroring that change; pooling isn't mentioned. Neither covered a pooler with prepared statements turned off, and the async conclusion doesn't carry over — the async engine reports
prepare_threshold=5too, so the fix applies to both.Also updated
config.ymldescribed both connect-args options as defaulting to an empty dict, and told operators the async psycopg3 driver needed no extra args.set-up-database.rstmade the same claim and now documents the default and how to opt out of it.Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 5) following the guidelines