Make submitting of A/B mailing atomic
The API function that submits A/B mailings performs the following actions:
- submits mailing A (= set scheduled date to now)
- submits mailing B
- distribute recipients between A and B
- set status of the A/B mailing from Draft to Testing, regardless of what happened in previous steps
I can see 3 problems in this code:
- Recipients distribution happens after submitting mailing. So if we are unlucky (i.e. if a mailing job picks up the mailings right at that moment) the mailing sender can start sending the mailings before the distribution has happened
- None of the steps checks whether the earlier steps reported any error
- The change is not atomic: if an error happens, there is no attempt to undo the earlier steps
These problems can have various consequences, from my experience:
- The submitting works but setting the status fails: the A/B mailing is being sent but this has status Draft. Opening it will display a report but it will not be possible to pick the Final
- The submitting fails but setting the status works: the A/B mailing is not sent but has status Testing. Opening it will display the composer but it will not be possible to submit the mailing.
- In any of the previous cases, if the distribution worked fine any attempt to send again the mailing without precaution will cause the distribution to happen again, which means that recipients of mailing A will be reset to whole target group and mailing B and C will receive due percentage of that group, without removing the recipients they already got in previous submit, hence duplicate sendings. That one can be even worse when the error is a deadlock, and the query is retried.
I think that making all 4 steps happen in a single DB transaction would solve all the problems I mention here.
Would that be a good approach? If so, does it make sense to put the whole function in a transaction regardless of the target status, or shall the function be re-organised a bit to have the status setting done in the case statement and make only the
Testing use case atomic?
With those answers, I'm happy to work on a PR.