Refactor: N+1 query by lalver1 · Pull Request #3867 · cal-itp/benefits

lalver1 · 2026-07-10T16:32:17Z

This PR installs the Django Debug Toolbar (DDT) to help diagnose N+1 Query issues. To turn on the toolbar, add a DJANGO_DEBUG_TOOLBAR environment variable to your .env and set it to true. It also makes the following changes to get rid of some redundant queries:

session refactor 1 - NULL agency/flow (aba8325 and 0363ca7): If an agency or a flow isn't selected (the user hasn't reached that part of the application), we shouldn't query the database.
session refactor 2 - cache agency/flow (078b9d4 and 6d68958): This one feels a bit hacky but it is restricted to only two places in the session module and it does remove a lot of redundant queries, especially in the eligiblity part of the application. Creating a sort of caching layer middleware would definitely be cleaner and maybe it's something we can come back to.
Lazy evaluation (2cae695): Simple change to remove casting the results of a QuerySet as a list (which forces a query evaluation) and keeping them as a QuerySet until the whole result needs to be evaluated.
Use select_related() (22deca9): Implements a SQL JOIN in a single database query. It limits the related fields to transit_processor_config which is a ForeignKey and accessed by the _agency_context context processor.

Interpreting SQL results from the DDT

To look for redundant queries, look for SQL items that say Duplicated N times.. For example, the screenshot below highlights a SELECT query duplicated 5 times in the DDT. Note that each instance (duplicate) of the query is shown as a unique item (the screenshot below has been truncated to two items, if you were to keep scrolling down you'd see all five items listed).

So a helpful method to troubleshoot redundant queries is:

Look for items that show Duplicated N times.
For each of these items, expand the result and look at the stack trace. The trace will show which .py file (or template) created the query.
Typically the stack trace will be the same for each of the duplicates. These are the "easier" fixes. Sometimes though, the same query is generated at different places in the code so fixes need to be made in different places.
After you make the fix, you should see the total number of SQL queries go down by the amount that had been shown in Duplicated N times.

Reviewing

Turn on the DDT. Confirm the following drop in the number of queries by checking out the commit associated with each refactor and running through the application:

page	`main` (or `19cf724`)	`NULL` agency/flow	`cache` agency/flow	lazy group agencies	`select_related()`
`/`	25	16	16	14	5
`/providers`	35	31	24	20	11
`/eligibility` (regional)	36	32	25	21	12
`/eligibility`	35	31	23	21	12
`/eligibility/start` (regional)	42	42	26	23	14
`/eligibility/start`	43	43	25	23	14

and configure Django to use it. To activate it, set the environment variable DJANGO_DEBUG_TOOLBAR=true. The toolbar interferes with Pytest, so to run tests in VS Code or from the run.sh script, set DJANGO_DEBUG_TOOLBAR=false.

github-actions · 2026-07-10T16:33:17Z

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
benefits
settings.py					111, 114-122
urls.py					83-85
benefits/core
context_processors.py
session.py
benefits/core/models
transit.py
Project Total

_{This report was generated by python-coverage-comment-action}

If session.flow equals None, this will avoid queries that look like SELECT ... WHERE "core_enrollmentflow"."id" IS NULL and which are unnecessary. If session.flow does not exist (KeyError), using .get on the session ensures safety so we can remove KeyError from the try...except.

If session.agency equals None, this will avoid queries that look like SELECT ... WHERE "core_transitagency"."id" IS NULL and which are unnecessary. If session.agency does not exist (KeyError), using .get on the session ensures safety so we can remove KeyError from the try...except.

The result from getting the transit agency from the request's session is cached to avoid querying the database every time session.agency(request) is called during the same request-response cycle.

The result from getting the flow from the request's session is cached to avoid querying the database every time session.flow(request) is called during the same request-response cycle.

which will avoid making extra queries by removing list, since the conversion to a list is not actually needed.

by using select_related() which implements a SQL JOIN in a single database query. Limit the related fields to transit_processor_config which is a ForeignKey and accessed by the _agency_context processor.

thekaveman · 2026-07-14T14:59:51Z

-        return models.TransitAgency.by_id(request.session[_AGENCY])
-    except (KeyError, models.TransitAgency.DoesNotExist):
+        agency = models.TransitAgency.by_id(agency_id)
+        request._cached_agency = agency


Is there a more "Django" way to do this caching? Attaching a random prop on to the request feels a little hacky and like it would be easy to get out of control.

Completely agree, I had the same feeling. I'll do a bit more searching for a cleaner, more "Django" way to do this. I think it's worth the effort it since it does lower the duplicated queries by a good bit especially on the /eligibility part.

thekaveman · 2026-07-14T15:00:01Z

-        return models.EnrollmentFlow.by_id(request.session[_FLOW])
-    except (KeyError, models.EnrollmentFlow.DoesNotExist):
+        flow = models.EnrollmentFlow.by_id(flow_id)
+        request._cached_flow = flow


Same question as above

Scotchester

A few comments on DDT loading before I dig into the meat of the PR.

Scotchester · 2026-07-14T21:28:53Z

+if DEBUG_TOOLBAR:
+    INSTALLED_APPS.append("debug_toolbar")
+    MIDDLEWARE.insert(0, "debug_toolbar.middleware.DebugToolbarMiddleware")
+    INTERNAL_IPS = ["127.0.0.1"]


I believe this has no effect when overriding SHOW_TOOLBAR_CALLBACK completely. If I comment it out or change the IP, DDT still appears.

Suggested change

INTERNAL_IPS = ["127.0.0.1"]

If we weren't using Docker, I think we could not override SHOW_TOOLBAR_CALLBACK and rely only on INTERNAL_IPS, but Docker makes that complicated. There's a utility to try and help with that, but it's not well documented and apparently doesn't work with Docker Compose.

Scotchester · 2026-07-14T21:29:58Z

+    def show_toolbar(request):
+        # Show the toolbar when in local development mode
+        return DEBUG
+
+    DEBUG_TOOLBAR_CONFIG = {
+        "SHOW_TOOLBAR_CALLBACK": show_toolbar,
+    }


This may not be worth the readability tradeoff, but you could simplify this with a lambda function:

Suggested change

def show_toolbar(request):

# Show the toolbar when in local development mode

return DEBUG

DEBUG_TOOLBAR_CONFIG = {

"SHOW_TOOLBAR_CALLBACK": show_toolbar,

}

DEBUG_TOOLBAR_CONFIG = {

"SHOW_TOOLBAR_CALLBACK": lambda request: DEBUG,

}

Scotchester · 2026-07-14T22:02:32Z

 # SECURITY WARNING: don't run with debug turned on in production!
 DEBUG = os.environ.get("DJANGO_DEBUG", "false").lower() == "true"

+# SECURITY WARNING: don't run with debug_toolbar turned on in production!


For extra caution?

Suggested change

# SECURITY WARNING: don't run with debug_toolbar turned on in production!

# SECURITY WARNING: don't run with debug_toolbar turned on in any non-local environment!

I also wonder if it'd be nice to move this down with the activation code? Everything else up here are core Django settings.

Scotchester · 2026-07-14T22:41:27Z

I agree with the feedback to look for a better caching mechanism, but besides that, the query optimizations are great!

chore: add the django-debug-toolbar dev dependency

19cf724

and configure Django to use it. To activate it, set the environment variable DJANGO_DEBUG_TOOLBAR=true. The toolbar interferes with Pytest, so to run tests in VS Code or from the run.sh script, set DJANGO_DEBUG_TOOLBAR=false.

lalver1 self-assigned this Jul 10, 2026

github-actions Bot added the back-end Django views, sessions, middleware, models, migrations etc. label Jul 10, 2026

thekaveman reviewed Jul 10, 2026

View reviewed changes

Comment thread benefits/settings.py

thekaveman reviewed Jul 10, 2026

View reviewed changes

Comment thread pyproject.toml

lalver1 added 5 commits July 10, 2026 21:39

refactor(session): cache result when calling agency

078b9d4

The result from getting the transit agency from the request's session is cached to avoid querying the database every time session.agency(request) is called during the same request-response cycle.

refactor(session): cache result when calling flow

6d68958

The result from getting the flow from the request's session is cached to avoid querying the database every time session.flow(request) is called during the same request-response cycle.

refactor(TransitAgency): evaluate QuerySet lazily

2cae695

which will avoid making extra queries by removing list, since the conversion to a list is not actually needed.

lalver1 force-pushed the refactor/n-plus-one branch from 02cd7cf to 2cae695 Compare July 13, 2026 17:20

refactor(context_processor): fetch related objects for active_agencies

22deca9

by using select_related() which implements a SQL JOIN in a single database query. Limit the related fields to transit_processor_config which is a ForeignKey and accessed by the _agency_context processor.

lalver1 marked this pull request as ready for review July 14, 2026 14:47

lalver1 requested a review from a team as a code owner July 14, 2026 14:47

thekaveman reviewed Jul 14, 2026

View reviewed changes

Scotchester reviewed Jul 14, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor: N+1 query#3867

Refactor: N+1 query#3867
lalver1 wants to merge 7 commits into
mainfrom
refactor/n-plus-one

lalver1 commented Jul 10, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 10, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

thekaveman Jul 14, 2026

Uh oh!

lalver1 Jul 14, 2026

Uh oh!

thekaveman Jul 14, 2026

Uh oh!

Scotchester left a comment

Uh oh!

Scotchester Jul 14, 2026

Uh oh!

Scotchester Jul 14, 2026

Uh oh!

Scotchester Jul 14, 2026

Uh oh!

Uh oh!

Scotchester commented Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	# SECURITY WARNING: don't run with debug_toolbar turned on in production!
	# SECURITY WARNING: don't run with debug_toolbar turned on in any non-local environment!

Uh oh!

Conversation

lalver1 commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Interpreting SQL results from the DDT

Reviewing

Uh oh!

github-actions Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage report

Uh oh!

Uh oh!

Uh oh!

thekaveman Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

lalver1 Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

thekaveman Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

Scotchester left a comment

Choose a reason for hiding this comment

Uh oh!

Scotchester Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

Scotchester Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

Scotchester Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Scotchester commented Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lalver1 commented Jul 10, 2026 •

edited

Loading

github-actions Bot commented Jul 10, 2026 •

edited

Loading