Skip to content

fix(psycopg): check connection health on pool checkout - #2285

Open
RitiGrover wants to merge 2 commits into
tortoise:developfrom
RitiGrover:fix/psycopg-pool-connection-check
Open

RitiGrover wants to merge 2 commits into
tortoise:developfrom
RitiGrover:fix/psycopg-pool-connection-check

Conversation

@RitiGrover

Copy link
Copy Markdown

Description

PsycopgClient.create_connection builds the psycopg_pool without a check callback, so getconn() never validates a connection before handing it out. A connection silently killed by the network (idle firewall/LB timeout, DB restart) sits in the pool looking fine until something tries to use it, then fails hard.

Motivation and Context

Fixes #2007. Users see psycopg.OperationalError: the connection is closed on in_transaction() after the app has been running a few days, with no automatic recovery.

Passing check=AsyncConnectionPool.check_connection uses psycopg_pool''s own built-in liveness probe on checkout, so a dead connection gets replaced transparently instead of surfacing to the caller.

How Has This Been Tested?

  • Added a regression test asserting the pool is constructed with the check enabled
  • Verified it fails against the old code and passes with the fix
  • Ran the existing backend test suite (test_connection_params.py, test_password_factory.py, and the rest of the non-DB-dependent tests under tests/backends/) - all pass
  • ruff and mypy clean on the changed files

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have added the changelog accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Connections silently dropped by the network while idle (firewall/LB
timeout, DB restart) were handed straight to callers, failing with
psycopg.OperationalError: the connection is closed. Pass psycopg_pool's
own check_connection callback so the pool validates and replaces dead
connections on checkout.

Fixes tortoise#2007
@codspeed

codspeed Bot commented Sep 17, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 24 untouched benchmarks


Comparing RitiGrover:fix/psycopg-pool-connection-check (180ac4d) with develop (8477e47)

Open in CodSpeed

@waketzheng

Copy link
Copy Markdown
Contributor
  1. Consider adding a behavioral test that simulates a dead connection and asserts getconn() replaces it, in addition to the construction-level test.
  2. Add a brief note about the per-checkout probe overhead (even if negligible)
  3. Confirm the CHANGELOG entry is present and references #2285.

Per review: verifies getconn() actually replaces a connection killed
on the server side (conn.closed stays False until used, exactly like
a network-dropped connection) by killing a real backend and confirming
the next ORM call succeeds instead of raising OperationalError.
Confirmed failing without the fix (psycopg.errors.AdminShutdown) and
passing with it. Also note the per-checkout probe overhead in the
changelog and point it at the PR rather than the issue.
@RitiGrover

Copy link
Copy Markdown
Author

Addressed all three: added a behavioral test that kills a real backend connection and confirms the next ORM call still succeeds (confirmed it fails with psycopg.errors.AdminShutdown without the fix), added a note on the per-checkout probe overhead in the changelog, and pointed the changelog entry at this PR instead of the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Getting psycopg.OperationalError constantly after a while on transactions

2 participants