Частичная работа выполняется дважды (ThreadPool.QueueUserWorkItem) - PullRequest
8 голосов
/ 09 марта 2012

Я создал систему рассылки, которая позволяет мне указывать, какие участники должны получать рассылку.Затем я перебираю список участников, которые соответствуют критериям, и для каждого участника я генерирую персонализированное сообщение и асинхронно отправляю им электронное письмо.

Когда я отправляю электронное письмо, я использую ThreadPool.QueueUserWorkItem.

По некоторым причинам подмножество участников получают электронное письмо дважды.В моей последней партии я отправлял сообщения только 712 участникам, но в итоге было отправлено 798 сообщений.

Я регистрирую отправленные сообщения и смог сказать, что первые 86участники получили сообщение дважды.Вот журнал (в порядке отправки сообщений)

No.  Member   Date
1.   163992   3/8/2012 12:28:13 PM
2.   163993   3/8/2012 12:28:13 PM
...
85.   164469   3/8/2012 12:28:37 PM
86.   163992   3/8/2012 12:28:44 PM
87.   163993   3/8/2012 12:28:44 PM
...
798.   167691   3/8/2012 12:32:36 PM

Каждый участник должен получать рассылку один раз, однако, как вы можете видеть, участник 163992 получает сообщения № 1 и № 86;участник 163993 получил сообщения № 2 и № 87;и так далее.

Еще одна вещь, которую следует отметить, это то, что между отправкой сообщений № 85 и # 86 произошла задержка в 7 секунд.

Я несколько раз просмотрел код и исключил почти весь кодкак причина этого, за исключением, возможно, ThreadPool.QueueUserWorkItem.

Это первый раз, когда я работаю с ThreadPool, поэтому я не настолько знаком с ним.Возможно ли иметь какое-то состояние гонки, которое вызывает такое поведение?

=== --- Пример кода --- ===

    foreach (var recipient in recipientsToEmail)
    {
        _emailSender.SendMemberRegistrationActivationReminder(eventArgs.Newsletter, eventArgs.RecipientNotificationInfo, previewEmail: string.Empty);
    }


    public void SendMemberRegistrationActivationReminder(DomainObjects.Newsletters.Newsletter newsletter, DomainObjects.Members.MemberEmailNotificationInfo recipient, string previewEmail)
    {
//Build message here .....

//Send the message
            this.SendEmailAsync(fromAddress: _settings.WebmasterEmail,
                                toAddress: previewEmail.IsEmailFormat()
                                            ? previewEmail
                                            : recipientNotificationInfo.Email,
                                subject: emailSubject,
                                body: completeMessageBody,
                                memberId: previewEmail.IsEmailFormat()
                                            ? null  //if this is a preview message, do not mark it as being sent to this member
                                            : (int?)recipientNotificationInfo.RecipientMemberPhotoInfo.Id,
                                newsletterId: newsletter.Id,
                                newsletterTypeId: newsletter.NewsletterTypeId,
                                utmCampaign: utmCampaign,
                                languageCode: recipientNotificationInfo.LanguageCode);
        }

    private void SendEmailAsync(string fromAddress, string toAddress, string subject, MultiPartMessageBody body, int? memberId, string utmCampaign, string languageCode, int? newsletterId = null, DomainObjects.Newsletters.NewsletterTypeEnum? newsletterTypeId = null)
    {
        var urlHelper = UrlHelper();
        var viewOnlineUrlFormat = urlHelper.RouteUrl("UtilityEmailRead", new { msgid = "msgid", hash = "hash" });
        ThreadPool.QueueUserWorkItem(state => SendEmail(fromAddress, toAddress, subject, body, memberId, newsletterId, newsletterTypeId, utmCampaign, viewOnlineUrlFormat, languageCode));
    }

Ответы [ 6 ]

3 голосов
/ 16 марта 2012

Вы уверены, что в запросе, который вы используете, чтобы получить список участников для отправки электронного письма, нет дубликатов? Вы присоединяетесь к другому столу? Что вы можете сделать, это:

List<DomainObjects.Members.MemberEmailNotificationInfo> list = GetListFromDatabase();
list = list.Distinct().ToList();
2 голосов
/ 17 марта 2012

Наличие 800+ потоков, работающих на сервере, не является хорошей практикой!Хотя вы используете ThreadPool, потоки ставятся в очередь на сервере и запускаются всякий раз, когда старые потоки возвращаются в пул и освобождают ресурс.Это может занять несколько минут на сервере, и в течение этого времени может произойти множество ситуаций, таких как условия гонки или параллелизм.Вместо этого вы можете поставить в очередь один рабочий элемент из одного защищенного списка:

lock (recipientsToEmail)
{
    ThreadPool.QueueUserWorkItem(t =>
        {
            // enumerate recipientsToEmail and send email
        });
}
1 голос
/ 17 марта 2012

В вашем примере кода мы не видим, где происходит регистрация.

Может быть, метод, который отправляет электронное письмо ошибочно, думал, что тогда что-то случилось, система повторила попытку, что может привести к тому, что электронное письмо было отправлено дважды.

Кроме того, как написано в других ответах и ​​комментариях, я бы еще раз проверил, что я не получаю дублированные записи в списке получателей, и проверил бы это в непараллельном контексте.

1 голос
/ 17 марта 2012

Одной из распространенных причин того, что задачи выполняются дважды в коде, когда вы помещаете задачу в фоновый поток, является ошибочная обработка ошибок.Вы можете перепроверить свой код, чтобы убедиться, что если есть ошибка, вы не всегда повторите попытку, независимо от типа ошибки (некоторые ошибки требуют повторной попытки, другие - нет).

Сказав это, код, который вы разместили, не содержит достаточно информации, чтобы окончательно ответить на ваш вопрос;Есть много возможностей.

FWIW, знаете ли вы, что класс SmtpClient имеет метод SendAsync(), который не требует использования отдельного рабочего потока?

1 голос
/ 16 марта 2012

Если этот код:

foreach (var recipient in recipientsToEmail)
{
    _emailSender.SendMemberRegistrationActivationReminder(eventArgs.Newsletter
    ,eventArgs.RecipientNotificationInfo, previewEmail: string.Empty);
}

соответствует тому, что вы на самом деле делаете ... у вас есть очевидная ошибка. а именно, что вы выполняете foreach, но не используете возвращаемое значение, поэтому вы будете отправлять одно и то же электронное письмо на eventArgs.RecipientNotificationInfo для каждой записи в recipientsToEmail.

1 голос
/ 11 марта 2012

Что нужно проверить (я полагаю, у вас есть способ высмеять отправку писем):

  • Всегда ли количество одинаковых писем одинаково?Что если вы увеличите / уменьшите количество входных значений?Всегда ли одинаковые идентификаторы пользователей дублируются?
  • Делает ли SendEmail() что-то значимое?(Я не вижу ваш код для этого)
  • Есть ли причина, по которой вы не используете SendAsync() метод фреймворка ?
  • Получаете ли вытакое же поведение без многопоточности?

Для чего стоит отправка массовой рассылки с вашего собственного сайта - даже если она полностью легитимна - не всегда стоит хлопот.Службы блокировки спама очень агрессивны, и вы не хотите, чтобы ваш домен попадал в черный список.Сторонние службы устраняют этот риск, предоставляют множество инструментов, а также управляют этой частью процесса для вас.

...