Skip to content

Commit

Permalink
Remove config option database.DBexport. Its implementation was broken…
Browse files Browse the repository at this point in the history
… by design as its value was always overwritten by the consition (database.maxDBdays > 0). Replace it with exactly this condition and document the behavior in the config file

Signed-off-by: DL6ER <[email protected]>
  • Loading branch information
DL6ER committed Feb 1, 2024
1 parent 04cf2e8 commit d026e7b
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 45 deletions.
3 changes: 0 additions & 3 deletions src/api/docs/content/specs/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,6 @@ components:
properties:
DBimport:
type: boolean
DBexport:
type: boolean
maxDBdays:
type: integer
DBinterval:
Expand Down Expand Up @@ -668,7 +666,6 @@ components:
refreshNames: IPV4_ONLY
database:
DBimport: true
DBexport: true
maxDBdays: 365
DBinterval: 60
network:
Expand Down
10 changes: 2 additions & 8 deletions src/config/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -795,15 +795,9 @@ void initConfig(struct config *conf)
conf->database.DBimport.t = CONF_BOOL;
conf->database.DBimport.d.b = true;

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.maxDBdays.k = "database.maxDBdays";
conf->database.maxDBdays.h = "How long should queries be stored in the database [days]?";
conf->database.maxDBdays.t = CONF_INT;
conf->database.maxDBdays.d.i = (365/4);
conf->database.maxDBdays.h = "How long should queries be stored in the database [days]?\n Setting this to 0 disables exporting queries to the database.";
conf->database.maxDBdays.d.ui = (365u/4);

conf->database.DBinterval.k = "database.DBinterval";
conf->database.DBinterval.h = "How often do we store queries in FTL's database [seconds]?";
Expand Down
1 change: 0 additions & 1 deletion src/config/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,6 @@ struct config {

struct {
struct conf_item DBimport;
struct conf_item DBexport;
struct conf_item maxDBdays;
struct conf_item DBinterval;
struct {
Expand Down
8 changes: 4 additions & 4 deletions src/config/legacy_reader.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 2 additions & 6 deletions src/database/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -558,13 +558,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");
}
Expand Down
42 changes: 20 additions & 22 deletions src/database/database-thread.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
1 change: 1 addition & 0 deletions test/pihole.toml
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,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]?
Expand Down

0 comments on commit d026e7b

Please sign in to comment.