Moved newsletter campaign sending onto the async message queue - #1233
Merged
Conversation
Campaigns were sent by newsletter_send_all walking 3 queues x 20 subscribers every 5 minutes, capping a blast at 240 recipients per hour per campaign, with no retry when a send failed. A campaign now dispatches a Mage_Newsletter_Model_Queue_SendMessage when it is started, resumed or scheduled (delayed until its start date), and its handler sends one batch and chains the next, so the campaign drains as fast as the worker allows. Batch size is configurable under Newsletter > Sending Options, defaulting to 100 recipients per message. The cron keeps its name and schedule but only queues a batch for due campaigns that have none, as a safety net for chains lost with a worker and for campaigns scheduled before this existed. A per-campaign lock keeps a redelivered or safety-net message from sending a batch twice, and the dedupe key keeps repeat dispatches from piling up. Both the campaign batches and the newsletters themselves go to a 'newsletter' queue, so a blast can be given a worker of its own instead of delaying transactional mail; Mage_Core_Model_Email_Template gained a queue name for that. Campaigns also flip to Sending when their first batch leaves rather than staying Not Sent until the end.
A campaign scheduled for a future date was dispatched at save time as a delayed message, which parked a pending row in the queue for as long as the wait lasted and made a reschedule depend on a dedupe key carrying the start date. scheduleSending() now dispatches only once the start date has come; the newsletter_send_all cron picks up the rest when their time arrives, and the dedupe key is back to the campaign id. The recipient snapshot moves with it: setStores() no longer materializes the audience on every save of a draft, the first batch does it instead, so the recipients are the ones subscribed when the campaign goes out. Linking them is now a chunked bulk insert rather than a row-per-insert loop, and the grid reports the projected audience for a campaign that has not started. Also fixed along the way: email:queue:process and email:config:show only looked at the email queue, so newsletter mail was never drained on hosts without a resident worker, and the batch handler decided whether to chain the next batch from a stale copy of the campaign status.
Deferring the recipient snapshot to the first batch left three ways for a campaign to lose its audience: - setStores() still deleted every unsent queue_link but no longer re-added them, so saving a paused campaign stranded whoever had not been mailed yet. It now refuses outright once the campaign is past STATUS_NEVER, which also stops a save from wiping the queue_store_link rows: the form disables those inputs, so a browser posts nothing and the empty set was being applied. - materializeRecipients() was gated on STATUS_NEVER, but start and resume reach STATUS_SENDING before any batch runs, so the first batch found no links and finished the campaign with nobody mailed. It self-guards on an empty store list and on links already existing, so the call is unconditional. - addSubscribersToQueue() rolled back and swallowed the exception, which now means a silently empty audience instead of a save-time error. It rethrows so the queue message retries with backoff. The admin save applies start_at, stores, customer_segments, subject, sender, text and styles only while the campaign is in STATUS_NEVER, and refuses a request that carries any of them afterwards; only resuming stays available. removeSubscribersFromQueue() committed and rolled back a transaction it never began, and swallowed the exception on top. It is a single delete.
setStores() writes outside $_data, so save() short-circuited on _hasModelChanged() and never reached _afterSave(). getStores() also read an empty $_stores as 'not loaded' and re-fetched stale rows.
- email:queue:clear and email:config:show now cover the newsletter queue as well as the email one, so a failed blast can be cleared and pending newsletter mail is reported - chunk the probe and the row build in addSubscribersToQueue(), not just the insert, so materializing a large audience no longer builds a multi-megabyte statement - a batch continuation carries the campaign dedupe key without enforcing it, keeping the cron sweep from starting a rival chain every run - index newsletter_subscriber on (store_id, subscriber_status) so the queue grid's projected-audience count is answered from the index - the subscribers_total filter compares the same expression the grid renders, instead of counting queue_link rows the grid may not be showing
# Conflicts: # lib/MahoCLI/Commands/EmailConfigShow.php # lib/MahoCLI/Commands/EmailQueueProcess.php # tests/Backend/Integration/Queue/DbTransportTest.php
- Serialized the dedupe check-and-insert on a core lock so concurrent dispatches of one campaign cannot start two chains - Failure-transport sends are never deduped against a live chain - Froze campaign content at the model layer once sending has started - Snapshot the audience with a single INSERT ... SELECT statement - Load the unsent batch first and materialize only when it comes back empty - Finish a campaign whose audience is an exact multiple of the batch size - Grid recipient count falls back to the projected audience for started and paused campaigns - Dropped the redundant newsletter_subscriber store_id index
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1226: the newsletter queue was the last hand-rolled queue in the mail path. Sending was
newsletter_send_allwalking 3 campaigns x 20 subscribers every 5 minutes, so a blast could not exceed 240 recipients/hour per campaign, and a failed send was logged and lost.Starting, resuming or scheduling a campaign now dispatches a
Mage_Newsletter_Model_Queue_SendMessage(delayed untilqueue_start_atwhen that is in the future). The handler sends one batch and chains the next, so a campaign drains as fast as the worker allows and each batch gets the queue's retries, backoff and failure grid. Batch size is configurable under Newsletter > Sending Options (default 100).newsletter_send_allkeeps its name and schedule but only queues a batch for due campaigns, as a safety net for chains lost with a worker and for campaigns scheduled before this existed;newsletter_queue/sendingdoes the same for hosts driving it by URL. Delivery is at-least-once, so a per-campaign lock keeps a redelivered or safety-net message from sending a batch twice, and the dedupe key keeps repeat dispatches from piling up.Both the campaign batches and the newsletters themselves go to a
newsletterqueue, so a blast can be given a worker of its own (queue:work --queue=newsletter) instead of delaying transactional mail;Mage_Core_Model_Email_Templategained a queue name to allow that.Two behavior changes worth knowing: a campaign flips to Sending when its first batch leaves instead of staying Not Sent until it finishes, and
addOnlyForSendingFilter()now matches a start date of exactly now (<=), agreeing with the handler's own readiness check.No schema change, no data migration: campaigns, recipients and problems keep their tables.
tests/Backend/Integration/Newsletter/QueueSendTest.phpcovers dispatch and dedupe, the delayed start, a batch marking its recipients and chaining the next, finishing on the last batch, paused campaigns, the lock, and the cron picking due campaigns only.Note
Developed with the help of AI.
As part of our commitment to GenAI transparency, we flag pull requests produced with AI assistance alongside human work. As with every change in Maho, a maintainer reviews and validates it before merge, we never merge purely AI-generated changes. See the GenAI transparency section for details.