Fixes #358: Close branch connections after CONN_MAX_AGE expires #364
+146
−1
Conversation
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
Django's close_old_connections() only handles database aliases found in DATABASES.keys(). Branch aliases (schema_*) are served dynamically via DynamicSchemaDict.__getitem__ but aren't yielded by __iter__, so they were never cleaned up, causing connection leaks. Solution: Track branch connection aliases in thread-local storage when they're first accessed. Register cleanup handler on request_started and request_finished signals to close expired branch connections using the same timing as Django's built-in cleanup. Implementation uses public APIs only (asgiref.Local, Django signals, connections.close_if_unusable_or_obsolete) and matches Django's pattern where DATABASES.keys() is effectively static - aliases are tracked once and never removed. Tests verify connections close after CONN_MAX_AGE for single branch, multiple branches, and deleted branch schemas.
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.
Fixes: #358
Branch-specific database connections never close after
CONN_MAX_AGEexpires, causing connection exhaustion when users visit multiple branches. Django'sclose_old_connections()iteratesDATABASES.keys()to discover aliases/names for connections to cleanup, but branch aliases are served dynamically viaDynamicSchemaDict.__getitem__()without being yielded by__iter__(), so they're never checked for expiry. They're essentially invisible to Django'sCONN_MAX_AGEmachinery.Solution Approaches
Caching aliases in
DATABASESdict: Fixed cleanup but broke all 43 existing tests with "Database connections not allowed in this test." Django's test framework checksDATABASES.keys()during setup and blocks non-default connections. The issue wasn't just about adding databases = 'all' to existing tests (or to some base test class), which this would have required. The deeper concern was that by making branch aliases visible in DATABASES.keys() during test setup, we'd be changing the fundamental isolation guarantees. Tests that never needed to know about branch databases would suddenly have them in scope, potentially causing cross-contamination between tests or allowing tests to accidentally depend on branch state.Custom
__iter__using private API: Usedconnections._connections._storage.__dict__to discover cached aliases. Fixed cleanup and all tests passed, but relied on undocumented Django internals. Rejected for maintenance concerns.(what I ended up with) Track branch aliases in our own thread-local storage when first accessed, register cleanup handlers on request_started and request_finished signals. Uses only public APIs (
asgiref.Local, Django signals,connections.close_if_unusable_or_obsolete). Matches Django's pattern whereDATABASES.keys()is static and implements thread-specific tracking in the same manner as Django.