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.

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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.