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
F14075945: D18124.diff
Thu, Nov 21, 1:45 PM
Unknown Object (File)
Wed, Nov 20, 9:33 PM
Unknown Object (File)
Tue, Nov 19, 7:53 AM
Unknown Object (File)
Sun, Nov 17, 2:20 AM
Unknown Object (File)
Fri, Nov 15, 2:28 AM
Unknown Object (File)
Thu, Oct 31, 9:17 PM
Unknown Object (File)
Thu, Oct 24, 10:21 AM
Unknown Object (File)
Oct 22 2024, 10:36 AM
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