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.

diff --git a/citadel/modules/smtp/serv_smtpeventclient.c b/citadel/modules/smtp/serv_smtpeventclient.c
index b38320d2a64b1923c0d4a92103b9cc93913c3178..518c00a335374df7cde766798d7bcf359a1c4c56 100644 (file)
--- 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 3e20779a1476d843b1ed4b408653e8371520f654..72b73bf1a27580410c0739ebcdc3a3d2cde3b8e0 100644 (file)
--- 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 12b2ad387f1591e32f56996ea0411c211107d2f6..97b76e7901518838e9f00b6294a030c940bca048 100644 (file)
--- 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;