From 77789bb705ae648835da40591d02e0ba318d8c71 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 1 Nov 2017 11:40:23 +1030 Subject: [PATCH] db: Implemented poor mans nested transactions Nesting is provided by only actually performing the outermost transaction and simulating the nested ones. This still allows us to ensure on lower levels that we are in the context of a transaction without having to resort to keeping explicitly track of it in the calling code. Signed-off-by: Christian Decker --- wallet/db.c | 15 +++++++++------ wallet/db.h | 8 +++++--- wallet/wallet.c | 39 +++++++++++++++++++++++---------------- 3 files changed, 37 insertions(+), 25 deletions(-) diff --git a/wallet/db.c b/wallet/db.c index e6fe5db4c..5aec8358d 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -227,12 +227,15 @@ static void close_db(struct db *db) { sqlite3_close(db->sql); } bool db_begin_transaction(struct db *db) { - assert(!db->in_transaction); - /* Clear any errors from previous transactions and - * non-transactional queries */ - db_clear_error(db); - db->in_transaction = db_exec(__func__, db, "BEGIN TRANSACTION;"); - return db->in_transaction; + if (!db->in_transaction) { + /* Clear any errors from previous transactions and + * non-transactional queries */ + db_clear_error(db); + db->in_transaction = db_exec(__func__, db, "BEGIN TRANSACTION;"); + assert(db->in_transaction); + return db->in_transaction; + } + return false; } bool db_commit_transaction(struct db *db) diff --git a/wallet/db.h b/wallet/db.h index 9064bb21d..3c84f8a15 100644 --- a/wallet/db.h +++ b/wallet/db.h @@ -44,9 +44,11 @@ bool PRINTF_FMT(3, 4) /** * db_begin_transaction - Begin a transaction * - * We do not support nesting multiple transactions, so make sure that - * we are not in a transaction when calling this. Returns true if we - * succeeded in starting a transaction. + * Begin a new DB transaction if we aren't already in one. Returns + * true if the call started a transaction, i.e., the caller MUST take + * care to either commit or rollback. If false, this is a nested + * transaction and the caller MUST not commit/rollback, since the + * transaction is handled at a higher level in the callstack. */ bool db_begin_transaction(struct db *db); diff --git a/wallet/wallet.c b/wallet/wallet.c index 2df4edfe6..eaa668204 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -139,6 +139,7 @@ const struct utxo **wallet_select_coins(const tal_t *ctx, struct wallet *w, u64 *fee_estimate, u64 *changesatoshi) { size_t i = 0; + bool should_commit; struct utxo **available; const struct utxo **utxos = tal_arr(ctx, const struct utxo *, 0); *fee_estimate = 0; @@ -147,9 +148,8 @@ const struct utxo **wallet_select_coins(const tal_t *ctx, struct wallet *w, u64 satoshi_in = 0, weight = (4 + (8 + 22) * 2 + 4) * 4; tal_add_destructor2(utxos, destroy_utxos, w); - if (!db_begin_transaction(w->db)) { - fatal("Unable to begin transaction: %s", w->db->err); - } + should_commit = db_begin_transaction(w->db); + available = wallet_get_utxos(ctx, w, output_state_available); for (i = 0; i < tal_count(available); i++) { @@ -177,10 +177,12 @@ const struct utxo **wallet_select_coins(const tal_t *ctx, struct wallet *w, if (satoshi_in < *fee_estimate + value) { /* Could not collect enough inputs, cleanup and bail */ utxos = tal_free(utxos); - db_rollback_transaction(w->db); + if (should_commit) + db_rollback_transaction(w->db); } else { /* Commit the db transaction to persist markings */ - db_commit_transaction(w->db); + if (should_commit) + db_commit_transaction(w->db); *changesatoshi = satoshi_in - value - *fee_estimate; } @@ -274,6 +276,7 @@ bool wallet_shachain_add_hash(struct wallet *wallet, const struct sha256 *hash) { sqlite3_stmt *stmt; + bool should_commit; bool ok = true; u32 pos = count_trailing_zeroes(index); assert(index < SQLITE_MAX_UINT); @@ -281,7 +284,7 @@ bool wallet_shachain_add_hash(struct wallet *wallet, return false; } - db_begin_transaction(wallet->db); + should_commit = db_begin_transaction(wallet->db); stmt = db_prepare(wallet->db, "UPDATE shachains SET num_valid=?, min_index=? WHERE id=?"); sqlite3_bind_int(stmt, 1, chain->chain.num_valid); @@ -298,10 +301,12 @@ bool wallet_shachain_add_hash(struct wallet *wallet, sqlite3_bind_blob(stmt, 4, hash, sizeof(*hash), SQLITE_TRANSIENT); ok &= db_exec_prepared(wallet->db, stmt); - if (ok) - ok &= db_commit_transaction(wallet->db); - else - db_rollback_transaction(wallet->db); + if (should_commit) { + if (ok) + ok &= db_commit_transaction(wallet->db); + else + db_rollback_transaction(wallet->db); + } return ok; } @@ -625,12 +630,12 @@ bool wallet_channel_config_load(struct wallet *w, const u64 id, } bool wallet_channel_save(struct wallet *w, struct wallet_channel *chan){ - bool ok = true; + bool should_commit, ok = true; struct peer *p = chan->peer; tal_t *tmpctx = tal_tmpctx(w); sqlite3_stmt *stmt; - db_begin_transaction(w->db); + should_commit = db_begin_transaction(w->db); if (p->dbid == 0) { /* Need to store the peer first */ @@ -751,10 +756,12 @@ bool wallet_channel_save(struct wallet *w, struct wallet_channel *chan){ ok &= db_exec_prepared(w->db, stmt); } - if (ok) - ok &= db_commit_transaction(w->db); - else - db_rollback_transaction(w->db); + if (should_commit) { + if (ok) + ok &= db_commit_transaction(w->db); + else + db_rollback_transaction(w->db); + } tal_free(tmpctx); return ok; }