Documents: index the file columns and the check date that file deletes and the scheduled scrub look up #1168

New issue

Open

opened 2026-10-31 03:04:28 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-31 03:04:28 +00:00

Owner

This issue holds 2 findings of the audit that share one fix or the same files.

Q6: Documents: deleting a file checks whether another row uses it with full scans of unindexed file columns

What exists today

src/postulo/documents/signals.py:68-79 remove_the_file_from_disk(): after commit, any(model.objects.filter(file=name).exists() for model in (RenderedDocument, UploadedDocument)).
src/postulo/jobs/signals.py:72-75: the same for CapturedPage.source and CapturedPage.rendering.

None of these columns is indexed (documents/models.py:679 and :990, jobs/models.py:2125 and the rendering field, no db_index and no Meta.indexes on them). EXPLAIN QUERY PLAN SELECT 1 FROM documents_uploadeddocument WHERE file = 'x' LIMIT 1 under the test settings: SCAN documents_uploadeddocument (probe audit/m_scripts/test_probe_queries.py).

It runs only on delete. For a single delete it is a scan of one person's-sized table, well under a millisecond. In an account erasure or a bulk delete of N documents it is 2N scans of tables of up to N rows.

What would change

db_index=True on UploadedDocument.file, RenderedDocument.file, CapturedPage.source and CapturedPage.rendering, in one migration per app. The checks become index lookups. Nothing else changes.

What it saves or protects

small: one full table scan per check, two to four per deleted document; quadratic only in an account erasure with many documents

Accessibility

none

Decisions

One issue with Q7? Recommended: yes, "Documents: index the columns the file checks and the scrub look up", one migration per app; Q6 and Q7 are both small and touch the same models.

Q7: Documents: the scheduled scrub sorts every upload to pick the 25 least recently checked

What exists today

src/postulo/documents/scrub.py:55 verify_uploads() builds UploadedDocument.objects.exclude(file="").order_by("verified_at", "pk") and takes [:limit]. notifications/management/commands/send_due_reminders.py:230 calls it with SCRUB_SLICE = 25 (:32) on every scheduler pass. verified_at (documents/models.py:716) has no index. EXPLAIN QUERY PLAN of that query under the test settings: SCAN documents_uploadeddocument then USE TEMP B-TREE FOR ORDER BY (probe audit/m_scripts/test_probe_queries.py).

The table holds every upload of every account, so the scan and sort grow with the instance while the work done stays at 25 rows.

What would change

Meta.indexes = [models.Index(fields=("verified_at", "id"))] on UploadedDocument (the order the query asks for, so both databases walk the index and stop at 25). Each scrub then updates 25 index entries as it writes verified_at, which is negligible. NULL verified_at (never checked) sorts first on SQLite and last on PostgreSQL by default; that is today's behaviour too and the index does not change it, but the issue should say whether "never checked first" is meant on PostgreSQL (F("verified_at").asc(nulls_first=True) with a matching index if so).

What it saves or protects

small: a full scan and a temporary sort of the uploads table at every scheduler pass, replaced by an index walk of 25 rows

Accessibility

none

Decisions

One issue with Q6? Recommended: yes, one migration per app for both.
Never-checked uploads first on PostgreSQL too? Recommended: yes, nulls_first=True in the query and the index, so a new upload is checked at the next pass on either database.

Found by the efficiency and reliability audit of 2026-10-30 (finding Q6, Q7; index postulo/postulo#1108). Checked against main at daee30b15 (2026-10-30).

This issue holds 2 findings of the audit that share one fix or the same files. ## Q6: Documents: deleting a file checks whether another row uses it with full scans of unindexed file columns ### What exists today - `src/postulo/documents/signals.py:68-79` `remove_the_file_from_disk()`: after commit, `any(model.objects.filter(file=name).exists() for model in (RenderedDocument, UploadedDocument))`. - `src/postulo/jobs/signals.py:72-75`: the same for `CapturedPage.source` and `CapturedPage.rendering`. None of these columns is indexed (`documents/models.py:679` and `:990`, `jobs/models.py:2125` and the `rendering` field, no `db_index` and no `Meta.indexes` on them). `EXPLAIN QUERY PLAN SELECT 1 FROM documents_uploadeddocument WHERE file = 'x' LIMIT 1` under the test settings: `SCAN documents_uploadeddocument` (probe `audit/m_scripts/test_probe_queries.py`). It runs only on delete. For a single delete it is a scan of one person's-sized table, well under a millisecond. In an account erasure or a bulk delete of N documents it is 2N scans of tables of up to N rows. ### What would change `db_index=True` on `UploadedDocument.file`, `RenderedDocument.file`, `CapturedPage.source` and `CapturedPage.rendering`, in one migration per app. The checks become index lookups. Nothing else changes. ### What it saves or protects small: one full table scan per check, two to four per deleted document; quadratic only in an account erasure with many documents ### Accessibility none ### Decisions - **One issue with Q7?** Recommended: yes, "Documents: index the columns the file checks and the scrub look up", one migration per app; Q6 and Q7 are both small and touch the same models. ## Q7: Documents: the scheduled scrub sorts every upload to pick the 25 least recently checked ### What exists today `src/postulo/documents/scrub.py:55` `verify_uploads()` builds `UploadedDocument.objects.exclude(file="").order_by("verified_at", "pk")` and takes `[:limit]`. `notifications/management/commands/send_due_reminders.py:230` calls it with `SCRUB_SLICE = 25` (`:32`) on every scheduler pass. `verified_at` (`documents/models.py:716`) has no index. `EXPLAIN QUERY PLAN` of that query under the test settings: `SCAN documents_uploadeddocument` then `USE TEMP B-TREE FOR ORDER BY` (probe `audit/m_scripts/test_probe_queries.py`). The table holds every upload of every account, so the scan and sort grow with the instance while the work done stays at 25 rows. ### What would change `Meta.indexes = [models.Index(fields=("verified_at", "id"))]` on `UploadedDocument` (the order the query asks for, so both databases walk the index and stop at 25). Each scrub then updates 25 index entries as it writes `verified_at`, which is negligible. NULL `verified_at` (never checked) sorts first on SQLite and last on PostgreSQL by default; that is today's behaviour too and the index does not change it, but the issue should say whether "never checked first" is meant on PostgreSQL (`F("verified_at").asc(nulls_first=True)` with a matching index if so). ### What it saves or protects small: a full scan and a temporary sort of the uploads table at every scheduler pass, replaced by an index walk of 25 rows ### Accessibility none ### Decisions - **One issue with Q6?** Recommended: yes, one migration per app for both. - **Never-checked uploads first on PostgreSQL too?** Recommended: yes, `nulls_first=True` in the query and the index, so a new upload is checked at the next pass on either database. --- Found by the efficiency and reliability audit of 2026-10-30 (finding Q6, Q7; index postulo/postulo#1108). Checked against `main` at `daee30b15` (2026-10-30).

tiagoagueda added this to the 1.0.0 milestone

2026-10-31 03:04:28 +00:00

tiagoagueda added the

performance

enhancement

labels

2026-10-31 03:04:28 +00:00

tiagoagueda referenced this issue

2026-10-31 03:04:41 +00:00

Efficiency and reliability audit of 2026-10-30: index #1108