Page MenuHomePhabricator

Queue a worker task to send mail only after committing the mail transaction
ClosedPublic

Authored by epriestley on Jun 14 2017, 7:15 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 4 2024, 3:18 AM
Unknown Object (File)
Feb 3 2024, 4:13 PM
Unknown Object (File)
Jan 24 2024, 4:35 PM
Unknown Object (File)
Jan 19 2024, 9:56 PM
Unknown Object (File)
Jan 19 2024, 9:49 PM
Unknown Object (File)
Dec 24 2023, 2:23 AM
Unknown Object (File)
Dec 22 2023, 4:51 AM
Unknown Object (File)
Dec 21 2023, 7:37 PM
Subscribers
None

Details

Summary

Fixes T12844. This code is misleading: the daemon insert is happening on a different connection, and is not inside the transaction on the Mail connection.

What actually happens is this:

  • (Connection A) BEGIN
  • (Connection A) INSERT INTO mail ...
  • (Connection B) INSERT INTO worker ... <-- This is a different connection, and it is NOT in a transaction!
  • There's a race window here: the worker row is globally visible but the mail row is still isolated inside the transaction.
  • (Connection A) COMMIT
  • Now we're clear: the mail row is globally visible.

Change this code to reflect what's actually happening.

This means that if the worker row insert fails for some reason, we'll now throw with a mail row written to the database. But this is fine: it doesn't send on its own (so it can't cause mail loops or anything) and it can be re-queued with bin/mail resend if necessary without too much trouble.

Test Plan

See T12844 for particulars. Made some comments on tasks, saw the daemons send mail.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable