From b5a5e017b07491403c7ae3a4305ecf22b0826aa5 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Thu, 27 Jun 2024 13:31:11 +0100 Subject: [PATCH] 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; -- 2.30.2