Notifications: without a worker, status, interview and offer messages are sent inside the caller's transaction #578

New issue

Closed

opened 2026-10-01 10:47:47 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

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

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, following slow.tell from the application services to the notifiers.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at core/csv_import.py:941 (the import's transaction.atomic()); the other locations have not moved.

slow.tell hands the message to errands.send, which, on an instance without a worker (POSTULO_BACKGROUND_WORK is off by default), performs the errand on the spot. Every caller of tell is inside a transaction, so each notifier's network call (SMTP, Apprise, Web Push) runs while the transaction is open and before it commits. #220 and #247 took slow work out of request transactions; this path puts it back.

Where: notifications/slow.py:139-149 (tell); core/errands.py:97-101 (send, which calls perform inline when worker_expected() is false); applications/services.py:87-88, 149 and 194 (change_status: @transaction.atomic, a select_for_update on the application, then slow.tell); applications/services.py:425, 461, 696, 713 (schedule_interview, reschedule_interview, record_offer, revise_offer, all atomic); core/csv_import.py:946 (the whole import in one transaction.atomic(), calling change_status per row), reached from core/views_import.py, which also runs under ATOMIC_REQUESTS.
What happens: a person switches An application's status changes on for their email or Apprise notifier (off by default for those; on for webhooks, whose send only writes a row).
1. Every board move holds the SQLite write lock (on PostgreSQL, the application's row lock) until the notifier has answered or timed out.
2. Importing a spreadsheet makes one or two inline notifier sends per row inside a single transaction; with SMTP at a second a message, two hundred rows hold the write lock for minutes, and every other worker's write waits past the twenty-second busy timeout and fails with database is locked.
3. If anything after change_status raises (a later row of the import, the rest of the view), the transaction rolls back, but the messages about the changes that never happened have already been sent.
Why it matters: a notification preference turns ordinary actions into long write locks for the whole instance, and people can be told about status changes, interviews and offers that were never saved.
Fix: in tell, queue after commit: transaction.on_commit(lambda: errands.send("notify", owner, ...)), with the payload built before the lambda. The inline errand then runs outside the transaction and only for committed changes, and a queued one is guaranteed to find its rows. on_commit runs immediately when no transaction is open, so callers outside one are unaffected.

Related to #220 and #247, which moved slow work out of request transactions but did not cover this path.

Done when

A test in tests/test_webhooks.py (or a new tests/test_notify_after_commit.py) with a recording notifier that has status_changed on: change_status inside transaction.atomic() that then raises sends nothing, and one that commits sends once, with the send observed only after commit (django_capture_on_commit_callbacks(execute=True)).

Found on 2026-09-29 in a read-only review of postulo at 95348908d, following `slow.tell` from the application services to the notifiers. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `core/csv_import.py:941` (the import's `transaction.atomic()`); the other locations have not moved. `slow.tell` hands the message to `errands.send`, which, on an instance without a worker (`POSTULO_BACKGROUND_WORK` is off by default), performs the errand on the spot. Every caller of `tell` is inside a transaction, so each notifier's network call (SMTP, Apprise, Web Push) runs while the transaction is open and before it commits. #220 and #247 took slow work out of request transactions; this path puts it back. - **Where:** `notifications/slow.py:139-149` (`tell`); `core/errands.py:97-101` (`send`, which calls `perform` inline when `worker_expected()` is false); `applications/services.py:87-88`, `149` and `194` (`change_status`: `@transaction.atomic`, a `select_for_update` on the application, then `slow.tell`); `applications/services.py:425`, `461`, `696`, `713` (`schedule_interview`, `reschedule_interview`, `record_offer`, `revise_offer`, all atomic); `core/csv_import.py:946` (the whole import in one `transaction.atomic()`, calling `change_status` per row), reached from `core/views_import.py`, which also runs under `ATOMIC_REQUESTS`. - **What happens:** a person switches *An application's status changes* on for their email or Apprise notifier (off by default for those; on for webhooks, whose `send` only writes a row). 1. Every board move holds the SQLite write lock (on PostgreSQL, the application's row lock) until the notifier has answered or timed out. 2. Importing a spreadsheet makes one or two inline notifier sends per row inside a single transaction; with SMTP at a second a message, two hundred rows hold the write lock for minutes, and every other worker's write waits past the twenty-second busy timeout and fails with *database is locked*. 3. If anything after `change_status` raises (a later row of the import, the rest of the view), the transaction rolls back, but the messages about the changes that never happened have already been sent. - **Why it matters:** a notification preference turns ordinary actions into long write locks for the whole instance, and people can be told about status changes, interviews and offers that were never saved. - **Fix:** in `tell`, queue after commit: `transaction.on_commit(lambda: errands.send("notify", owner, ...))`, with the payload built before the lambda. The inline errand then runs outside the transaction and only for committed changes, and a queued one is guaranteed to find its rows. `on_commit` runs immediately when no transaction is open, so callers outside one are unaffected. Related to #220 and #247, which moved slow work out of request transactions but did not cover this path. ## Done when - A test in `tests/test_webhooks.py` (or a new `tests/test_notify_after_commit.py`) with a recording notifier that has `status_changed` on: `change_status` inside `transaction.atomic()` that then raises sends nothing, and one that commits sends once, with the send observed only after commit (`django_capture_on_commit_callbacks(execute=True)`).

tiagoagueda added this to the 0.5.0 milestone

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

tiagoagueda added the

bug

label

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

tiagoagueda referenced this issue from a commit

2026-10-04 10:04:59 +00:00

Announce status, interview and offer changes after the commit

tiagoagueda closed this issue

2026-10-04 10:04:59 +00:00

tiagoagueda referenced this issue from a commit