From bfbee055ea3f646132c0abada23e03ed1296f100 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 20 Nov 2025 12:07:16 +1030 Subject: [PATCH] db: don't start transactions unless we really need to. We always start a transaction before processing, but there are cases where we don't need to. Switch to doing it on-demand. This doesn't make a big difference for sqlite3, but it can for Postgres because of the latency: 12% or so. Every bit helps! 30 runs, min-max(mean+/-stddev): Postgres before: 8.842773-9.769030(9.19531+/-0.21) Postgres after: 8.007967-8.321856(8.14172+/-0.066) sqlite3 before: 7.486042-8.371831(8.15544+/-0.19) sqlite3 after: 7.973411-8.576135(8.3025+/-0.12) Signed-off-by: Rusty Russell --- db/common.h | 2 ++ db/db_sqlite3.c | 4 ++++ db/exec.c | 24 +++++++++++++++++++++--- db/exec.h | 5 +++++ db/utils.c | 8 +++++++- 5 files changed, 39 insertions(+), 4 deletions(-) diff --git a/db/common.h b/db/common.h index 829a3d2cf..2533d7be1 100644 --- a/db/common.h +++ b/db/common.h @@ -29,6 +29,8 @@ struct db { char *filename; const char *in_transaction; + /* For lazy transaction activation */ + bool transaction_started; /* DB-specific context */ void *conn; diff --git a/db/db_sqlite3.c b/db/db_sqlite3.c index 5df248b6f..ed63989d4 100644 --- a/db/db_sqlite3.c +++ b/db/db_sqlite3.c @@ -474,12 +474,14 @@ static char **prepare_table_manip(const tal_t *ctx, /* But core insists we're "in a transaction" for all ops, so fake it */ db->in_transaction = "Not really"; + db->transaction_started = true; /* Turn off foreign keys first. */ db_prepare_for_changes(db); db_exec_prepared_v2(take(db_prepare_untranslated(db, "PRAGMA foreign_keys = OFF;"))); db_report_changes(db, NULL, 0); db->in_transaction = NULL; + db->transaction_started = false; db_begin_transaction(db); cmd = tal_fmt(tmpctx, "ALTER TABLE %s RENAME TO temp_%s;", @@ -525,11 +527,13 @@ static bool complete_table_manip(struct db *db, /* Allow links between them (esp. cascade deletes!) */ db->in_transaction = "Not really"; + db->transaction_started = true; db_prepare_for_changes(db); db_exec_prepared_v2(take(db_prepare_untranslated(db, "PRAGMA foreign_keys = ON;"))); db_report_changes(db, NULL, 0); db->in_transaction = NULL; + db->transaction_started = false; /* migrations are performed inside transactions, so start one. */ db_begin_transaction(db); diff --git a/db/exec.c b/db/exec.c index 5ab6e9b04..382ad9f15 100644 --- a/db/exec.c +++ b/db/exec.c @@ -121,19 +121,29 @@ static void db_data_version_incr(struct db *db) void db_begin_transaction_(struct db *db, const char *location) { - bool ok; if (db->in_transaction) db_fatal(db, "Already in transaction from %s", db->in_transaction); + db->in_transaction = location; /* No writes yet. */ db->dirty = false; +} + +void db_need_transaction(struct db *db, const char *location) +{ + bool ok; + + if (!db->in_transaction) + db_fatal(db, "Not in a transaction for %s", location); + + if (db->transaction_started) + return; db_prepare_for_changes(db); ok = db->config->begin_tx_fn(db); if (!ok) db_fatal(db, "Failed to start DB transaction: %s", db->error); - - db->in_transaction = location; + db->transaction_started = true; } bool db_in_transaction(struct db *db) @@ -150,6 +160,13 @@ void db_commit_transaction(struct db *db) { bool ok; assert(db->in_transaction); + + if (!db->transaction_started) { + db->in_transaction = NULL; + assert(!db->dirty); + return; + } + db_assert_no_outstanding_statements(db); /* Increment before reporting changes to an eventual plugin. */ @@ -164,4 +181,5 @@ void db_commit_transaction(struct db *db) db->in_transaction = NULL; db->dirty = false; + db->transaction_started = false; } diff --git a/db/exec.h b/db/exec.h index d923ae0f6..c852d9501 100644 --- a/db/exec.h +++ b/db/exec.h @@ -40,6 +40,11 @@ void db_begin_transaction_(struct db *db, const char *location); bool db_in_transaction(struct db *db); +/** + * db_need_transaction: we now need to actually enable the transaction, if not + * already. */ +void db_need_transaction(struct db *db, const char *location); + /** * db_commit_transaction - Commit a running transaction * diff --git a/db/utils.c b/db/utils.c index 25bce2f4f..8c4781890 100644 --- a/db/utils.c +++ b/db/utils.c @@ -3,6 +3,7 @@ #include #include #include +#include #include /* Matches the hash function used in devtools/sql-rewrite.py */ @@ -143,6 +144,7 @@ bool db_query_prepared_canfail(struct db_stmt *stmt) assert(stmt->query->readonly); trace_span_start("db_query_prepared", stmt); trace_span_tag(stmt, "query", stmt->query->query); + db_need_transaction(stmt->db, stmt->query->query); ret = stmt->db->config->query_fn(stmt); stmt->executed = true; list_del_from(&stmt->db->pending_statements, &stmt->list); @@ -174,9 +176,12 @@ bool db_step(struct db_stmt *stmt) void db_exec_prepared_v2(struct db_stmt *stmt TAKES) { + bool ret; + + db_need_transaction(stmt->db, stmt->query->query); trace_span_start("db_exec_prepared", stmt); trace_span_tag(stmt, "query", stmt->query->query); - bool ret = stmt->db->config->exec_fn(stmt); + ret = stmt->db->config->exec_fn(stmt); trace_span_end(stmt); if (stmt->db->readonly) @@ -358,6 +363,7 @@ struct db *db_open_(const tal_t *ctx, const char *filename, db_fatal(db, "Unable to find DB queries for %s", db->config->name); db->in_transaction = NULL; + db->transaction_started = false; db->changes = NULL; /* This must be outside a transaction, so catch it */