[exim-cvs] Transactions in retry hintsdb

Top Page
Delete this message
Reply to this message
Author: Exim Git Commits Mailing List
Date:  
To: exim-cvs
Subject: [exim-cvs] Transactions in retry hintsdb
Gitweb: https://git.exim.org/exim.git/commitdiff/b5a5e017b07491403c7ae3a4305ecf22b0826aa5
Commit:     b5a5e017b07491403c7ae3a4305ecf22b0826aa5
Parent:     600dc06981df5a906125f8442c36056a117412d4
Author:     Jeremy Harris <jgh146exb@???>
AuthorDate: Thu Jun 27 13:31:11 2024 +0100
Committer:  Jeremy Harris <jgh146exb@???>
CommitDate: Thu Jun 27 13:38:08 2024 +0100


    Transactions in retry hintsdb
---
 doc/doc-txt/ChangeLog |  8 +++----
 src/src/dbfn.c        |  8 +++----
 src/src/dbfunctions.h |  2 +-
 src/src/deliver.c     | 60 +++++++++++++++++++++++++++++++++++----------------
 src/src/globals.c     |  1 +
 src/src/globals.h     |  1 +
 src/src/retry.c       | 10 +++++++--
 src/src/transport.c   |  2 ++
 8 files changed, 62 insertions(+), 30 deletions(-)


diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 833ac7d69..6d50f1bb4 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -11,10 +11,10 @@ JH/01 Use fewer forks & execs for sending many messages to a single host.
       particularly for mailinglist and smarthost cases.


 JH/02 Add transaction support for hintsdbs. The sole initial provider is
-      sqlite, and is used for the wait-trasnprot DB. Transactions imply
-      locking internal to the DB. We no longer need a separate lockfile, can
-      keep the DB handle open for extended periods, and still potentially
-      benefit from concurrency on non-conlicting record uses.
+      sqlite, and is used for the wait-transport and retry DBs. Transactions
+      imply locking internal to the DB. We no longer need a separate lockfile,
+      can keep the DB handle open for extended periods, yet potentially benefit
+      from concurrency on non-conflicting record uses.


Exim version 4.98
-----------------
diff --git a/src/src/dbfn.c b/src/src/dbfn.c
index e12871cdd..d1d8c08d4 100644
--- a/src/src/dbfn.c
+++ b/src/src/dbfn.c
@@ -242,7 +242,7 @@ starting a transaction. "lof" and "panic" always true; read/write mode.
*/

open_db *
-dbfn_open_multi(const uschar * name, open_db * dbblock)
+dbfn_open_multi(const uschar * name, int flags, open_db * dbblock)
{
int rc, save_errno;
flock_t lock_data;
@@ -257,8 +257,8 @@ snprintf(CS dirname, sizeof(dirname), "%s/db", spool_directory);
snprintf(CS filename, sizeof(filename), "%s/%s", dirname, name);

 priv_drop_temp(exim_uid, exim_gid);
-dbblock->dbptr = exim_dbopen_multi(filename, dirname, O_RDWR, EXIMDB_MODE);
-if (!dbblock->dbptr && errno == ENOENT)
+dbblock->dbptr = exim_dbopen_multi(filename, dirname, flags, EXIMDB_MODE);
+if (!dbblock->dbptr && errno == ENOENT && flags == O_RDWR)
   {
   DEBUG(D_hints_lookup)
     debug_printf_indent("%s appears not to exist: trying to create\n", filename);
@@ -305,7 +305,7 @@ void
 dbfn_transaction_commit(open_db * dbp)
 {
 DEBUG(D_hints_lookup) debug_printf_indent("dbfn_transaction_commit\n");
-return exim_dbtransaction_commit(dbp->dbptr);
+exim_dbtransaction_commit(dbp->dbptr);
 }



diff --git a/src/src/dbfunctions.h b/src/src/dbfunctions.h
index cc4c4f655..64d82a024 100644
--- a/src/src/dbfunctions.h
+++ b/src/src/dbfunctions.h
@@ -16,7 +16,7 @@ void     dbfn_close(open_db *);
 void     dbfn_close_multi(open_db *);
 int      dbfn_delete(open_db *, const uschar *);
 open_db *dbfn_open(const uschar *, int, open_db *, BOOL, BOOL);
-open_db *dbfn_open_multi(const uschar *, open_db *);
+open_db *dbfn_open_multi(const uschar *, int, open_db *);
 void    *dbfn_read_with_length(open_db *, const uschar *, int *);
 void    *dbfn_read_enforce_length(open_db *, const uschar *, size_t);
 uschar  *dbfn_scan(open_db *, BOOL, EXIM_CURSOR **);
diff --git a/src/src/deliver.c b/src/src/deliver.c
index 24be14982..4e6624bc7 100644
--- a/src/src/deliver.c
+++ b/src/src/deliver.c
@@ -2717,8 +2717,7 @@ Returns:     Nothing
 static void
 do_local_deliveries(void)
 {
-open_db dbblock;
-open_db *dbm_file = NULL;
+open_db dbblock, * dbm_file = NULL;
 time_t now = time(NULL);


/* Loop until we have exhausted the supply of local deliveries */
@@ -4715,16 +4714,15 @@ Could take the initial continued-tpt hit, and then do the next-id thing?
do_remote_deliveries par_reduce par_wait par_read_pipe
*/

-if (  continue_transport
-   && !continue_wait_db
-   && !exim_lockfile_needed()
-   )
-  {
-  open_db * dbp = store_get(sizeof(open_db), GET_UNTAINTED);
-  continue_wait_db = dbfn_open_multi(
-              string_sprintf("wait-%.200s", continue_transport), dbp);
-  continue_next_id[0] = '\0';
-  }
+if (continue_transport && !exim_lockfile_needed())
+  if (!continue_wait_db)
+    {
+    continue_wait_db = dbfn_open_multi(
+          string_sprintf("wait-%.200s", continue_transport),
+          O_RDWR,
+          (open_db *) store_get(sizeof(open_db), GET_UNTAINTED));
+    continue_next_id[0] = '\0';
+    }


   if ((pid = exim_fork(US"transport")) == 0)
     {
@@ -5133,8 +5131,8 @@ if (  continue_transport
   if (continue_transport)
     {
     par_reduce(0, fallback);
-    if (continue_wait_db && !continue_next_id)
-      { dbfn_close_multi(continue_wait_db); continue_wait_db = NULL; }
+    if (!continue_next_id && continue_wait_db)
+    { dbfn_close_multi(continue_wait_db); continue_wait_db = NULL; }
     }


   /* Otherwise, if we are running in the test harness, wait a bit, to let the
@@ -7310,9 +7308,30 @@ while (addr_new)           /* Loop until all addresses dealt with */


   if (f.queue_2stage)
     dbm_file = NULL;
-  else if (!(dbm_file = dbfn_open(US"retry", O_RDONLY, &dbblock, FALSE, TRUE)))
-    DEBUG(D_deliver|D_retry|D_route|D_hints_lookup)
-      debug_printf("no retry data available\n");
+  else
+    {
+    /* If we have transaction-capable hintsdbs, open the retry db without
+    locking, and leave open for the transport process and for subsequent
+    deliveries. If the open fails, tag that explicitly for the transport but
+    retry the open next time around, in case it was created in the interim. */
+
+    if (continue_retry_db == (open_db *)-1)
+      continue_retry_db = NULL;
+
+    if (continue_retry_db)
+      dbm_file = continue_retry_db;
+    else if (!exim_lockfile_needed() && continue_transport)
+      {
+      dbm_file = dbfn_open_multi(US"retry", O_RDONLY, &dbblock);
+      continue_retry_db = dbm_file ? dbm_file : (open_db *)-1;
+      }
+    else
+      dbm_file = dbfn_open(US"retry", O_RDONLY, &dbblock, FALSE, TRUE);
+
+    if (!dbm_file)
+      DEBUG(D_deliver|D_retry|D_route|D_hints_lookup)
+    debug_printf("no retry data available\n");
+    }


   /* Scan the current batch of new addresses, to handle pipes, files and
   autoreplies, and determine which others are ready for routing. */
@@ -7720,7 +7739,8 @@ while (addr_new)           /* Loop until all addresses dealt with */
   /* The database is closed while routing is actually happening. Requests to
   update it are put on a chain and all processed together at the end. */


-  if (dbm_file) dbfn_close(dbm_file);
+  if (dbm_file && !continue_retry_db)
+    { dbfn_close(dbm_file); dbm_file = NULL; }


   /* If queue_domains is set, we don't even want to try routing addresses in
   those domains. During queue runs, queue_domains is forced to be unset.
@@ -7889,8 +7909,10 @@ while (addr_new)           /* Loop until all addresses dealt with */
       }
     }  /* Continue with routing the next address. */
   }    /* Loop to process any child addresses that the routers created, and
-          any rerouted addresses that got put back on the new chain. */
+       any rerouted addresses that got put back on the new chain. */


+if (dbm_file)        /* Can only be continue_retry_db */
+  { dbfn_close_multi(continue_retry_db); continue_retry_db = dbm_file = NULL; }


/* Debugging: show the results of the routing */

diff --git a/src/src/globals.c b/src/src/globals.c
index 7c7395022..492d631ac 100644
--- a/src/src/globals.c
+++ b/src/src/globals.c
@@ -751,6 +751,7 @@ int     continue_sequence      = 1;
 uschar *continue_transport     = NULL;
 #ifndef COMPILE_UTILITY
 open_db *continue_wait_db      = NULL;
+open_db *continue_retry_db     = NULL;
 #endif
 #ifndef DISABLE_ESMTP_LIMITS
 unsigned continue_limit_mail   = 0;
diff --git a/src/src/globals.h b/src/src/globals.h
index 57b930fd4..1f08d78e9 100644
--- a/src/src/globals.h
+++ b/src/src/globals.h
@@ -455,6 +455,7 @@ extern int     continue_sequence;      /* Sequence num for continued delivery */
 extern uschar *continue_transport;     /* Transport for continued delivery */
 #ifndef COMPILE_UTILITY
 extern open_db *continue_wait_db;      /* Hintsdb for wait-transport */
+extern open_db *continue_retry_db;     /* Hintsdb for retries */
 #endif
 #ifndef DISABLE_ESMTP_LIMITS
 extern unsigned continue_limit_mail;   /* Peer advertised limit */
diff --git a/src/src/retry.c b/src/src/retry.c
index 90319d9d7..fdcb6abea 100644
--- a/src/src/retry.c
+++ b/src/src/retry.c
@@ -192,7 +192,12 @@ if ((node = tree_search(tree_unusable, host_key)))
 /* Open the retry database, giving up if there isn't one. Otherwise, search for
 the retry records, and then close the database again. */


-if (!(dbm_file = dbfn_open(US"retry", O_RDONLY, &dbblock, FALSE, TRUE)))
+if (!continue_retry_db)
+  dbm_file = dbfn_open(US"retry", O_RDONLY, &dbblock, FALSE, TRUE);
+else if ((dbm_file = continue_retry_db) == (open_db *)-1)
+  dbm_file = NULL;
+
+if (!dbm_file)
   {
   DEBUG(D_deliver|D_retry|D_hints_lookup)
     debug_printf("no retry data available\n");
@@ -200,7 +205,8 @@ if (!(dbm_file = dbfn_open(US"retry", O_RDONLY, &dbblock, FALSE, TRUE)))
   }
 host_retry_record = dbfn_read(dbm_file, host_key);
 message_retry_record = dbfn_read(dbm_file, message_key);
-dbfn_close(dbm_file);
+if (!continue_retry_db)
+  dbfn_close(dbm_file);


/* Ignore the data if it is too old - too long since it was written */

diff --git a/src/src/transport.c b/src/src/transport.c
index 6468f9f23..c5565062b 100644
--- a/src/src/transport.c
+++ b/src/src/transport.c
@@ -2083,6 +2083,8 @@ DEBUG(D_transport) debug_printf("transport_pass_socket entered\n");
case (instead, passing back the next-id. But in case it does... */
if (continue_wait_db)
{ dbfn_close_multi(continue_wait_db); continue_wait_db = NULL; }
+if (continue_retry_db)
+ { dbfn_close_multi(continue_retry_db); continue_retry_db = NULL; }

#ifndef DISABLE_ESMTP_LIMITS
continue_limit_mail = peer_limit_mail;

--
## subscription configuration (requires account):
## https://lists.exim.org/mailman3/postorius/lists/exim-cvs.lists.exim.org/
## unsubscribe (doesn't require an account):
## exim-cvs-unsubscribe@???
## Exim details at http://www.exim.org/
## Please use the Wiki with this list - http://wiki.exim.org/