From 74eba5f605fafc331e0e08d50c2da8d688e6f42e Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 1 Feb 2024 21:51:42 +0100 Subject: [PATCH] Remove config option database.DBexport. Its implementation was broken by design as its value was always overwritten by the condition (database.maxDBdays > 0). Replace it with exactly this condition and document the behavior in the config file Signed-off-by: DL6ER --- src/api/docs/content/specs/config.yaml | 3 -- src/config/config.c | 6 ---- src/config/config.h | 1 - src/config/legacy_reader.c | 8 ++--- src/database/common.c | 8 ++--- src/database/database-thread.c | 42 ++++++++++++-------------- src/main.c | 2 +- test/pihole.toml | 1 + 8 files changed, 28 insertions(+), 43 deletions(-) diff --git a/src/api/docs/content/specs/config.yaml b/src/api/docs/content/specs/config.yaml index 69a458fa..bab4fe43 100644 --- a/src/api/docs/content/specs/config.yaml +++ b/src/api/docs/content/specs/config.yaml @@ -338,8 +338,6 @@ components: properties: DBimport: type: boolean - DBexport: - type: boolean maxDBdays: type: integer DBinterval: @@ -660,7 +658,6 @@ components: refreshNames: IPV4_ONLY database: DBimport: true - DBexport: true maxDBdays: 365 DBinterval: 60 useWAL: true diff --git a/src/config/config.c b/src/config/config.c index 122b26e6..7255675e 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -832,12 +832,6 @@ void initConfig(struct config *conf) conf->database.DBimport.d.b = true; conf->database.DBimport.c = validate_stub; // Only type-based checking - conf->database.DBexport.k = "database.DBexport"; - conf->database.DBexport.h = "Should FTL store queries in the long-term database?"; - conf->database.DBexport.t = CONF_BOOL; - conf->database.DBexport.d.b = true; - conf->database.DBexport.c = validate_stub; // Only type-based checking - conf->database.maxDBdays.k = "database.maxDBdays"; conf->database.maxDBdays.h = "How long should queries be stored in the database [days]?"; conf->database.maxDBdays.t = CONF_INT; diff --git a/src/config/config.h b/src/config/config.h index cba4cbdf..524b4466 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -203,7 +203,6 @@ struct config { struct { struct conf_item DBimport; - struct conf_item DBexport; struct conf_item maxDBdays; struct conf_item DBinterval; struct conf_item useWAL; diff --git a/src/config/legacy_reader.c b/src/config/legacy_reader.c index 03760cab..857bcc2b 100644 --- a/src/config/legacy_reader.c +++ b/src/config/legacy_reader.c @@ -132,9 +132,9 @@ const char *readFTLlegacy(struct config *conf) if(value > maxdbdays_max) value = maxdbdays_max; - // Only use valid values - if(value == -1 || value >= 0) - conf->database.maxDBdays.v.i = value; + // Import value if it is >= than 0, convert negative values + // to 0 to disable the database + conf->database.maxDBdays.v.ui = value >= 0 ? value : 0; } // RESOLVE_IPV6 @@ -177,7 +177,7 @@ const char *readFTLlegacy(struct config *conf) // Use standard path if path was set to zero but override // MAXDBDAYS=0 to ensure no queries are stored in the database conf->files.database.v.s = conf->files.database.d.s; - conf->database.maxDBdays.v.i = 0; + conf->database.maxDBdays.v.ui = 0; } // MAXLOGAGE diff --git a/src/database/common.c b/src/database/common.c index 7fedd09a..2161fd38 100644 --- a/src/database/common.c +++ b/src/database/common.c @@ -578,13 +578,9 @@ void db_init(void) dbclose(&db); // Log if users asked us to not use the long-term database for queries - // We will still use it to store warnings in it - config.database.DBexport.v.b = true; - if(config.database.maxDBdays.v.i == 0) - { + // We will still use it to store warnings (Pi-hole diagnosis system) + if(config.database.maxDBdays.v.ui == 0) log_info("Not using the database for storing queries"); - config.database.DBexport.v.b = false; - } log_info("Database successfully initialized"); } diff --git a/src/database/database-thread.c b/src/database/database-thread.c index 250c6f05..efdd1bd6 100644 --- a/src/database/database-thread.c +++ b/src/database/database-thread.c @@ -41,7 +41,7 @@ static bool delete_old_queries_in_DB(sqlite3 *db) // Even when the database storing interval is set to 1 hour, this // method would still delete 24% of the database per day so maxDBdays > 4 // does still work. - const time_t timestamp = time(NULL) - config.database.maxDBdays.v.i * 86400; + const time_t timestamp = time(NULL) - config.database.maxDBdays.v.ui * 86400; SQL_bool(db, "DELETE FROM query_storage WHERE id IN (SELECT id FROM query_storage WHERE timestamp <= %lu LIMIT (SELECT COUNT(*)/100 FROM query_storage));", (unsigned long)timestamp); @@ -135,34 +135,32 @@ void *DB_thread(void *val) break; // Store queries in on-disk database - if(now - lastDBsave >= (time_t)config.database.DBinterval.v.ui) + if(config.database.maxDBdays.v.ui > 0 && + now - lastDBsave >= (time_t)config.database.DBinterval.v.ui) { // Update lastDBsave timer lastDBsave = now - now%config.database.DBinterval.v.ui; - // Save data to database (if enabled) - if(config.database.DBexport.v.b) + // Save data to database + DBOPEN_OR_AGAIN(); + lock_shm(); + export_queries_to_disk(false); + unlock_shm(); + + // Intermediate cancellation-point + if(killed) + break; + + // Check if GC should be done on the database + if(DBdeleteoldqueries) { - DBOPEN_OR_AGAIN(); - lock_shm(); - export_queries_to_disk(false); - unlock_shm(); - - // Intermediate cancellation-point - if(killed) - break; - - // Check if GC should be done on the database - if(DBdeleteoldqueries && config.database.maxDBdays.v.i != -1) - { - // No thread locks needed - delete_old_queries_in_DB(db); - DBdeleteoldqueries = false; - } - - DBCLOSE_OR_BREAK(); + // No thread locks needed + delete_old_queries_in_DB(db); + DBdeleteoldqueries = false; } + DBCLOSE_OR_BREAK(); + // Parse neighbor cache (fill network table) set_event(PARSE_NEIGHBOR_CACHE); } diff --git a/src/main.c b/src/main.c index f5b0419d..3d18c74e 100644 --- a/src/main.c +++ b/src/main.c @@ -148,7 +148,7 @@ int main (int argc, char *argv[]) sleepms(250); // Save new queries to database (if database is used) - if(config.database.DBexport.v.b) + if(config.database.maxDBdays.v.ui > 0) { export_queries_to_disk(true); log_info("Finished final database update"); diff --git a/test/pihole.toml b/test/pihole.toml index b8165552..e8c05b91 100644 --- a/test/pihole.toml +++ b/test/pihole.toml @@ -482,6 +482,7 @@ DBexport = true # How long should queries be stored in the database [days]? + # Setting this to 0 disables exporting queries to the database. maxDBdays = 365 # How often do we store queries in FTL's database [seconds]?