fix: keep counter sample time pairs synchronized

CHANGELOG.md

@@ -7,6 +7,7 @@
 * issue#710: Fixing Typo in thold_daemons.service File
 * issue#714: Increase the Name column to 255 characters
 * issue#719: Plugin Disabled due to mix of string and int
+* issue#815: Keep counter sample values and timestamps synchronized, recover from backward sample clocks, preserve alert state while samples are unavailable, and fail closed when expression sources cannot be resolved
 * issue: All Columns checkd on Thresholds page
 * issue: Special character previous value handling broken on data query indexes with special characters
 

README.md

@@ -27,6 +27,24 @@ and become familiar with its settings.  From there, you can provide overall
 control of thold, and set defaults for things like Email bodies, weekend
 exemptions, alert log retention, logging, etc.
 
+Counter, derive, and absolute rate thresholds preserve the previous value and
+timestamp together when a poll has no numeric sample, preventing the next rate
+from using mismatched interval data. A gap of more than two effective sampling
+intervals (the greater of the RRD step and poller interval), or the configured
+RRD heartbeat when available, is treated as unknown while the current sample
+starts a fresh baseline instead of producing a stale rate. Thresholds with an
+unknown rate remain eligible for the next poll, but that poll preserves the
+existing alert state and writes a warning instead of treating the missing
+sample as a restoral. Warnings are emitted only when the threshold enters the
+unavailable state. A backward sample clock is re-anchored without calculating a
+rate for that cycle, so later samples can recover normally. Expression
+thresholds require a numeric sibling value in the current poll and use its
+cached DSStats rate, with an RRD fallback, even when the sibling has no
+threshold row. This preserves COUNTER and DERIVE rate units without adding an
+rrdtool process per expression during normal operation. They fail closed when
+the current sibling value is unavailable. Gauge readings remain valid across
+such a gap.
+
 As with much of Cacti, settings should be documented in line with the actual
 setting.  If you find that any of these settings are ambiguous, please create a
 pull request with your proposed changes.

includes/polling.php

@@ -138,7 +138,7 @@ function thold_poller_output(&$rrd_update_array) {
 		td.cdef, td.local_data_id, td.data_template_rrd_id, td.lastread,
 		UNIX_TIMESTAMP(td.lasttime) AS lasttime, td.oldvalue,
 		td.data_source_name AS name, dtr.data_source_type_id,
-		dtd.rrd_step, dtr.rrd_maximum
+		dtd.rrd_step, dtr.rrd_maximum, dtr.rrd_heartbeat
 		FROM thold_data AS td
 		LEFT JOIN data_template_rrd AS dtr
 		ON dtr.id = td.data_template_rrd_id
@@ -148,7 +148,8 @@ function thold_poller_output(&$rrd_update_array) {
 		AND td.local_data_id IN($local_data_ids)");
 
 	if (cacti_sizeof($tholds)) {
-		$sql = [];
+		$sql        = [];
+		$status_sql = [];
 
 		foreach ($tholds as $thold_data) {
 			thold_debug("Checking Threshold: Name: '" . $thold_data['thold_name'] . "', Graph: '" . $thold_data['local_graph_id'] . "'");
@@ -206,17 +207,13 @@ function thold_poller_output(&$rrd_update_array) {
 				}
 			}
 
-			// This stores the raw value into the data source and is important for
-			// Counters, where calculating the difference is important.
-			// The unset case is problematic and may lead to false triggering
-			// events.  So, in those cases, we will store the 'oldvalue'.
-			if (isset($item[$thold_data['name']])) {
-				$rawvalue = $item[$thold_data['name']];
-			} else {
-				$rawvalue = $thold_data['oldvalue'];
-			}
+			$sample_rows = thold_polling_sample_row($thold_data, $item, $currentval, $currenttime);
 
-			$sql[] = '(' . $thold_data['id'] . ', 1, ' . db_qstr($currentval) . ', FROM_UNIXTIME(' . $currenttime . '), ' . db_qstr($rawvalue) . ')';
+			if ($sample_rows['sample_row'] !== null) {
+				$sql[] = $sample_rows['sample_row'];
+			} elseif ($sample_rows['status_row'] !== null) {
+				$status_sql[] = $sample_rows['status_row'];
+			}
 		}
 
 		if (cacti_sizeof($sql)) {
@@ -233,13 +230,30 @@ function thold_poller_output(&$rrd_update_array) {
 						oldvalue = VALUES(oldvalue)');
 			}
 
-			// accommodate deleted tholds
-			db_execute('DELETE FROM thold_data WHERE local_data_id = 0');
+		}
+
+		if (cacti_sizeof($status_sql)) {
+			foreach (array_chunk($status_sql, 400) as $chunk) {
+				$placeholders = implode(', ', array_fill(0, cacti_sizeof($chunk), '(?, ?, ?)'));
+				$params       = [];
 
-			if (db_affected_rows() > 0) {
-				set_config_option('time_last_change_thold', time());
+				foreach ($chunk as $row) {
+					$params[] = $row['id'];
+					$params[] = $row['tcheck'];
+					$params[] = $row['lastread'];
+				}
+
+				db_execute_prepared('INSERT INTO thold_data
+					(id, tcheck, lastread)
+					VALUES ' . $placeholders . '
+					ON DUPLICATE KEY UPDATE
+						tcheck = VALUES(tcheck),
+						lastread = VALUES(lastread)',
+					$params);
 			}
 		}
+
+		thold_polling_cleanup(cacti_sizeof($sql) || cacti_sizeof($status_sql));
 	}
 
 	return $rrd_update_array;

tests/Unit/GetCurrentValueTest.php

@@ -96,6 +96,12 @@ public function testMissingDataSourceNamesReturnsZero(): void {
 		$this->rrdReturns([]);
 
 		$this->assertSame(0, get_current_value(4, 'traffic_in'));
+
+		CactiStubs::reset();
+		CactiStubs::willReturn('db_fetch_row_prepared', ['rrd_step' => 300]);
+		CactiStubs::willReturn('rrdtool_execute', '1700000000');
+		$this->rrdReturns([]);
+		$this->assertSame('', get_current_value(4, 'traffic_in', 0, ''));
 	}
 
 	/**
@@ -105,6 +111,12 @@ public function testMissingValuesReturnsZero(): void {
 		$this->rrdReturns(['data_source_names' => ['traffic_in']]);
 
 		$this->assertSame(0, get_current_value(4, 'traffic_in'));
+
+		CactiStubs::reset();
+		CactiStubs::willReturn('db_fetch_row_prepared', ['rrd_step' => 300]);
+		CactiStubs::willReturn('rrdtool_execute', '1700000000');
+		$this->rrdReturns(['data_source_names' => ['traffic_out'], 'values' => [['1700000000' => 20.0]]]);
+		$this->assertSame('', get_current_value(4, 'traffic_in', 0, ''));
 	}
 
 	/**

tests/Unit/TholdCalculatePercentTest.php

@@ -80,8 +80,8 @@ public function testZeroDenominatorGivesZeroRatherThanDividingByZero(): void {
 	/**
 	 * @return void
 	 */
-	public function testNonNumericDenominatorGivesZero(): void {
-		$this->assertSame(0, $this->percent('U'));
+	public function testNonNumericDenominatorYieldsTheNoValueSentinel(): void {
+		$this->assertSame('', $this->percent('U'));
 	}
 
 	/**