From 1ecf31bae758a454c012a120ab3942fd1b1c3b1d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 21 Sep 2023 15:06:27 +0930 Subject: [PATCH] db: remove #if DEVELOPER in favor of runtime flag inside db struct. Signed-off-by: Rusty Russell --- db/bindings.c | 5 ++--- db/common.h | 8 +++++--- db/db_postgres.c | 2 -- db/utils.c | 30 +++++++++--------------------- db/utils.h | 6 +++--- plugins/bkpr/db.c | 2 +- plugins/bkpr/test/run-bkpr_db.c | 2 +- plugins/bkpr/test/run-recorder.c | 1 + wallet/db.c | 3 ++- wallet/test/run-db.c | 2 +- wallet/test/run-wallet.c | 2 +- 11 files changed, 26 insertions(+), 37 deletions(-) diff --git a/db/bindings.c b/db/bindings.c index 7a0951e31..eac0df0e8 100644 --- a/db/bindings.c +++ b/db/bindings.c @@ -616,7 +616,6 @@ struct onionreply *db_col_onionreply(const tal_t *ctx, void db_col_ignore(struct db_stmt *stmt, const char *colname) { -#if DEVELOPER - db_query_colnum(stmt, colname); -#endif + if (stmt->db->developer) + db_query_colnum(stmt, colname); } diff --git a/db/common.h b/db/common.h index 2e87bf05e..518f78fcc 100644 --- a/db/common.h +++ b/db/common.h @@ -61,6 +61,9 @@ struct db { u32 data_version; void (*report_changes_fn)(struct db *); + + /* Set by --developer */ + bool developer; }; struct db_query { @@ -128,10 +131,9 @@ struct db_stmt { int row; -#if DEVELOPER - /* Map as we reference into a SELECT statement in query. */ + /* --developer: map as we reference into a SELECT statement + * in query. */ struct strset *cols_used; -#endif }; struct db_query_set { diff --git a/db/db_postgres.c b/db/db_postgres.c index 20323a122..68cfd9561 100644 --- a/db/db_postgres.c +++ b/db/db_postgres.c @@ -255,12 +255,10 @@ static bool db_postgres_vacuum(struct db *db) { PGresult *res; -#if DEVELOPER /* This can use a lot of diskspacem breaking CI! */ if (getenv("LIGHTNINGD_POSTGRES_NO_VACUUM") && streq(getenv("LIGHTNINGD_POSTGRES_NO_VACUUM"), "1")) return true; -#endif res = PQexec(db->conn, "VACUUM FULL;"); if (PQresultStatus(res) != PGRES_COMMAND_OK) { diff --git a/db/utils.c b/db/utils.c index 33e797ab1..fa7efc2da 100644 --- a/db/utils.c +++ b/db/utils.c @@ -31,9 +31,8 @@ size_t db_query_colnum(const struct db_stmt *stmt, col = (col + 1) % stmt->query->num_colnames; } -#if DEVELOPER - strset_add(stmt->cols_used, colname); -#endif + if (stmt->db->developer) + strset_add(stmt->cols_used, colname); return stmt->query->colnames[col].val; } @@ -43,9 +42,8 @@ static void db_stmt_free(struct db_stmt *stmt) if (!stmt->executed) db_fatal(stmt->db, "Freeing an un-executed statement from %s: %s", stmt->location, stmt->query->query); -#if DEVELOPER /* If they never got a db_step, we don't track */ - if (stmt->cols_used) { + if (stmt->db->developer && stmt->cols_used) { for (size_t i = 0; i < stmt->query->num_colnames; i++) { if (!stmt->query->colnames[i].sqlname) continue; @@ -58,7 +56,7 @@ static void db_stmt_free(struct db_stmt *stmt) } strset_clear(stmt->cols_used); } -#endif + if (stmt->inner_stmt) stmt->db->config->stmt_free_fn(stmt); assert(stmt->inner_stmt == NULL); @@ -81,16 +79,12 @@ static struct db_stmt *db_prepare_core(struct db *db, stmt->query = db_query; stmt->executed = false; stmt->inner_stmt = NULL; + stmt->cols_used = NULL; stmt->bind_pos = -1; tal_add_destructor(stmt, db_stmt_free); - list_add(&db->pending_statements, &stmt->list); -#if DEVELOPER - stmt->cols_used = NULL; -#endif /* DEVELOPER */ - return stmt; } @@ -166,13 +160,12 @@ bool db_step(struct db_stmt *stmt) assert(stmt->executed); ret = stmt->db->config->step_fn(stmt); -#if DEVELOPER /* We only track cols_used if we return a result! */ - if (ret && !stmt->cols_used) { + if (stmt->db->developer && ret && !stmt->cols_used) { stmt->cols_used = tal(stmt, struct strset); strset_init(stmt->cols_used); } -#endif + return ret; } @@ -254,7 +247,6 @@ void db_changes_add(struct db_stmt *stmt, const char * expanded) tal_arr_expand(&db->changes, tal_strdup(db->changes, expanded)); } -#if DEVELOPER void db_assert_no_outstanding_statements(struct db *db) { struct db_stmt *stmt; @@ -263,12 +255,6 @@ void db_assert_no_outstanding_statements(struct db *db) if (stmt) db_fatal(stmt->db, "Unfinalized statement %s", stmt->location); } -#else -void db_assert_no_outstanding_statements(struct db *db) -{ -} -#endif - static void destroy_db(struct db *db) { @@ -337,6 +323,7 @@ void db_warn(const struct db *db, const char *fmt, ...) } struct db *db_open_(const tal_t *ctx, const char *filename, + bool developer, void (*errorfn)(void *arg, bool fatal, const char *fmt, va_list ap), void *arg) { @@ -344,6 +331,7 @@ struct db *db_open_(const tal_t *ctx, const char *filename, db = tal(ctx, struct db); db->filename = tal_strdup(db, filename); + db->developer = developer; db->errorfn = errorfn; db->errorfn_arg = arg; list_head_init(&db->pending_statements); diff --git a/db/utils.h b/db/utils.h index f83ea87b5..0cb699ee8 100644 --- a/db/utils.h +++ b/db/utils.h @@ -85,13 +85,13 @@ struct db_stmt *db_prepare_v2_(const char *location, struct db *db, /** * db_open - Open or create a database */ -#define db_open(ctx, filename, errfn, arg) \ - db_open_((ctx), (filename), \ +#define db_open(ctx, filename, developer, errfn, arg) \ + db_open_((ctx), (filename), (developer), \ typesafe_cb_postargs(void, void *, (errfn), (arg), \ bool, const char *, va_list), \ (arg)) -struct db *db_open_(const tal_t *ctx, const char *filename, +struct db *db_open_(const tal_t *ctx, const char *filename, bool developer, void (*errorfn)(void *arg, bool fatal, const char *fmt, va_list ap), void *arg); diff --git a/plugins/bkpr/db.c b/plugins/bkpr/db.c index 56d2da8de..330255a3e 100644 --- a/plugins/bkpr/db.c +++ b/plugins/bkpr/db.c @@ -200,7 +200,7 @@ struct db *db_setup(const tal_t *ctx, struct plugin *p, bool migrated; struct db *db; - db = db_open(ctx, db_dsn, db_error, p); + db = db_open(ctx, db_dsn, plugin_developer_mode(p), db_error, p); db->report_changes_fn = NULL; db_begin_transaction(db); diff --git a/plugins/bkpr/test/run-bkpr_db.c b/plugins/bkpr/test/run-bkpr_db.c index 49e5d1e3f..bd4e92e89 100644 --- a/plugins/bkpr/test/run-bkpr_db.c +++ b/plugins/bkpr/test/run-bkpr_db.c @@ -240,7 +240,7 @@ static struct db *create_test_db(void) char *dsn; dsn = tmp_dsn(NULL); - db = db_open(NULL, dsn, db_error, (struct plugin *)NULL); + db = db_open(NULL, dsn, true, db_error, (struct plugin *)NULL); db->data_version = 0; db->report_changes_fn = NULL; diff --git a/plugins/bkpr/test/run-recorder.c b/plugins/bkpr/test/run-recorder.c index ee45b90b2..a082b9b26 100644 --- a/plugins/bkpr/test/run-recorder.c +++ b/plugins/bkpr/test/run-recorder.c @@ -1376,6 +1376,7 @@ int main(int argc, char *argv[]) /* Dummy for migration hooks */ struct plugin *plugin = tal(NULL, struct plugin); list_head_init(&plugin->js_list); + plugin->developer = true; common_setup(argv[0]); diff --git a/wallet/db.c b/wallet/db.c index 5aaad02d4..abab020f5 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -1057,7 +1057,8 @@ static void db_error(struct lightningd *ld, bool fatal, const char *fmt, va_list struct db *db_setup(const tal_t *ctx, struct lightningd *ld, const struct ext_key *bip32_base) { - struct db *db = db_open(ctx, ld->wallet_dsn, db_error, ld); + struct db *db = db_open(ctx, ld->wallet_dsn, ld->developer, + db_error, ld); bool migrated; db->report_changes_fn = plugin_hook_db_sync; diff --git a/wallet/test/run-db.c b/wallet/test/run-db.c index c69c04d81..995560ba3 100644 --- a/wallet/test/run-db.c +++ b/wallet/test/run-db.c @@ -288,7 +288,7 @@ static struct db *create_test_db(void) dsn = tal_fmt(NULL, "sqlite3://%s", filename); tal_free(filename); - db = db_open(NULL, dsn, db_error, (struct lightningd *)NULL); + db = db_open(NULL, dsn, true, db_error, (struct lightningd *)NULL); db->data_version = 0; db->report_changes_fn = NULL; diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index bb518cbf5..d5bab6e8f 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -1002,7 +1002,7 @@ static struct wallet *create_test_wallet(struct lightningd *ld, const tal_t *ctx close(fd); dsn = tal_fmt(NULL, "sqlite3://%s", filename); - w->db = db_open(w, dsn, test_error, ld); + w->db = db_open(w, dsn, true, test_error, ld); w->db->report_changes_fn = NULL; tal_free(dsn); tal_add_destructor2(w, cleanup_test_wallet, filename);