[exim-cvs] Fix ultimate retry timeouts for intermittently de…

Startseite
Nachricht löschen
Nachricht beantworten
Autor: Exim Git Commits Mailing List
Datum:  
To: exim-cvs
Betreff: [exim-cvs] Fix ultimate retry timeouts for intermittently deliverable recipients.
Gitweb: http://git.exim.org/exim.git/commitdiff/ba9af0af397dd7d395378b883f8d9beb3bdd5ffd
Commit:     ba9af0af397dd7d395378b883f8d9beb3bdd5ffd
Parent:     29343b0827b6da8cd4e86a8afbc54070462b834b
Author:     Tony Finch <dot@???>
AuthorDate: Thu Nov 29 18:39:52 2012 +0000
Committer:  Tony Finch <dot@???>
CommitDate: Thu Nov 29 19:31:53 2012 +0000


    Fix ultimate retry timeouts for intermittently deliverable recipients.


    When a queue runner is handling a message, Exim first routes the
    recipient addresses, during which it prunes them based on the retry
    hints database. After that it attempts to deliver the message to
    any remaining recipients. It then updates the hints database using
    the retry rules.


    So if a recipient address works intermittently, it can get repeatedly
    deferred at routing time. The retry hints record remains fresh so the
    address never reaches the final cutoff time.


    This is a fairly common occurrence when a user is bumping up against
    their storage quota. Exim had some logic in its local delivery code
    to deal with this. However it did not apply to per-recipient defers
    in remote deliveries, e.g. over LMTP to a separate IMAP message store.


    This commit adds a proper retry rule check during routing so that
    the final cutoff time is checked against the message's age. I also
    took the opportunity to unify three very similar blocks of code.


    I suspect this new check makes the old local delivery cutoff check
    redundant, but I have not verified this so I left the code in place.
---
 src/src/deliver.c   |   52 ++++++++-----------------------------
 src/src/functions.h |    2 +
 src/src/retry.c     |   72 ++++++++++++++++++++------------------------------
 3 files changed, 42 insertions(+), 84 deletions(-)


diff --git a/src/src/deliver.c b/src/src/deliver.c
index 34f75fe..c743fee 100644
--- a/src/src/deliver.c
+++ b/src/src/deliver.c
@@ -2383,48 +2383,12 @@ while (addr_local != NULL)
                retry_record->expired;


           /* If we haven't reached the retry time, there is one more check
-          to do, which is for the ultimate address timeout. */
+          to do, which is for the ultimate address timeout. We also do this
+          check during routing so this one might be redundant... */


           if (!ok)
-            {
-            retry_config *retry =
-              retry_find_config(retry_key+2, addr2->domain,
-                retry_record->basic_errno,
-                retry_record->more_errno);
-
-            DEBUG(D_deliver|D_retry)
-              {
-              debug_printf("retry time not reached for %s: "
-                "checking ultimate address timeout\n", addr2->address);
-              debug_printf("  now=%d first_failed=%d next_try=%d expired=%d\n",
-                (int)now, (int)retry_record->first_failed,
-                (int)retry_record->next_try, retry_record->expired);
-              }
-
-            if (retry != NULL && retry->rules != NULL)
-              {
-              retry_rule *last_rule;
-              for (last_rule = retry->rules;
-                   last_rule->next != NULL;
-                   last_rule = last_rule->next);
-              DEBUG(D_deliver|D_retry)
-                debug_printf("  received_time=%d diff=%d timeout=%d\n",
-                  received_time, (int)now - received_time, last_rule->timeout);
-              if (now - received_time > last_rule->timeout) ok = TRUE;
-              }
-            else
-              {
-              DEBUG(D_deliver|D_retry)
-                debug_printf("no retry rule found: assume timed out\n");
-              ok = TRUE;    /* No rule => timed out */
-              }
-
-            DEBUG(D_deliver|D_retry)
-              {
-              if (ok) debug_printf("on queue longer than maximum retry for "
-                "address - allowing delivery\n");
-              }
-            }
+            ok = retry_ultimate_address_timeout(retry_key, addr2->domain,
+                retry_record, now);
           }
         }
       else DEBUG(D_retry) debug_printf("no retry record exists\n");
@@ -5656,7 +5620,10 @@ while (addr_new != NULL)           /* Loop until all addresses dealt with */
     will be far too many attempts for an address that gets a 4xx error. In
     fact, after such an error, we should not get here because, the host should
     not be remembered as one this message needs. However, there was a bug that
-    used to cause this to  happen, so it is best to be on the safe side. */
+    used to cause this to  happen, so it is best to be on the safe side.
+
+    Even if we haven't reached the retry time in the hints, there is one
+    more check to do, which is for the ultimate address timeout. */


     else if (((queue_running && !deliver_force) || continue_hostname != NULL)
             &&
@@ -5666,6 +5633,9 @@ while (addr_new != NULL)           /* Loop until all addresses dealt with */
             ||
             (address_retry_record != NULL &&
               now < address_retry_record->next_try))
+            &&
+            !retry_ultimate_address_timeout(addr->address_retry_key,
+          addr->domain, address_retry_record, now)
             )
       {
       addr->message = US"retry time not reached";
diff --git a/src/src/functions.h b/src/src/functions.h
index 174db07..604dd4a 100644
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -269,6 +269,8 @@ extern void    retry_add_item(address_item *, uschar *, int);
 extern BOOL    retry_check_address(uschar *, host_item *, uschar *, BOOL,
                  uschar **, uschar **);
 extern retry_config *retry_find_config(uschar *, uschar *, int, int);
+extern BOOL    retry_ultimate_address_timeout(uschar *, uschar *,
+                 dbdata_retry *, time_t);
 extern void    retry_update(address_item **, address_item **, address_item **);
 extern uschar *rewrite_address(uschar *, BOOL, BOOL, rewrite_rule *, int);
 extern uschar *rewrite_address_qualify(uschar *, BOOL);
diff --git a/src/src/retry.c b/src/src/retry.c
index e877bf7..4809335 100644
--- a/src/src/retry.c
+++ b/src/src/retry.c
@@ -17,26 +17,34 @@
 *************************************************/


/* This function tests whether a message has been on the queue longer than
-the maximum retry time for a particular host.
+the maximum retry time for a particular host or address.

 Arguments:
-  host_key      the key to look up a host retry rule
+  retry_key     the key to look up a retry rule
   domain        the domain to look up a domain retry rule
-  basic_errno   a specific error number, or zero if none
-  more_errno    additional data for the error
+  retry_record  contains error information for finding rule
   now           the time


 Returns:        TRUE if the ultimate timeout has been reached
 */


-static BOOL
-ultimate_address_timeout(uschar *host_key, uschar *domain, int basic_errno,
-  int more_errno, time_t now)
+BOOL
+retry_ultimate_address_timeout(uschar *retry_key, uschar *domain,
+  dbdata_retry *retry_record, time_t now)
 {
-BOOL address_timeout = TRUE;   /* no rule => timed out */
+BOOL address_timeout;
+
+DEBUG(D_retry)
+  {
+  debug_printf("retry time not reached: checking ultimate address timeout\n");
+  debug_printf("  now=%d first_failed=%d next_try=%d expired=%d\n",
+    (int)now, (int)retry_record->first_failed,
+    (int)retry_record->next_try, retry_record->expired);
+  }


 retry_config *retry =
-  retry_find_config(host_key+2, domain, basic_errno, more_errno);
+  retry_find_config(retry_key+2, domain,
+    retry_record->basic_errno, retry_record->more_errno);


 if (retry != NULL && retry->rules != NULL)
   {
@@ -44,17 +52,23 @@ if (retry != NULL && retry->rules != NULL)
   for (last_rule = retry->rules;
        last_rule->next != NULL;
        last_rule = last_rule->next);
-  DEBUG(D_transport|D_retry)
+  DEBUG(D_retry)
     debug_printf("  received_time=%d diff=%d timeout=%d\n",
       received_time, (int)(now - received_time), last_rule->timeout);
   address_timeout = (now - received_time > last_rule->timeout);
   }
 else
   {
-  DEBUG(D_transport|D_retry)
+  DEBUG(D_retry)
     debug_printf("no retry rule found: assume timed out\n");
+  address_timeout = TRUE;
   }


+DEBUG(D_retry)
+  if (address_timeout)
+    debug_printf("on queue longer than maximum retry for address - "
+      "allowing delivery\n");
+
 return address_timeout;
 }


@@ -206,25 +220,10 @@ if (host_retry_record != NULL)

   if (now < host_retry_record->next_try && !deliver_force)
     {
-    DEBUG(D_transport|D_retry)
-      {
-      debug_printf("host retry time not reached: checking ultimate address "
-        "timeout\n");
-      debug_printf("  now=%d first_failed=%d next_try=%d expired=%d\n",
-        (int)now, (int)host_retry_record->first_failed,
-        (int)host_retry_record->next_try,
-        host_retry_record->expired);
-      }
-
     if (!host_retry_record->expired &&
-        ultimate_address_timeout(host_key, domain,
-          host_retry_record->basic_errno, host_retry_record->more_errno, now))
-      {
-      DEBUG(D_transport|D_retry)
-        debug_printf("on queue longer than maximum retry for "
-          "address - allowing delivery\n");
+        retry_ultimate_address_timeout(host_key, domain,
+          host_retry_record, now))
       return FALSE;
-      }


     /* We have not hit the ultimate address timeout; host is unusable. */


@@ -249,25 +248,12 @@ if (message_retry_record != NULL)
   *retry_message_key = message_key;
   if (now < message_retry_record->next_try && !deliver_force)
     {
-    DEBUG(D_transport|D_retry)
-      {
-      debug_printf("host+message retry time not reached: checking ultimate "
-        "address timeout\n");
-      debug_printf("  now=%d first_failed=%d next_try=%d expired=%d\n",
-        (int)now, (int)message_retry_record->first_failed,
-        (int)message_retry_record->next_try, message_retry_record->expired);
-      }
-    if (!ultimate_address_timeout(host_key, domain, 0, 0, now))
+    if (!retry_ultimate_address_timeout(host_key, domain,
+        message_retry_record, now))
       {
       host->status = hstatus_unusable;
       host->why = hwhy_retry;
       }
-    else
-      {
-      DEBUG(D_transport|D_retry)
-        debug_printf("on queue longer than maximum retry for "
-          "address - allowing delivery\n");
-      }
     return FALSE;
     }
   }