Spreadsheet import: every row runs in one request and one transaction, so a sheet within the 5,000-row cap cannot finish #555

New issue

Closed

opened 2026-10-01 10:47:36 +00:00 by tiagoagueda · 1 comment

tiagoagueda commented

2026-10-01 10:47:36 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, by reading the import path and timing csv_import.perform against the snapshot.

Related to #220.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at core/csv_import.py:37-38 (MAX_BYTES, MAX_ROWS) and core/csv_import.py:923-965 (perform, with transaction.atomic() at :941); the other locations have not moved.

The spreadsheet import accepts up to 2 MB and MAX_ROWS = 5000, but it imports every row inside the request, inside one transaction.atomic(), at 18 to 26 queries a row, with a cost per row that grows with what is already there. A sheet well inside the cap takes longer than gunicorn's 120-second worker timeout: the worker is killed, the transaction rolls back, nothing is imported, and on SQLite the instance's write lock is held the whole time.

Where: core/views_import.py:17-63 import_csv (no non_atomic_requests), core/csv_import.py:33-34 (MAX_BYTES, MAX_ROWS), core/csv_import.py:928-970 perform (with transaction.atomic(): at :946), notifications/slow.py:120-148 (anybody_wants queries the notifier connections on every status change), docker/Dockerfile:354 (--timeout 120).
What happens: for each row perform asks whether a listing has the same url, whether a company has the name case-insensitively, whether a Wikidata identifier matches, gets or creates the company, looks for a twin application on applied_at__date, creates the posting and the application, and runs change_status, which records events and asks slow.tell → anybody_wants, a Connection query, even for a person with no notifier. None of the lookups is resolved once per sheet. Measured on the snapshot: 200 rows with URLs in 3.7 s and 5,144 queries, the next 200 in 4.8 s; the other reviewer measured 1,500 rows at 52.8 s (35 ms a row, against 28 ms at 500). At that rate 5,000 rows is well past 120 s on a desktop, and a small machine reaches the limit far sooner. Meanwhile, because the view is inside ATOMIC_REQUESTS and SQLite opens it IMMEDIATE, every other request of the instance waits for the lock and fails with database is locked after 20 s.
Why it matters: a person importing a year or two of past applications, the case the import exists for, gets a proxy error and an empty result, and pressing Import again repeats all of it. Other people on the instance get 500s while it runs.
Fix: run the import as an errand, as the export and the PDFs are, from a non_atomic_requests view that answers with the page that watches it. Inside perform: resolve each distinct company name once into a dict; fetch the existing posting URLs and the (company, title, applied date) twins in one query each before the loop; ask anybody_wants once per import (or do not announce imported history at all); and commit in chunks (for example 200 rows) so the lock is released between them. If the cap stays at 5,000 rows, test that size; otherwise lower MAX_ROWS to what one run can do.

Done when

A test in tests/test_csv_import.py imports a generated sheet of MAX_ROWS rows inside django_assert_max_num_queries with a bound independent of the row count (plus a small constant per row for the inserts), and fails today.
tests/test_slow_requests.py lists the import address in SLOW.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, by reading the import path and timing `csv_import.perform` against the snapshot. Related to #220. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `core/csv_import.py:37-38` (`MAX_BYTES`, `MAX_ROWS`) and `core/csv_import.py:923-965` (`perform`, with `transaction.atomic()` at `:941`); the other locations have not moved. The spreadsheet import accepts up to 2 MB and `MAX_ROWS = 5000`, but it imports every row inside the request, inside one `transaction.atomic()`, at 18 to 26 queries a row, with a cost per row that grows with what is already there. A sheet well inside the cap takes longer than gunicorn's 120-second worker timeout: the worker is killed, the transaction rolls back, nothing is imported, and on SQLite the instance's write lock is held the whole time. - **Where:** `core/views_import.py:17-63` `import_csv` (no `non_atomic_requests`), `core/csv_import.py:33-34` (`MAX_BYTES`, `MAX_ROWS`), `core/csv_import.py:928-970` `perform` (`with transaction.atomic():` at `:946`), `notifications/slow.py:120-148` (`anybody_wants` queries the notifier connections on every status change), `docker/Dockerfile:354` (`--timeout 120`). - **What happens:** for each row `perform` asks whether a listing has the same `url`, whether a company has the name case-insensitively, whether a Wikidata identifier matches, gets or creates the company, looks for a twin application on `applied_at__date`, creates the posting and the application, and runs `change_status`, which records events and asks `slow.tell` → `anybody_wants`, a `Connection` query, even for a person with no notifier. None of the lookups is resolved once per sheet. Measured on the snapshot: 200 rows with URLs in 3.7 s and 5,144 queries, the next 200 in 4.8 s; the other reviewer measured 1,500 rows at 52.8 s (35 ms a row, against 28 ms at 500). At that rate 5,000 rows is well past 120 s on a desktop, and a small machine reaches the limit far sooner. Meanwhile, because the view is inside `ATOMIC_REQUESTS` and SQLite opens it `IMMEDIATE`, every other request of the instance waits for the lock and fails with *database is locked* after 20 s. - **Why it matters:** a person importing a year or two of past applications, the case the import exists for, gets a proxy error and an empty result, and pressing *Import* again repeats all of it. Other people on the instance get 500s while it runs. - **Fix:** run the import as an errand, as the export and the PDFs are, from a `non_atomic_requests` view that answers with the page that watches it. Inside `perform`: resolve each distinct company name once into a dict; fetch the existing posting URLs and the `(company, title, applied date)` twins in one query each before the loop; ask `anybody_wants` once per import (or do not announce imported history at all); and commit in chunks (for example 200 rows) so the lock is released between them. If the cap stays at 5,000 rows, test that size; otherwise lower `MAX_ROWS` to what one run can do. ## Done when - A test in `tests/test_csv_import.py` imports a generated sheet of `MAX_ROWS` rows inside `django_assert_max_num_queries` with a bound independent of the row count (plus a small constant per row for the inserts), and fails today. - `tests/test_slow_requests.py` lists the import address in `SLOW`.

tiagoagueda added this to the 0.6.0 milestone

2026-10-01 10:47:36 +00:00

tiagoagueda added the

enhancement

label

2026-10-01 10:47:36 +00:00

tiagoagueda referenced this issue from a commit

2026-10-06 04:51:05 +00:00

Import a spreadsheet as an errand, in chunks, without a lookup per row

tiagoagueda referenced this issue from a commit

2026-10-06 04:51:05 +00:00

Sweep the import report address for somebody else's errand (#555)