bug: Skip partition REORGANIZE when an existing boundary already covers the day (#434)

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 --- develop ---
 
+* bug: Stop partition recovery from wedging with MySQL error 1493 ('VALUES LESS THAN value must be strictly increasing') when an existing partition boundary already covers the day being created. Both syslog_partition_check() and syslog_partition_create() decided a daily partition was missing purely by name, so a table carried over from an older TO_DAYS schema, or left inconsistent by an interrupted 4.2-to-4.11 upgrade, would repeatedly emit a REORGANIZE of dMaxValue that MySQL rejected, leaving writes parked in the dMaxValue safety partition. Both now compare the target boundary against the highest existing concrete boundary, so already-covered days no longer count as missing, no longer consume the bounded per-run recovery budget, and are treated as a successful no-op, while genuine gaps are still created (issue #434)
 * bug: Stop preloading every distinct program into the query-builder choice list; on a busy collector the unbounded SELECT over syslog_programs built a very large choice array that could exhaust the PHP memory limit and fatal (HTTP 503) when opening the Alert, Removal and Dashboard editors, while the Report editor (which does not use the filter builder) kept working. The program and program_id fields already resolve through the bounded AJAX suggestion endpoint, so only the small facility and priority enumerations are preloaded now
 * bug: Backport the PHP 8.0 syntax that prevented the Syslog pages from parsing under a PHP 7.4 web server (match() expressions, union type declarations such as string|false and int|string, and mixed type declarations) across includes/functions.php, syslog.php, syslog_reports.php and syslog_removal.php; includes/functions.php loads on every page, so on PHP 7.4 the file failed to compile and the page returned a blank 503 while the CLI poller on a newer PHP kept working. The documented parameter/return types are retained in the PHPDoc blocks, restoring the declared 1.2.23 compatibility floor
 * bug: Create dashboard, status, saved-search and share tables with a string primary key on Cacti older than 1.2.32, whose db_table_create() fataled on an array primary key, so the 4.2 to 4.11 upgrade completes on the 1.2.23+ compatibility floor

includes/functions.php

@@ -3031,6 +3031,52 @@ function syslog_partition_create($table, $time = null, &$reason = null) {
 			[$syslogdb_default, $cformat, $table]);
 
 		if (!cacti_sizeof($exists)) {
+			/*
+			 * The target partition name is absent, but an existing concrete
+			 * partition may already cover this day under a different name or
+			 * boundary. Tables carried over from an older schema, or left
+			 * inconsistent by an interrupted upgrade, can hold a partition
+			 * whose boundary already reaches or exceeds the target while using
+			 * a name the lookup above does not match. Splitting dMaxValue only
+			 * succeeds when the new boundary is strictly greater than every
+			 * existing concrete boundary; otherwise MySQL rejects it with
+			 * error 1493 ('VALUES LESS THAN value must be strictly
+			 * increasing'), which wedges recovery and parks writes in
+			 * dMaxValue indefinitely. Compare the target boundary against the
+			 * highest existing boundary first and treat an already-covered day
+			 * as a successful no-op instead of emitting a doomed REORGANIZE.
+			 */
+			$boundary_info = syslog_db_fetch_row_prepared('SELECT
+				MAX(CAST(partition_description AS SIGNED)) AS max_boundary,
+				MAX(partition_expression) AS partition_expression
+				FROM `information_schema`.`partitions`
+				WHERE table_schema = ?
+				AND table_name = ?
+				AND partition_description IS NOT NULL
+				AND partition_description != ?',
+				[$syslogdb_default, $table, 'MAXVALUE']);
+
+			$max_boundary         = isset($boundary_info['max_boundary']) ? $boundary_info['max_boundary'] : null;
+			$partition_expression = isset($boundary_info['partition_expression']) ? (string) $boundary_info['partition_expression'] : '';
+
+			if (stripos($partition_expression, 'to_days') !== false) {
+				$target_boundary = syslog_db_fetch_cell_prepared('SELECT TO_DAYS(?)', [$boundary_date]);
+			} elseif (stripos($partition_expression, 'unix_timestamp') !== false) {
+				$target_boundary = $boundary_epoch;
+			} else {
+				$target_boundary = null;
+			}
+
+			if ($target_boundary !== null && $max_boundary !== null
+				&& is_numeric($target_boundary) && is_numeric($max_boundary)
+				&& (float) $target_boundary <= (float) $max_boundary) {
+				cacti_log("SYSLOG: Partition '$cformat' on '$table' is already covered by an existing partition boundary ($max_boundary); skipping creation", false, 'SYSTEM');
+
+				$success = true;
+
+				return true;
+			}
+
 			$overflow = syslog_db_fetch_row_prepared('SELECT TABLE_ROWS AS overflow_rows,
 				DATA_LENGTH + INDEX_LENGTH AS overflow_bytes
 				FROM information_schema.PARTITIONS
@@ -3262,23 +3308,55 @@ function syslog_partition_check($table, $time = null) {
 		$time = time() + 3600;
 	}
 
-	// find date of last partition
-	$last_part = syslog_db_fetch_cell_prepared('SELECT PARTITION_NAME
+	/*
+	 * Decide whether a concrete partition still needs to be created for
+	 * $time by comparing boundaries, not partition names. A legacy or
+	 * partly-upgraded table can carry partition names that no longer line up
+	 * with their upper boundaries, so a name-only comparison both misses real
+	 * gaps and flags days an existing boundary already covers. Flagging a
+	 * covered day as missing would make recovery spend its bounded per-run
+	 * budget on no-op creations and starve the genuine gap. Mirror
+	 * syslog_partition_create(): the day needs a new partition only when its
+	 * target boundary is strictly greater than every existing concrete
+	 * boundary.
+	 */
+	$partition_expression = syslog_db_fetch_cell_prepared('SELECT partition_expression
 		FROM `information_schema`.`partitions`
 		WHERE table_schema = ?
 		AND table_name = ?
+		AND partition_description IS NOT NULL
+		AND partition_description != ?
 		ORDER BY partition_ordinal_position DESC
-		LIMIT 1,1',
-		[$syslogdb_default, $table]);
+		LIMIT 1',
+		[$syslogdb_default, $table, 'MAXVALUE']);
 
-	$lformat   = str_replace('d', '', $last_part);
-	$cformat   = gmdate('Ymd', $time);
+	$boundary_epoch = (intdiv((int) $time, 86400) + 1) * 86400;
 
-	if ($cformat > $lformat) {
-		return true;
+	if (stripos((string) $partition_expression, 'to_days') !== false) {
+		$target_boundary = syslog_db_fetch_cell_prepared('SELECT TO_DAYS(?)', [gmdate('Y-m-d', $boundary_epoch)]);
+	} elseif (stripos((string) $partition_expression, 'unix_timestamp') !== false) {
+		$target_boundary = $boundary_epoch;
 	} else {
-		return false;
+		// Boundary units are unknown (unpartitioned or unreadable metadata):
+		// conservatively request creation so a real gap is never ignored.
+		// syslog_partition_create() applies the same boundary guard before
+		// issuing DDL, so a covered day is still a safe no-op there.
+		return true;
+	}
+
+	$max_boundary = syslog_db_fetch_cell_prepared('SELECT MAX(CAST(partition_description AS SIGNED))
+		FROM `information_schema`.`partitions`
+		WHERE table_schema = ?
+		AND table_name = ?
+		AND partition_description IS NOT NULL
+		AND partition_description != ?',
+		[$syslogdb_default, $table, 'MAXVALUE']);
+
+	if (!is_numeric($max_boundary) || !is_numeric($target_boundary)) {
+		return true;
 	}
+
+	return (float) $target_boundary > (float) $max_boundary;
 }
 
 /**

tests/Unit/PartitionReorganizeCoverageTest.php

@@ -0,0 +1,253 @@
+<?php
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2004-2026 The Cacti Group                                 |
+ +-------------------------------------------------------------------------+
+ | Cacti: The Complete RRDTool-based Graphing Solution                     |
+ +-------------------------------------------------------------------------+
+*/
+
+/*
+ * Regression coverage for syslog_partition_create(): when the target
+ * partition name is absent but an existing partition boundary already
+ * reaches or exceeds the day being recovered, splitting dMaxValue would be
+ * rejected by MySQL with error 1493 ("VALUES LESS THAN value must be
+ * strictly increasing"). That left partition recovery wedged and parked
+ * writes in the dMaxValue safety partition (issue #434). The boundary guard
+ * must treat an already-covered day as a successful no-op and never emit the
+ * doomed REORGANIZE, while still creating genuinely new partitions.
+ */
+
+function partition_reorg_guard_setup($expression, $max_boundary, $to_days_value, &$alters, &$released) {
+	test_override('syslog_db_fetch_cell_prepared', function ($sql) use ($to_days_value, &$released) {
+		if (str_contains($sql, 'GET_LOCK')) {
+			return 1;
+		}
+
+		if (str_contains($sql, 'RELEASE_LOCK')) {
+			$released++;
+
+			return 1;
+		}
+
+		if (str_contains($sql, 'TO_DAYS')) {
+			return $to_days_value;
+		}
+
+		return 1;
+	});
+
+	test_override('syslog_db_fetch_row_prepared', function ($sql) use ($expression, $max_boundary) {
+		if (str_contains($sql, 'AS max_boundary')) {
+			return ['max_boundary' => $max_boundary, 'partition_expression' => $expression];
+		}
+
+		if (str_contains($sql, 'AS overflow_rows')) {
+			return ['overflow_rows' => 0, 'overflow_bytes' => 0];
+		}
+
+		if (str_contains($sql, 'SHOW CREATE TABLE')) {
+			return ['Create Table' => str_contains($expression, 'to_days')
+				? 'PARTITION BY RANGE (TO_DAYS(logtime))'
+				: 'PARTITION BY RANGE (UNIX_TIMESTAMP(logtime))'];
+		}
+
+		// The information_schema existence lookup resolves to "not found".
+		return [];
+	});
+
+	test_override('syslog_db_execute_prepared', function ($sql) use (&$alters) {
+		$alters[] = $sql;
+
+		return true;
+	});
+}
+
+it('skips the REORGANIZE when a TO_DAYS boundary already covers the day', function () {
+	syslog_load_plugin_source('includes/functions.php');
+	$GLOBALS['syslogdb_default'] = 'syslog_test';
+
+	$alters = [];
+	$released = 0;
+
+	// Target TO_DAYS boundary equals the highest existing boundary: covered.
+	partition_reorg_guard_setup('to_days(logtime)', 739568, 739568, $alters, $released);
+
+	$reason = 'prior deferral';
+
+	expect(syslog_partition_create('syslog', 1767398400, $reason))->toBeTrue();
+	expect($alters)->toBe([], 'No REORGANIZE may be issued for an already-covered day');
+	expect($reason)->toBe('');
+	expect($released)->toBe(1, 'The creation lock must be released');
+});
+
+it('skips the REORGANIZE when a UNIX_TIMESTAMP boundary already covers the day', function () {
+	syslog_load_plugin_source('includes/functions.php');
+	$GLOBALS['syslogdb_default'] = 'syslog_test';
+
+	$alters = [];
+	$released = 0;
+
+	// Highest existing epoch boundary is far beyond the target: covered.
+	partition_reorg_guard_setup('unix_timestamp(logtime)', 9999999999, 0, $alters, $released);
+
+	$reason = '';
+
+	expect(syslog_partition_create('syslog_removed', 1767398400, $reason))->toBeTrue();
+	expect($alters)->toBe([], 'No REORGANIZE may be issued for an already-covered day');
+	expect($released)->toBe(1, 'The creation lock must be released');
+});
+
+it('still creates a genuinely new partition when the boundary extends the range', function () {
+	syslog_load_plugin_source('includes/functions.php');
+	$GLOBALS['syslogdb_default'] = 'syslog_test';
+
+	$alters = [];
+	$released = 0;
+
+	// Highest existing boundary is below the target: a real new partition.
+	partition_reorg_guard_setup('to_days(logtime)', 739000, 739568, $alters, $released);
+
+	$reason = '';
+
+	expect(syslog_partition_create('syslog', 1767398400, $reason))->toBeTrue();
+	expect($alters)->toHaveCount(1);
+	expect($alters[0])->toContain('REORGANIZE PARTITION dMaxValue')->toContain('TO_DAYS');
+	expect($released)->toBe(1, 'The creation lock must be released');
+});
+
+/*
+ * End-to-end coverage for the reviewer scenario on issue #434: a legacy
+ * TO_DAYS table whose last concrete partition is named d20261111 but whose
+ * upper boundary already reaches 2026-11-15. Recovery on 2026-11-12 with the
+ * default three-day horizon must not waste its creation budget on the three
+ * already-covered days (Nov 12-14); it must create only the genuine gap
+ * (Nov 15), and a second recovery run must be a stable no-op.
+ */
+it('spends the recovery budget only on the genuine gap and stays stable across runs', function () {
+	syslog_load_plugin_source('includes/functions.php');
+	$GLOBALS['syslogdb_default'] = 'syslog_test';
+
+	// Monotonic stand-in for MySQL TO_DAYS(), sufficient for ordering.
+	$fake_to_days = static function ($date) {
+		return (int) floor(strtotime($date . ' 00:00:00 UTC') / 86400) + 719528;
+	};
+
+	// Existing partitions already cover every day whose data is below the
+	// 2026-11-15 boundary, i.e. through Nov 14. Nov 15 is the real gap.
+	$covering_boundary = $fake_to_days('2026-11-15');
+	$alters = [];
+
+	// Default recovery limit (3) and horizon (3): without the fix the three
+	// covered days would exhaust the budget before reaching Nov 15.
+	test_override('read_config_option', static fn() => '');
+
+	// syslog_partition_check() reads boundaries through _cell_prepared.
+	test_override('syslog_db_fetch_cell_prepared', function ($sql, $params = []) use (&$covering_boundary, $fake_to_days) {
+		if (str_contains($sql, 'GET_LOCK') || str_contains($sql, 'RELEASE_LOCK')) {
+			return 1;
+		}
+
+		if (str_contains($sql, 'partition_expression')) {
+			return 'to_days(`logtime`)';
+		}
+
+		if (str_contains($sql, 'MAX(CAST(partition_description')) {
+			return $covering_boundary;
+		}
+
+		if (str_contains($sql, 'TO_DAYS(?)')) {
+			return $fake_to_days($params[0]);
+		}
+
+		return '';
+	});
+
+	// syslog_partition_create() reads existence/overflow/boundary and the
+	// table DDL through _row_prepared.
+	test_override('syslog_db_fetch_row_prepared', function ($sql) use (&$covering_boundary) {
+		if (str_contains($sql, 'AS max_boundary')) {
+			return ['max_boundary' => $covering_boundary, 'partition_expression' => 'to_days(`logtime`)'];
+		}
+
+		if (str_contains($sql, 'AS overflow_rows')) {
+			return ['overflow_rows' => 0, 'overflow_bytes' => 0];
+		}
+
+		if (str_contains($sql, 'SHOW CREATE TABLE')) {
+			return ['Create Table' => 'CREATE TABLE `syslog` (`logtime` timestamp NOT NULL) PARTITION BY RANGE (TO_DAYS(`logtime`))'];
+		}
+
+		// The information_schema existence lookup resolves to "not found".
+		return [];
+	});
+
+	// A successful REORGANIZE extends coverage to the new partition boundary.
+	test_override('syslog_db_execute_prepared', function ($sql) use (&$alters, &$covering_boundary, $fake_to_days) {
+		if (preg_match("/PARTITION (d\\d{8}) VALUES LESS THAN \\(TO_DAYS\\('(\\d{4}-\\d{2}-\\d{2})'\\)\\)/", $sql, $m)) {
+			$alters[] = $m[1];
+			$covering_boundary = $fake_to_days($m[2]);
+		}
+
+		return true;
+	});
+
+	$base_time = gmmktime(12, 0, 0, 11, 12, 2026);
+
+	$run1 = syslog_partition_recover('syslog', $base_time, 3);
+
+	expect($run1['created'])->toBe(1, 'Only the genuine Nov 15 gap is created');
+	expect($run1['missing'])->toBe(0);
+	expect($run1['stop_reason'])->toBe('');
+	expect($run1['dmax_risk'])->toBeFalse();
+	expect($alters)->toBe(['d20261115'], 'Covered days must not emit a REORGANIZE');
+
+	$run2 = syslog_partition_recover('syslog', $base_time, 3);
+
+	expect($run2['created'])->toBe(0, 'Second run finds nothing missing');
+	expect($run2['missing'])->toBe(0);
+	expect($alters)->toBe(['d20261115'], 'Repeated recovery issues no further DDL');
+});
+
+it('resolves UNIX_TIMESTAMP coverage by boundary, not partition name', function () {
+	syslog_load_plugin_source('includes/functions.php');
+	$GLOBALS['syslogdb_default'] = 'syslog_test';
+
+	// Highest existing boundary is the next UTC midnight after 1767398400.
+	$max_boundary = (intdiv(1767398400, 86400) + 1) * 86400;
+
+	test_override('syslog_db_fetch_cell_prepared', function ($sql) use ($max_boundary) {
+		if (str_contains($sql, 'partition_expression')) {
+			return 'unix_timestamp(`logtime`)';
+		}
+
+		if (str_contains($sql, 'MAX(CAST(partition_description')) {
+			return $max_boundary;
+		}
+
+		return '';
+	});
+
+	// That day's boundary equals the highest existing boundary: covered.
+	expect(syslog_partition_check('syslog', 1767398400))->toBeFalse();
+
+	// A week later the boundary extends past the range: a real gap.
+	expect(syslog_partition_check('syslog', 1767398400 + 7 * 86400))->toBeTrue();
+});
+
+it('requests creation when the existing boundary metadata is not numeric', function () {
+	syslog_load_plugin_source('includes/functions.php');
+	$GLOBALS['syslogdb_default'] = 'syslog_test';
+
+	test_override('syslog_db_fetch_cell_prepared', function ($sql) {
+		if (str_contains($sql, 'partition_expression')) {
+			return 'unix_timestamp(`logtime`)';
+		}
+
+		// Unreadable max boundary forces the conservative "create" path,
+		// which syslog_partition_create()'s own guard keeps safe.
+		return 'MAXVALUE';
+	});
+
+	expect(syslog_partition_check('syslog', 1767398400))->toBeTrue();
+});