Use Django connections in the geomap data view#4113
Open
adobloug wants to merge 1 commit into
Open
Conversation
The geomap data view was the last multithreaded web caller of the legacy nav.db connection cache (ObjectCache). Under mod_wsgi that cache is shared across request threads and was the path that triggered the connection-leak race in Uninett#4104. Switch the view to Django's connection (django.db.connection), which is thread-local and pooled via CONN_MAX_AGE, so no shared cache is involved. The geomap queries still need a psycopg2 DictCursor, so we reach through to the underlying psycopg2 connection and close only the cursor -- Django owns the connection's lifecycle. The explicit set_isolation_level(1) call is dropped; Django runs in autocommit and READ COMMITTED is PostgreSQL's default. The remaining nav.db callers are non-web scripts and daemons that run outside Django's request lifecycle, so the legacy cache stays for them. Added a unit test asserting the view uses the Django-managed connection and never closes it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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
2 participants
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.
Scope and purpose
Follow-up to #4111 (the #4104 connection-leak fix). That PR hardened the
legacy
nav.dbObjectCacheso it survives concurrent access. This PRremoves the last multithreaded web caller of that cache, which was the
actual path that triggered the race under mod_wsgi.
The geomap
dataview opened its cursor vianav.db.getConnection(),sharing the process-wide
ObjectCacheacross request threads. It now usesDjango's
django.db.connection, which is thread-local and pooled viaCONN_MAX_AGE— the same pattern already used by the report system and 10+other NAV modules.
Because the geomap queries need a
psycopg2DictCursor, the view reachesthrough to the underlying psycopg2 connection (
connection.connection) andcloses only the cursor; Django owns the connection's lifecycle. The explicit
set_isolation_level(1)call is dropped — Django runs in autocommit andREAD COMMITTED is PostgreSQL's default.
The remaining
nav.dbcallers are non-web scripts and daemons (snmptrapd,smsd, logengine, cron bins) that run outside Django's request lifecycle, so
the legacy cache stays in place for them.
This pull request
ObjectCacheContributor Checklist
master)test_geomap_data_should_not_crashexercises the view end-to-end)🤖 Generated with Claude Code