Query performance: N+1 in search, one-join aggregates, closed cards loaded, uncached site settings, missing composite indexes #231

New issue

Closed

opened 2026-09-15 21:23:28 +00:00 by tiagoagueda · 1 comment

tiagoagueda commented

2026-09-15 21:23:28 +00:00

Owner

None of these hurts in the first months; all of them grow with a year or two of data. Each is cheap to fix. Found in the 2026-09-15 code audit (the API pagination problem has its own issue, #230).

Global search. One extra query per matching application (core/search.py:143-147, application.events.filter(...).first()). Every group is loaded and sorted in Python to show five hits (:408,416), and listings load full descriptions (:109-115). Searching "engineer" over 300 applications and 400 listings costs 300 queries and 400 descriptions.
→ Per group, count() plus [:limit] in SQL, ordered with Case(When(title__icontains=q)); the excerpt event through a Prefetch or Subquery.
Companies table. CompanyQuerySet.with_table_data (jobs/models.py:119-124) combines Count("postings"), Count("postings__applications"), Count("contacts") and Max("postings__applications__events__occurred_at") in one GROUP BY. With search joins and .distinct() on top, the paginator's count() runs it all again. One company with 4 postings, 12 events per application and 5 contacts makes 240 intermediate rows.
→ Correlated Subquery counts, as the identifier columns already do, and Exists for search.
Board. applications/views.py:160-165 loads every application, closed ones included, with prefetch and four subqueries, and drops the columns it doesn't show in Python (:202-216). quiet_applications() then recomputes the same subqueries (:204).
→ status__in=BOARD_STATUSES in SQL; work out quiet from the annotations already present.
Site settings, about five queries per page. site.current() runs SiteSettings.objects.filter(pk=1).first() every time (core/site.py:103). It is reached from the middleware (core/middleware.py:52,66), the ui context processor (context_processors.py:62-64) and is_empty(), and htmx fragments pay too because ui is not lazy.
→ Memoise per request, or cache and clear on SiteSettings.save; make context values callables.
Plugin policy. policy.decide reads the plugins JSON record from disk and queries PluginPolicy on every call. CompanyDetailView calls it six times (jobs/views.py:211-234), and Company.descendants runs one query per node.
→ Memoise decide per request; cache read_record() keyed on the file's mtime.
Server → Plugins page. It walks every installed distribution's files on every render: about 4.6 s in a test profile, dominated by importlib.metadata path checks (core/server_views.py:801-824, available_sources(refresh=True)). This page is slow on a Raspberry Pi and costs about 40 s of CI.
→ No refresh=True on GET; provenance only for plugins on the data volume, cached per record mtime.
Missing composite indexes. The hottest subqueries (latest event per application, next reminder, next interview) have only single-column indexes (applications/models.py:143-149,127,181,381).
→ ApplicationEvent("application", "-occurred_at"), Reminder("application", "done_at", "due_at"), Interview("application", "outcome", "starts_at").
Bulk tagging. ApplicationBulkView._tag and CompanyBulkView._industry run exists() and add() per row, about 300 queries for 100 rows.
→ One query for the ids that already have the tag, then through.objects.bulk_create(..., ignore_conflicts=True).
Listings tabs. Six grouped COUNT queries per load (jobs/listing_views.py:59-76).
→ One aggregate() with conditional Count(filter=Q(...)), using Exists for "has applications".
Reports and Insights. reports.build loads every application ever sent and filters the period in Python (reports.py:404-412); _first_reaching loads all status events three times per report. analytics.build runs on every dashboard view with no cache.
→ Filter by the period in SQL, compute Min(occurred_at) per application in SQL, and cache Insights keyed on the newest event id and updated_at.
Status changes can race on PostgreSQL (plausible, not reproduced). change_status reads previous from the caller's object without select_for_update (services.py:89), and ApplicationUpdateView saves every field, status included. Two concurrent changes can write a transition that never happened.
→ Re-read with select_for_update() inside change_status, and exclude status from the update view's update_fields.

Add a django_assert_max_num_queries test for the table, board, search and API list endpoints so none of these comes back.

None of these hurts in the first months; all of them grow with a year or two of data. Each is cheap to fix. Found in the 2026-09-15 code audit (the API pagination problem has its own issue, #230). 1. **Global search.** One extra query per matching application (`core/search.py:143-147`, `application.events.filter(...).first()`). Every group is loaded and sorted in Python to show five hits (`:408,416`), and listings load full descriptions (`:109-115`). Searching "engineer" over 300 applications and 400 listings costs 300 queries and 400 descriptions. → Per group, `count()` plus `[:limit]` in SQL, ordered with `Case(When(title__icontains=q))`; the excerpt event through a `Prefetch` or `Subquery`. 2. **Companies table.** `CompanyQuerySet.with_table_data` (`jobs/models.py:119-124`) combines `Count("postings")`, `Count("postings__applications")`, `Count("contacts")` and `Max("postings__applications__events__occurred_at")` in one GROUP BY. With search joins and `.distinct()` on top, the paginator's `count()` runs it all again. One company with 4 postings, 12 events per application and 5 contacts makes 240 intermediate rows. → Correlated `Subquery` counts, as the identifier columns already do, and `Exists` for search. 3. **Board.** `applications/views.py:160-165` loads every application, closed ones included, with prefetch and four subqueries, and drops the columns it doesn't show in Python (`:202-216`). `quiet_applications()` then recomputes the same subqueries (`:204`). → `status__in=BOARD_STATUSES` in SQL; work out quiet from the annotations already present. 4. **Site settings, about five queries per page.** `site.current()` runs `SiteSettings.objects.filter(pk=1).first()` every time (`core/site.py:103`). It is reached from the middleware (`core/middleware.py:52,66`), the `ui` context processor (`context_processors.py:62-64`) and `is_empty()`, and htmx fragments pay too because `ui` is not lazy. → Memoise per request, or cache and clear on `SiteSettings.save`; make context values callables. 5. **Plugin policy.** `policy.decide` reads the plugins JSON record from disk and queries `PluginPolicy` on every call. `CompanyDetailView` calls it six times (`jobs/views.py:211-234`), and `Company.descendants` runs one query per node. → Memoise `decide` per request; cache `read_record()` keyed on the file's mtime. 6. **Server → Plugins page.** It walks every installed distribution's files on every render: about 4.6 s in a test profile, dominated by `importlib.metadata` path checks (`core/server_views.py:801-824`, `available_sources(refresh=True)`). This page is slow on a Raspberry Pi and costs about 40 s of CI. → No `refresh=True` on GET; provenance only for plugins on the data volume, cached per record mtime. 7. **Missing composite indexes.** The hottest subqueries (latest event per application, next reminder, next interview) have only single-column indexes (`applications/models.py:143-149,127,181,381`). → `ApplicationEvent("application", "-occurred_at")`, `Reminder("application", "done_at", "due_at")`, `Interview("application", "outcome", "starts_at")`. 8. **Bulk tagging.** `ApplicationBulkView._tag` and `CompanyBulkView._industry` run `exists()` and `add()` per row, about 300 queries for 100 rows. → One query for the ids that already have the tag, then `through.objects.bulk_create(..., ignore_conflicts=True)`. 9. **Listings tabs.** Six grouped COUNT queries per load (`jobs/listing_views.py:59-76`). → One `aggregate()` with conditional `Count(filter=Q(...))`, using `Exists` for "has applications". 10. **Reports and Insights.** `reports.build` loads every application ever sent and filters the period in Python (`reports.py:404-412`); `_first_reaching` loads all status events three times per report. `analytics.build` runs on every dashboard view with no cache. → Filter by the period in SQL, compute `Min(occurred_at)` per application in SQL, and cache Insights keyed on the newest event id and `updated_at`. 11. **Status changes can race on PostgreSQL** (plausible, not reproduced). `change_status` reads `previous` from the caller's object without `select_for_update` (`services.py:89`), and `ApplicationUpdateView` saves every field, status included. Two concurrent changes can write a transition that never happened. → Re-read with `select_for_update()` inside `change_status`, and exclude status from the update view's `update_fields`. Add a `django_assert_max_num_queries` test for the table, board, search and API list endpoints so none of these comes back.

tiagoagueda added the

enhancement

label

2026-09-15 21:23:28 +00:00

tiagoagueda referenced this issue

2026-09-15 21:24:48 +00:00

CI and operations tooling: 5,800 tests run serially three times, unpinned tools, releases not gated on CI, no config checks or request ids #233

tiagoagueda added this to the 0.5.0 milestone

2026-09-15 21:33:29 +00:00

tiagoagueda commented

2026-09-17 17:54:01 +00:00

Author

Owner

Two tooling notes for this work: one to find them, one to keep them fixed

Every problem listed above was found by reading code. That worked, and it does not
scale to the next one — nothing in the dev dependency group would have surfaced any of
them, and nothing will notice when one comes back.

Finding them: django-debug-toolbar in the dev group. Version 8.0.0 classifies
Django 6.1, so it is in range for django>=6.1,<6.2. Its SQL panel gives the per-request
query count and the duplicate-query grouping that turns "the companies table feels slow"
into the GROUP BY fan-out described in item 2. Development-only, no production surface.
nplusone is the narrower alternative if a full toolbar is unwanted — it only warns on
N+1 access patterns.

Keeping them fixed: assertNumQueries, which needs no package at all. This is the
more important half. Each fix here should land with a test pinning the query count for
that view:

global search over a fixture of applications and listings — item 1;
CompanyQuerySet.with_table_data through the paginator, which runs count() over the
same joins — item 2;
the board, which should not load closed cards — item 3;
a page render asserting site.current() is one query and not five — item 4.

Without those, every item above is a fix that regresses silently the next time somebody
adds an annotation. With them, the regression fails CI on the commit that causes it, which
is the only point at which it is cheap.

The assertions are worth adding even if the toolbar is not: the toolbar is a
convenience for exploration, the assertions are the guard.

## Two tooling notes for this work: one to find them, one to keep them fixed Every problem listed above was found by **reading code**. That worked, and it does not scale to the next one — nothing in the `dev` dependency group would have surfaced any of them, and nothing will notice when one comes back. **Finding them: `django-debug-toolbar` in the `dev` group.** Version 8.0.0 classifies Django 6.1, so it is in range for `django>=6.1,<6.2`. Its SQL panel gives the per-request query count and the duplicate-query grouping that turns "the companies table feels slow" into the `GROUP BY` fan-out described in item 2. Development-only, no production surface. `nplusone` is the narrower alternative if a full toolbar is unwanted — it only warns on N+1 access patterns. **Keeping them fixed: `assertNumQueries`, which needs no package at all.** This is the more important half. Each fix here should land with a test pinning the query count for that view: - global search over a fixture of applications and listings — item 1; - `CompanyQuerySet.with_table_data` through the paginator, which runs `count()` over the same joins — item 2; - the board, which should not load closed cards — item 3; - a page render asserting `site.current()` is one query and not five — item 4. Without those, every item above is a fix that regresses silently the next time somebody adds an annotation. With them, the regression fails CI on the commit that causes it, which is the only point at which it is cheap. The assertions are worth adding **even if the toolbar is not**: the toolbar is a convenience for exploration, the assertions are the guard.

tiagoagueda referenced this issue from a commit

2026-09-20 10:24:04 +00:00

Say which language a CV or a letter is written in

tiagoagueda referenced this issue from a commit

2026-09-22 11:46:54 +00:00

Stop the daily pages growing with the search

tiagoagueda closed this issue

2026-09-22 11:46:54 +00:00