Merge pull request #2127 from pi-hole/tweak/dynamic_GCinterval

Dynamically lower the GC interval

src/FTL.h

@@ -52,14 +52,6 @@
 // Constant socket buffer length
 #define SOCKETBUFFERLEN 1024
 
-// How often do we garbage collect (to ensure we only have data fitting to the MAXLOGAGE defined above)? [seconds]
-// Default: 600 (10 minute intervals)
-#define GCinterval 600
-
-// Delay applied to the garbage collecting [seconds]
-// Default: -60 (one minute before the end of the interval set above)
-#define GCdelay (-60)
-
 // How many client connection do we accept at once?
 #define MAXCONNS 255
 
@@ -70,8 +62,8 @@
 #define MAXLOGAGE 24
 
 // Interval for overTime data [seconds]
-// Default: same as GCinterval
-#define OVERTIME_INTERVAL GCinterval
+// Default: 600 (10 minutes)
+#define OVERTIME_INTERVAL 600
 
 // How many overTime slots do we need?
 // This is the maximum log age divided by the overtime interval

src/gc.c

@@ -45,8 +45,22 @@
 // default: 10 seconds
 #define CPU_AVERAGE_INTERVAL 10
 
+// Global boolean indicating whether garbage collection is requested at the next
+// opportunity
 bool doGC = false;
 
+// The garbage collection interval [seconds]
+// Default: 600 seconds (10 minutes)
+static unsigned int GCinterval = 600;
+// Delay applied to the garbage collecting [seconds]
+// Default: 60 seconds (one minute before the end of the interval set above)
+// The delay is applied to the garbage collection interval to avoid
+// garbage collection to run on, e.g., a full hour boundary when other
+// tasks are scheduled to run on the host (e.g., cron-jobs) as - during
+// garbage collection - DNS resolution may be delayed for up to few
+// seconds on really slow systems
+static unsigned int GCdelay = 60;
+
 // Recycle old clients and domains in our internal data structure
 // This has the side-effect of recycling intermediate domains
 // seen during CNAME inspection, too, as they are never referenced
@@ -324,7 +338,7 @@ void runGC(const time_t now, time_t *lastGCrun, const bool flush)
 	doGC = false;
 	// Update lastGCrun timer
 	if(lastGCrun != NULL)
-		*lastGCrun = now - GCdelay - (now - GCdelay)%GCinterval;
+		*lastGCrun = now + GCdelay - (now + GCdelay)%GCinterval;
 
 	// Lock FTL's data structure, since it is likely that it will be changed here
 	// Requests should not be processed/answered when data is about to change
@@ -336,7 +350,7 @@ void runGC(const time_t now, time_t *lastGCrun, const bool flush)
 	if(!flush)
 	{
 		// Normal GC run
-		mintime -= GCdelay + config.webserver.api.maxHistory.v.ui;
+		mintime -= - GCdelay + config.webserver.api.maxHistory.v.ui;
 
 		// Align the start time of this GC run to the GCinterval. This will also align with the
 		// oldest overTime interval after GC is done.
@@ -530,7 +544,36 @@ static bool check_files_on_same_device(const char *path1, const char *path2)
 	return s1.st_dev == s2.st_dev;
 }
 
-static bool is_debugged = false;
+/**
+ * @brief Set the garbage collection interval dynamically based on the
+ *        webserver.api.maxHistory configuration.
+ *
+ * This function adjusts the garbage collection (GC) interval depending on the
+ * value of `webserver.api.maxHistory`. The default GC interval is ten minutes.
+ * However, if `webserver.api.maxHistory` is set to zero, indicating that the
+ * user wants to keep as few queries as possible in the history, the GC interval
+ * is reduced to one minute. Despite this reduction, a small number of queries
+ * are still retained to correlate replies with their corresponding queries. We
+ * also set the GC delay to ten seconds to ensure that the GC thread does not
+ * run at the top of the hour, which is a common time for other tasks to run on
+ * the host.
+ *
+ * This function is called during the initialization of the FTL engine by
+ * initOverTime()
+ *
+ * @return The current, possibly altered, GC interval in seconds.
+ */
+unsigned int set_gc_interval(void)
+{
+	if(config.webserver.api.maxHistory.v.ui == 0)
+	{
+		GCinterval = 60;
+		GCdelay = 10;
+	}
+
+	return GCinterval;
+}
+
 void *GC_thread(void *val)
 {
 	(void)val; // Mark parameter as unused
@@ -548,6 +591,9 @@ void *GC_thread(void *val)
 	unsigned int LastLogStorageUsage = 0;
 	unsigned int LastDBStorageUsage = 0;
 
+	// Whether an external tracer has been detected
+	bool is_debugged = false;
+
 	bool db_and_log_on_same_dev = false;
 	db_and_log_on_same_dev = check_files_on_same_device(config.files.database.v.s, config.files.log.ftl.v.s);
 
@@ -601,7 +647,7 @@ void *GC_thread(void *val)
 		if(killed)
 			break;
 
-		if(now - GCdelay - lastGCrun >= GCinterval || doGC)
+		if(now + GCdelay - lastGCrun >= GCinterval || doGC)
 			runGC(now, &lastGCrun, false);
 
 		// Intermediate cancellation-point

src/gc.h

@@ -16,6 +16,7 @@
 void *GC_thread(void *val);
 void runGC(const time_t now, time_t *lastGCrun, const bool flush);
 time_t get_rate_limit_turnaround(const unsigned int rate_limit_count);
+unsigned int set_gc_interval(void);
 
 // Defined in src/dnsmasq_interface.c
 void set_dnsmasq_debug(const bool debug, const pid_t pid);

src/ntp/client.c

@@ -607,13 +607,12 @@ static void *ntp_client_thread(void *arg)
 		// Get time after NTP sync
 		const double after = double_time();
 
-		// If the time was updated by more than a certain amount,
-		// restart FTL to import recent data. This is relevant when the
-		// system time was set to an incorrect value (e.g., due to a
-		// dead CMOS battery or overall missing RTC) and the time was
-		// off.
+		// If the time was updated by more than ten minutes, restart FTL
+		// to import recent data. This is relevant when the system time
+		// was set to an incorrect value (e.g., due to a dead CMOS
+		// battery or overall missing RTC) and the time was off.
 		double time_delta = fabs(after - before);
-		if(first_run && time_delta > GCinterval)
+		if(first_run && time_delta > 600)
 		{
 			log_info("System time was updated by %.1f seconds", time_delta);
 			restart_ftl("System time updated");

src/overTime.c

@@ -15,6 +15,8 @@
 #include "log.h"
 // data getter functions
 #include "datastructure.h"
+// set_gc_interval()
+#include "gc.h"
 
 overTimeData *overTime = NULL;
 
@@ -62,6 +64,9 @@ void initOverTime(void)
 	// Get current timestamp
 	time_t now = time(NULL);
 
+	// Get the garbage collection interval
+	const unsigned int GCinterval = set_gc_interval();
+
 	// Get the centered timestamp of the end of the next garbage collection interval
 	// This is necessary to construct all slots until the point where we are moving
 	// the entire overTime structure into the future (i.e., generate "new" slots)