From: Jeremy Harris <jgh146exb@wizmail.org>
Date: Thu, 27 Jun 2024 12:31:11 +0000 (+0100)
Subject: Transactions in retry hintsdb
X-Git-Url: https://git.exim.org/exim.git/commitdiff_plain/b5a5e017b07491403c7ae3a4305ecf22b0826aa5

Transactions in retry hintsdb
---

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;