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;
}
}
