spatie / mailcoach-support

Questions and support for Mailcoach
https://mailcoach.app
31 stars 2 forks source link

Campaign sent twice to same subscriber #248

Closed electronick86 closed 3 years ago

electronick86 commented 3 years ago

Hello!

Today was a big "Mailcoach" days with campaigns sent to more than 400k subscribers. A lot of tuning done to improve the queues workers etc but always pleasant to work (and learn) with mailcoach!

I'm facing the following issue: the campaign is sometime sent twice to the same subscribers. This happens on Mailcoach 2.23.13 and also 2.23.9

I have the same issue with 2 Mailcoach instances today. I firstly though to a problem with a custom segment but one of the instance use just mailcoach "default" segments and has the problem.

I realized that the number of "sends" was higher that my email list population.

_MySQL_5_7_26-29-log__BMYSCTRP01_-_mailing7d_mailing7d_webhook_calls

Do you have any Idea of what could cause that problem?

What I can also say is that there is sometimes 2 sends for the same subscriber, but never 3.

 select subscriber_id, count(*)
 from mailcoach_sends
 where campaign_id=273
 group by subscriber_id
 having count(*)>1;

gives 22312 lines but

 select subscriber_id, count(*)
 from mailcoach_sends
 where campaign_id=273
 group by subscriber_id
 having count(*)>2;

don't return anything.

I also see that the two sends for every subscribers have consecutive ids

_MySQL_5_7_26-29-log__BMYSCTRP01_-_mailing7d_mailing7d_webhook_calls

Any idea? Thanks in advance

electronick86 commented 3 years ago

One extra check :


>>> $campaign=App\Campaign::find(273);
>>> $subscribersQuery = $campaign->baseSubscribersQuery();
=> Illuminate\Database\Eloquent\Builder {#3571}
>>> $subscribersQuery->count()
=> 252656
>>> $segment = $campaign->getSegment();
PHP Notice:  unserialize(): Error at offset 0 of 60 bytes in /var/www/html/vendor/spatie/laravel-mailcoach/src/Models/Campaign.php on line 427
=> Spatie\Mailcoach\Support\Segments\SubscribersWithTagsSegment {#3562}
>>> $segment->subscribersQuery($subscribersQuery);
=> null
>>> $subscribersQuery->count()
=> 146609
freekmurze commented 3 years ago

That's kinda strange. There are several protections in mailcoach to prevent sending a mail to the same recipient twice.

We loop over every subscriber in the list you're sending a campaign to

https://github.com/spatie/laravel-mailcoach/blob/master/src/Actions/Campaigns/SendCampaignAction.php#L72-L74

In the createSend method we check that no sent is created if one already exists. https://github.com/spatie/laravel-mailcoach/blob/master/src/Actions/Campaigns/SendCampaignAction.php#L100-L112

(If you can't see these links, you can get access to spatie/laravel-mailcoach on your profile at https://spatie.be)

Were there perhaps two SendCampaign jobs running at the same time?

electronick86 commented 3 years ago

Hello @freekmurze ,

Indeed, I red the code of the SendCampaignAction and I saw theses checks. One more check could be to have an index on the table to make the combination "campaign_id" + "subscriber_id" unique.

Were there perhaps two SendCampaign jobs running at the same time?

I think that you got the problem. Looking n horizon again :

Horizon_-_Failed_Jobs

the details says

Illuminate\Queue\MaxAttemptsExceededException: Spatie\Mailcoach\Jobs\SendCampaignJob has been attempted too many times or run too long. The job may have previously timed out. in /var/www/html/vendor/laravel/framework/src/Illuminate/Queue/Worker.php:648

I'm already happy to find an explanation to this annoying problem! Could I/we do something to avoid this situation?

I think that the situation is caused by an overloaded Database this morning with the high volumes (and the feedback processing taking a lot of DB time #249 )

freekmurze commented 3 years ago

One more check could be to have an index on the table to make the combination "campaign_id" + "subscriber_id" unique.

That's a good suggestion. I think we could also make it so that the SendCampaignJob is re-attempted by specifically setting the $tries to 1. @riasvdv could you add these things in v3?

freekmurze commented 3 years ago

@electronick86 I've created a new release where I've added the $tries = 1 to SendCampaignJob. Run composer update to pull it in. You should create that index you suggested manually in your mailcoach installation.

@riasvdv I've took care of this in v3 now.

electronick86 commented 3 years ago

Thanks @freekmurze .

Maybe my reflexion doesn't make any sense with the new logic in v3 but at I wonder if the SendCampaignAction (https://github.com/spatie/laravel-mailcoach/blob/3b3e1e722c4a6ad0217af65a697ed23b46618fc9/src/Actions/Campaigns/SendCampaignAction.php#L72-L74) would not be faster if all the inserts were done before the sends begins.

At the moment, the inserts continue while the SendMailJob are already dispatched. So the table mailcoach_sends is constantly locked by inserts and updates. Could we create all the sends (and maybe multiple records at once) and after that create the jobs with something similar with the RetryPendingSendsCommand.

My SendCampaignAction takes more than one hour with 160k subscribers. But I think that increasing the timeout for the queue mailcoachwith avoid the problem I got yesterday.

freekmurze commented 3 years ago

This is an awesome suggestion. In Mailcoach v3 we'll do just that. We'll first create all sends and prepare all jobs, before sending out any mails.

@riasvdv is already on it 👍

electronick86 commented 3 years ago

Hello guys,

Just to share you some finding related to the duration of the SendCampaignJob. Even if we found that this problem (unexpected retry of the job) was caused by a wrong queue.connections.mailcoach-redis.retry_after value ( #253) , I changed the folowing mysql setting innodb_flush_log_at_trx_commit=0 . This value make the sendscreation 3 time faster (and impact the complete database).

⚠️ Read the doc before changing this value.

_MySQL_5_7_26-29-log__BMYSCTRP01_-_ctrmail_PROD_ctrmail_mailcoach_campaigns
freekmurze commented 3 years ago

Thanks for letting us know. 👍