From f76f3c0605ef8837dbba97014c1381365f3a82ba Mon Sep 17 00:00:00 2001 From: Wilfried Goesgens Date: Thu, 26 Jan 2012 23:11:25 +0100 Subject: [PATCH] SMTP-Client: fix reattempt control - when failing correctly handle the Status-field - fix the handling of the QUEUE-Fields 'submitted' and 'attempted' so we can decide when to resend and when to give up properly. --- citadel/modules/smtp/serv_smtpeventclient.c | 4 ++ citadel/modules/smtp/serv_smtpqueue.c | 68 ++++++--------------- citadel/modules/smtp/smtpqueue.h | 12 +--- 3 files changed, 24 insertions(+), 60 deletions(-) diff --git a/citadel/modules/smtp/serv_smtpeventclient.c b/citadel/modules/smtp/serv_smtpeventclient.c index b38320d2a..518c00a33 100644 --- a/citadel/modules/smtp/serv_smtpeventclient.c +++ b/citadel/modules/smtp/serv_smtpeventclient.c @@ -627,6 +627,7 @@ eNextState SMTP_C_Timeout(AsyncIO *IO) { SmtpOutMsg *pMsg = IO->Data; + pMsg->MyQEntry->Status = 4; EVS_syslog(LOG_DEBUG, "SMTP: %s\n", __FUNCTION__); StrBufPlain(IO->ErrMsg, CKEY(ReadErrors[pMsg->State])); return FailOneAttempt(IO); @@ -635,12 +636,15 @@ eNextState SMTP_C_ConnFail(AsyncIO *IO) { SmtpOutMsg *pMsg = IO->Data; + pMsg->MyQEntry->Status = 4; EVS_syslog(LOG_DEBUG, "SMTP: %s\n", __FUNCTION__); StrBufPlain(IO->ErrMsg, CKEY(ReadErrors[pMsg->State])); return FailOneAttempt(IO); } eNextState SMTP_C_DNSFail(AsyncIO *IO) { + SmtpOutMsg *pMsg = IO->Data; + pMsg->MyQEntry->Status = 4; EVS_syslog(LOG_DEBUG, "SMTP: %s\n", __FUNCTION__); return FailOneAttempt(IO); } diff --git a/citadel/modules/smtp/serv_smtpqueue.c b/citadel/modules/smtp/serv_smtpqueue.c index 3e20779a1..72b73bf1a 100644 --- a/citadel/modules/smtp/serv_smtpqueue.c +++ b/citadel/modules/smtp/serv_smtpqueue.c @@ -94,6 +94,7 @@ pthread_mutex_t ActiveQItemsLock; HashList *ActiveQItems = NULL; HashList *QItemHandlers = NULL; +static const long MaxRetry = SMTP_RETRY_INTERVAL * 2 * 2 * 2 * 2 * 2 * 2 * 2 * 2 * 2 * 2 * 2 * 2 * 2 * 2; int MsgCount = 0; int run_queue_now = 0; /* Set to 1 to ignore SMTP send retry times */ @@ -217,7 +218,7 @@ OneQueItem *DeserializeQueueItem(StrBuf *RawQItem, long QueMsgID) Item = (OneQueItem*)malloc(sizeof(OneQueItem)); memset(Item, 0, sizeof(OneQueItem)); - Item->LastAttempt.retry = SMTP_RETRY_INTERVAL; + Item->Retry = SMTP_RETRY_INTERVAL; Item->MessageID = -1; Item->QueMsgID = QueMsgID; @@ -240,6 +241,9 @@ OneQueItem *DeserializeQueueItem(StrBuf *RawQItem, long QueMsgID) FreeStrBuf(&Line); FreeStrBuf(&Token); + if (Item->Retry >= MaxRetry) + Item->FailNow = 1; + pthread_mutex_lock(&ActiveQItemsLock); if (GetHash(ActiveQItems, LKEY(Item->MessageID), @@ -289,30 +293,24 @@ StrBuf *SerializeQueueItem(OneQueItem *MyQItem) StrBufAppendBuf(QMessage, MyQItem->EnvelopeFrom, 0); } + StrBufAppendBufPlain(QMessage, HKEY("\nretry|"), 0); + StrBufAppendPrintf(QMessage, "%ld", + MyQItem->Retry); + + StrBufAppendBufPlain(QMessage, HKEY("\nattempted|"), 0); + StrBufAppendPrintf(QMessage, "%ld", + MyQItem->ReattemptWhen); + It = GetNewHashPos(MyQItem->MailQEntries, 0); while (GetNextHashPos(MyQItem->MailQEntries, It, &len, &Key, &vQE)) { MailQEntry *ThisItem = vQE; - int i; if (!ThisItem->Active) { /* skip already sent ones from the spoolfile. */ continue; } - - for (i=0; i < ThisItem->nAttempts; i++) { - /* TODO: most probably - * there is just one retry/attempted per message! - */ - StrBufAppendBufPlain(QMessage, HKEY("\nretry|"), 0); - StrBufAppendPrintf(QMessage, "%ld", - ThisItem->Attempts[i].retry); - - StrBufAppendBufPlain(QMessage, HKEY("\nattempted|"), 0); - StrBufAppendPrintf(QMessage, "%ld", - ThisItem->Attempts[i].when); - } StrBufAppendBufPlain(QMessage, HKEY("\nremote|"), 0); StrBufAppendBuf(QMessage, ThisItem->Recipient, 0); StrBufAppendBufPlain(QMessage, HKEY("|"), 0); @@ -378,16 +376,9 @@ void QItem_Handle_Recipient(OneQueItem *Item, StrBuf *Line, const char **Pos) void QItem_Handle_retry(OneQueItem *Item, StrBuf *Line, const char **Pos) { - if (Item->Current == NULL) - NewMailQEntry(Item); - if (Item->Current->Attempts[Item->Current->nAttempts].retry != 0) - Item->Current->nAttempts++; - if (Item->Current->nAttempts > MaxAttempts) { - Item->FailNow = 1; - return; - } - Item->Current->Attempts[Item->Current->nAttempts].retry = + Item->Retry = StrBufExtractNext_int(Line, Pos, '|'); + Item->Retry *= 2; } @@ -399,31 +390,7 @@ void QItem_Handle_Submitted(OneQueItem *Item, StrBuf *Line, const char **Pos) void QItem_Handle_Attempted(OneQueItem *Item, StrBuf *Line, const char **Pos) { - if (Item->Current == NULL) - NewMailQEntry(Item); - if (Item->Current->Attempts[Item->Current->nAttempts].when != 0) - Item->Current->nAttempts++; - if (Item->Current->nAttempts > MaxAttempts) { - Item->FailNow = 1; - return; - } - - Item->Current->Attempts[Item->Current->nAttempts].when = - StrBufExtractNext_int(Line, Pos, '|'); - if (Item->Current->Attempts[Item->Current->nAttempts].when > - Item->LastAttempt.when) - { - Item->LastAttempt.when = - Item->Current->Attempts[Item->Current->nAttempts].when; - - Item->LastAttempt.retry = - Item->Current->Attempts[ - Item->Current->nAttempts - ].retry * 2; - - if (Item->LastAttempt.retry > SMTP_RETRY_MAX) - Item->LastAttempt.retry = SMTP_RETRY_MAX; - } + Item->ReattemptWhen = StrBufExtractNext_int(Line, Pos, '|'); } @@ -694,8 +661,7 @@ void smtp_do_procmsg(long msgnum, void *userdata) { /* * Postpone delivery if we've already tried recently. */ - if (((time(NULL) - MyQItem->LastAttempt.when) < - MyQItem->LastAttempt.retry) && + if (((time(NULL) - MyQItem->ReattemptWhen) > 0) && (run_queue_now == 0)) { syslog(LOG_DEBUG, "SMTP client: Retry time not yet reached.\n"); diff --git a/citadel/modules/smtp/smtpqueue.h b/citadel/modules/smtp/smtpqueue.h index 12b2ad387..97b76e790 100644 --- a/citadel/modules/smtp/smtpqueue.h +++ b/citadel/modules/smtp/smtpqueue.h @@ -21,17 +21,9 @@ /* SMTP CLIENT (Queue Management) STUFF */ /*****************************************************************************/ - - #define MaxAttempts 15 -typedef struct _delivery_attempt { - time_t when; - time_t retry; -}DeliveryAttempt; typedef struct _mailq_entry { - DeliveryAttempt Attempts[MaxAttempts]; - int nAttempts; StrBuf *Recipient; StrBuf *StatusMessage; int Status; @@ -58,7 +50,9 @@ typedef struct queueitem { * if null add a new one. */ MailQEntry *Current; - DeliveryAttempt LastAttempt; + time_t ReattemptWhen; + time_t Retry; + long ActiveDeliveries; StrBuf *EnvelopeFrom; StrBuf *BounceTo; -- 2.30.2