Additional checks for valid db during db_execute (#20560)
Additional checks for valid db during db_execute -- shutdown will close and NULL the databases Update db_execute calls to include NULL for sqlite_rc parameter. This will return the actual db error and help refine error handling from the caller
Stelios Fragkakis committed
Jun 24, 2025 at 15:12 UTC
d7809e1a654670fb9f479fe3363e0a6e4b650b05
7 files changed
+38
-27
src/database/sqlite/sqlite_aclk.c
+1
-1
@@ -207,7 +207,7 @@ static void sql_delete_aclk_table_list(void)
207
208
SQLITE_FINALIZE(res);
209
210
- int rc = db_execute(db_meta, buffer_tostring(sql));
210
+ int rc = db_execute(db_meta, buffer_tostring(sql), NULL);
211
if (unlikely(rc))
212
netdata_log_error("Failed to drop unused ACLK tables");
213
src/database/sqlite/sqlite_db_migration.c
+2
-2
@@ -402,7 +402,7 @@ static int do_migration_v14_v15(sqlite3 *database)
402
SQLITE_FINALIZE(res);
403
404
if (count)
405
- (void) db_execute(database, buffer_tostring(wb));
405
+ (void)db_execute(database, buffer_tostring(wb), NULL);
406
407
buffer_free(wb);
408
return 0;
@@ -431,7 +431,7 @@ static int do_migration_v15_v16(sqlite3 *database)
431
SQLITE_FINALIZE(res);
432
433
if (count)
434
- (void) db_execute(database, buffer_tostring(wb));
434
+ (void)db_execute(database, buffer_tostring(wb), NULL);
435
436
buffer_free(wb);
437
return 0;
src/database/sqlite/sqlite_functions.c
+9
-2
@@ -297,11 +297,15 @@ int init_database_batch(sqlite3 *database, const char *batch[], const char *desc
297
298
// Return 0 OK
299
// Return 1 Failed
300
-int db_execute(sqlite3 *db, const char *cmd)
300
+// sqlite_rc - if not NULL, it will be set to the return code of the sqlite3_exec_monitored call
301
+int db_execute(sqlite3 *db, const char *cmd, int *sqlite_rc)
302
{
303
int rc;
304
int cnt = 0;
305
306
+ if (unlikely(!db))
307
+ return 1;
308
+
309
while (cnt < SQL_MAX_RETRY) {
310
char *err_msg = NULL;
311
rc = sqlite3_exec_monitored(db, cmd, 0, 0, &err_msg);
@@ -323,6 +327,9 @@ int db_execute(sqlite3 *db, const char *cmd)
327
mark_database_to_recover(NULL, db, rc);
328
break;
329
}
330
+ if (sqlite_rc)
331
+ *sqlite_rc = rc;
332
+
333
return (rc != SQLITE_OK);
334
}
335
@@ -395,7 +402,7 @@ void sql_close_database(sqlite3 *database, const char *database_name)
402
if (unlikely(!database))
403
return;
404
398
- (void) db_execute(database, "PRAGMA optimize");
405
+ (void)db_execute(database, "PRAGMA optimize", NULL);
406
407
netdata_log_info("%s: Closing sqlite database", database_name);
408
src/database/sqlite/sqlite_functions.h
+1
-1
@@ -105,7 +105,7 @@ void finalize_self_prepared_sql_statements();
105
void finalize_all_prepared_sql_statements();
106
107
int execute_insert(sqlite3_stmt *res);
108
-int db_execute(sqlite3 *database, const char *cmd);
108
+int db_execute(sqlite3 *database, const char *cmd, int *sqlite_rc);
109
char *get_database_extented_error(sqlite3 *database, int i, const char *description);
110
111
void sql_drop_table(const char *table);
src/database/sqlite/sqlite_health.c
+4
-4
@@ -1412,7 +1412,7 @@ void sql_alert_transitions(
1412
}
1413
1414
snprintfz(sql, sizeof(sql) - 1, SQL_BUILD_ALERT_TRANSITION, nodes);
1415
- rc = db_execute(db_meta, sql);
1415
+ rc = db_execute(db_meta, sql, NULL);
1416
if (rc)
1417
return;
1418
@@ -1519,7 +1519,7 @@ done:
1519
done_only_drop:
1520
if (likely(!transition)) {
1521
(void)snprintfz(sql, sizeof(sql) - 1, "DROP TABLE IF EXISTS v_%p", nodes);
1522
- (void)db_execute(db_meta, sql);
1522
+ (void)db_execute(db_meta, sql, NULL);
1523
buffer_free(command);
1524
}
1525
}
@@ -1552,7 +1552,7 @@ int sql_get_alert_configuration(
1552
return added;
1553
1554
snprintfz(sql, sizeof(sql) - 1, SQL_BUILD_CONFIG_TARGET_LIST, configs);
1555
- rc = db_execute(db_meta, sql);
1555
+ rc = db_execute(db_meta, sql, NULL);
1556
if (rc)
1557
return added;
1558
@@ -1645,7 +1645,7 @@ int sql_get_alert_configuration(
1645
1646
fail_only_drop:
1647
(void)snprintfz(sql, sizeof(sql) - 1, "DROP TABLE IF EXISTS c_%p", configs);
1648
- (void)db_execute(db_meta, sql);
1648
+ (void)db_execute(db_meta, sql, NULL);
1649
buffer_free(command);
1650
return added;
1651
}
src/database/sqlite/sqlite_metadata.c
+18
-14
@@ -581,7 +581,7 @@ static void recover_database(const char *sqlite_database, const char *new_sqlite
581
netdata_log_info(" to %s", new_sqlite_database);
582
583
// This will remove the -shm and -wal files when we close the database
584
- (void) db_execute(database, "select count(*) from sqlite_master limit 0");
584
+ (void)db_execute(database, "select count(*) from sqlite_master limit 0", NULL);
585
586
sqlite3_recover *recover = sqlite3_recover_init(database, "main", new_sqlite_database);
587
if (recover) {
@@ -736,7 +736,7 @@ int sql_init_meta_database(db_check_action_type_t rebuild, int memory)
736
sqlite3_free(err_msg);
737
}
738
else {
739
- (void) db_execute(db_meta, "select count(*) from sqlite_master limit 0");
739
+ (void)db_execute(db_meta, "select count(*) from sqlite_master limit 0", NULL);
740
(void) sqlite3_close(db_meta);
741
}
742
return 1;
@@ -751,7 +751,7 @@ int sql_init_meta_database(db_check_action_type_t rebuild, int memory)
751
sqlite3_free(err_msg);
752
}
753
else {
754
- (void) db_execute(db_meta, "select count(*) from sqlite_master limit 0");
754
+ (void)db_execute(db_meta, "select count(*) from sqlite_master limit 0", NULL);
755
(void) sqlite3_close(db_meta);
756
}
757
return 1;
@@ -840,7 +840,7 @@ static int check_and_update_chart_labels(RRDSET *st, BUFFER *work_buffer)
840
uuid_unparse_lower(st->chart_uuid, tmp.uuid_str);
841
rrdlabels_walkthrough_read(st->rrdlabels, chart_label_store_to_sql_callback, &tmp);
842
buffer_strcat(work_buffer, " ON CONFLICT (chart_id, label_key) DO UPDATE SET source_type = excluded.source_type, label_value=excluded.label_value, date_created=UNIXEPOCH()");
843
- int rc = db_execute(db_meta, buffer_tostring(work_buffer));
843
+ int rc = db_execute(db_meta, buffer_tostring(work_buffer), NULL);
844
if (likely(!rc))
845
st->rrdlabels_last_saved_version = new_version;
846
@@ -854,7 +854,7 @@ void detect_machine_guid_change(nd_uuid_t *host_uuid)
854
855
rc = exec_statement_with_uuid(CONVERT_EXISTING_LOCALHOST, host_uuid);
856
if (!rc) {
857
- if (unlikely(db_execute(db_meta, DELETE_MISSING_NODE_INSTANCES)))
857
+ if (unlikely(db_execute(db_meta, DELETE_MISSING_NODE_INSTANCES, NULL)))
858
error_report("Failed to remove deleted hosts from node instances");
859
}
860
}
@@ -1499,9 +1499,13 @@ static void cleanup_health_log(struct meta_config_s *config)
1499
return;
1500
}
1501
1502
- (void) db_execute(db_meta,"DELETE FROM health_log WHERE host_id NOT IN (SELECT host_id FROM host)");
1503
- (void) db_execute(db_meta,"DELETE FROM health_log_detail WHERE health_log_id NOT IN (SELECT health_log_id FROM health_log)");
1504
- (void) db_execute(db_meta,"DELETE FROM alert_version WHERE health_log_id NOT IN (SELECT health_log_id FROM health_log)");
1502
+ (void)db_execute(db_meta, "DELETE FROM health_log WHERE host_id NOT IN (SELECT host_id FROM host)", NULL);
1503
+ (void)db_execute(
1504
+ db_meta,
1505
+ "DELETE FROM health_log_detail WHERE health_log_id NOT IN (SELECT health_log_id FROM health_log)",
1506
+ NULL);
1507
+ (void)db_execute(
1508
+ db_meta, "DELETE FROM alert_version WHERE health_log_id NOT IN (SELECT health_log_id FROM health_log)", NULL);
1509
worker_is_idle();
1510
}
1511
@@ -1573,7 +1577,7 @@ void vacuum_database(sqlite3 *database, const char *db_alias, int threshold, int
1577
1578
char sql[128];
1579
snprintfz(sql, sizeof(sql) - 1, "PRAGMA incremental_vacuum(%d)", do_free_pages);
1576
- (void)db_execute(database, sql);
1580
+ (void)db_execute(database, sql, NULL);
1581
}
1582
}
1583
@@ -1972,7 +1976,7 @@ size_t populate_metrics_from_database(void *mrg, void (*populate_cb)(void *mrg,
1976
}
1977
1978
if (local_meta_db)
1975
- (void) db_execute(local_meta_db, "PRAGMA cache_size=10000");
1979
+ (void)db_execute(local_meta_db, "PRAGMA cache_size=10000", NULL);
1980
1981
if (!PREPARE_STATEMENT(local_meta_db ? local_meta_db : db_meta, GET_UUID_LIST, &res)) {
1982
sqlite3_close(local_meta_db);
@@ -2017,7 +2021,7 @@ static void metadata_scan_host(RRDHOST *host, BUFFER *work_buffer, bool is_worke
2021
bool load_ml_models = is_worker;
2022
2023
bool host_need_recheck = false;
2020
- (void)db_execute(db_meta, "BEGIN TRANSACTION");
2024
+ (void)db_execute(db_meta, "BEGIN TRANSACTION", NULL);
2025
2026
rrdset_foreach_reentrant(st, host) {
2027
@@ -2095,7 +2099,7 @@ static void metadata_scan_host(RRDHOST *host, BUFFER *work_buffer, bool is_worke
2099
}
2100
rrdset_foreach_done(st);
2101
2098
- (void)db_execute(db_meta, "COMMIT TRANSACTION");
2102
+ (void)db_execute(db_meta, "COMMIT TRANSACTION", NULL);
2103
if (host_need_recheck)
2104
rrdhost_flag_set(host,RRDHOST_FLAG_METADATA_UPDATE);
2105
@@ -2315,7 +2319,7 @@ static void meta_store_host_labels(RRDHOST *host, BUFFER *work_buffer)
2319
buffer_strcat(
2320
work_buffer,
2321
" ON CONFLICT (host_id, label_key) DO UPDATE SET source_type = excluded.source_type, label_value=excluded.label_value, date_created=UNIXEPOCH()");
2318
- rc = db_execute(db_meta, buffer_tostring(work_buffer));
2322
+ rc = db_execute(db_meta, buffer_tostring(work_buffer), NULL);
2323
2324
if (unlikely(rc)) {
2325
error_report("METADATA: 'host:%s': failed to update metadata host labels", rrdhost_hostname(host));
@@ -2871,7 +2875,7 @@ done:
2875
2876
void cleanup_agent_event_log(void)
2877
{
2874
- (void) db_execute(db_meta, "DELETE FROM agent_event_log WHERE date_created < UNIXEPOCH() - 30 * 86400");
2878
+ (void)db_execute(db_meta, "DELETE FROM agent_event_log WHERE date_created < UNIXEPOCH() - 30 * 86400", NULL);
2879
}
2880
2881
#define SQL_GET_AGENT_EVENT_TYPE_MEDIAN \
src/ml/ml.cc
+3
-3
@@ -992,7 +992,7 @@ static void ml_flush_pending_models(ml_worker_t *worker) {
992
int op_no = 1;
993
994
// begin transaction
995
- int rc = db_execute(ml_db, "BEGIN TRANSACTION;");
995
+ int rc = db_execute(ml_db, "BEGIN TRANSACTION;", NULL);
996
997
// add/delete models
998
if (!rc) {
@@ -1019,14 +1019,14 @@ static void ml_flush_pending_models(ml_worker_t *worker) {
1019
// commit transaction
1020
if (!rc) {
1021
op_no++;
1022
- rc = db_execute(ml_db, "COMMIT TRANSACTION;");
1022
+ rc = db_execute(ml_db, "COMMIT TRANSACTION;", NULL);
1023
}
1024
1025
// rollback transaction on failure
1026
if (rc) {
1027
netdata_log_error("Trying to rollback ML transaction because it failed with rc=%d, op_no=%d", rc, op_no);
1028
op_no++;
1029
- rc = db_execute(ml_db, "ROLLBACK;");
1029
+ rc = db_execute(ml_db, "ROLLBACK;", NULL);
1030
if (rc)
1031
netdata_log_error("ML transaction rollback failed with rc=%d", rc);
1032
}