bugfix: scope partition maintenance to target table and lock create path

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 --- develop ---
 
+* issue#254: Scope partition maintenance to the target table and serialize partition creation
 * issue: Making changes to support Cacti 1.3
 * issue: Don't use MyISAM for non-analytical tables
 * issue: The install advisor for Syslog was broken in current Cacti releases

functions.php

@@ -186,32 +186,56 @@ function syslog_partition_manage() {
 	return $syslog_deleted;
 }
 
+/**
+ * Validate tables that support partition maintenance.
+ */
+function syslog_partition_table_allowed($table) {
+	return in_array($table, array('syslog', 'syslog_removed'), true);
+}
+
 /**
  * This function will create a new partition for the specified table.
  */
 function syslog_partition_create($table) {
 	global $syslogdb_default;
 
+	if (!syslog_partition_table_allowed($table)) {
+		return;
+	}
+
+	$lock_name = $syslogdb_default . '.syslog_partition_create.' . $table;
+	$locked    = syslog_db_fetch_cell_prepared('SELECT GET_LOCK(?, 10)', array($lock_name));
+
+	if ((int)$locked !== 1) {
+		cacti_log("SYSLOG: Unable to acquire partition create lock for '$table'", false, 'SYSTEM');
+		return;
+	}
+
 	/* determine the format of the table name */
 	$time    = time();
 	$cformat = 'd' . date('Ymd', $time);
 	$lnow    = date('Y-m-d', $time+86400);
 
-	$exists = syslog_db_fetch_row("SELECT *
-		FROM `information_schema`.`partitions`
-        WHERE table_schema='" . $syslogdb_default . "'
-		AND partition_name='" . $cformat . "'
-		AND table_name='syslog'
-        ORDER BY partition_ordinal_position");
+	try {
+		$exists = syslog_db_fetch_row_prepared("SELECT *
+			FROM `information_schema`.`partitions`
+			WHERE table_schema = ?
+			AND partition_name = ?
+			AND table_name = ?
+			ORDER BY partition_ordinal_position",
+			array($syslogdb_default, $cformat, $table));
 
-	if (!cacti_sizeof($exists)) {
-		cacti_log("SYSLOG: Creating new partition '$cformat'", false, 'SYSTEM');
+		if (!cacti_sizeof($exists)) {
+			cacti_log("SYSLOG: Creating new partition '$cformat'", false, 'SYSTEM');
 
-		syslog_debug("Creating new partition '$cformat'");
+			syslog_debug("Creating new partition '$cformat'");
 
-		syslog_db_execute("ALTER TABLE `" . $syslogdb_default . "`.`$table` REORGANIZE PARTITION dMaxValue INTO (
-			PARTITION $cformat VALUES LESS THAN (TO_DAYS('$lnow')),
-			PARTITION dMaxValue VALUES LESS THAN MAXVALUE)");
+			syslog_db_execute("ALTER TABLE `" . $syslogdb_default . "`.`$table` REORGANIZE PARTITION dMaxValue INTO (
+				PARTITION $cformat VALUES LESS THAN (TO_DAYS('$lnow')),
+				PARTITION dMaxValue VALUES LESS THAN MAXVALUE)");
+		}
+	} finally {
+		syslog_db_fetch_cell_prepared('SELECT RELEASE_LOCK(?)', array($lock_name));
 	}
 }
 
@@ -221,11 +245,16 @@ function syslog_partition_create($table) {
 function syslog_partition_remove($table) {
 	global $syslogdb_default;
 
+	if (!syslog_partition_table_allowed($table)) {
+		return 0;
+	}
+
 	$syslog_deleted = 0;
-	$number_of_partitions = syslog_db_fetch_assoc("SELECT *
+	$number_of_partitions = syslog_db_fetch_assoc_prepared("SELECT *
 		FROM `information_schema`.`partitions`
-		WHERE table_schema='" . $syslogdb_default . "' AND table_name='syslog'
-		ORDER BY partition_ordinal_position");
+		WHERE table_schema = ? AND table_name = ?
+		ORDER BY partition_ordinal_position",
+		array($syslogdb_default, $table));
 
 	$days = read_config_option('syslog_retention');
 
@@ -257,16 +286,21 @@ function syslog_partition_remove($table) {
 function syslog_partition_check($table) {
 	global $syslogdb_default;
 
+	if (!syslog_partition_table_allowed($table)) {
+		return false;
+	}
+
 	if (defined('SYSLOG_CONFIG')) {
 		include(SYSLOG_CONFIG);
 	}
 
 	/* find date of last partition */
-	$last_part = syslog_db_fetch_cell("SELECT PARTITION_NAME
+	$last_part = syslog_db_fetch_cell_prepared("SELECT PARTITION_NAME
 		FROM `information_schema`.`partitions`
-		WHERE table_schema='" . $syslogdb_default . "' AND table_name='syslog'
+		WHERE table_schema = ? AND table_name = ?
 		ORDER BY partition_ordinal_position DESC
-		LIMIT 1,1;");
+		LIMIT 1,1",
+		array($syslogdb_default, $table));
 
 	$lformat   = str_replace('d', '', $last_part);
 	$cformat   = date('Ymd');
@@ -2421,4 +2455,3 @@ function alert_replace_variables($alert, $results, $hostname = '') {
 
 	return $command;
 }
-

tests/regression/issue254_partition_table_locking_test.php

@@ -0,0 +1,32 @@
+<?php
+
+$functions = file_get_contents(dirname(__DIR__, 2) . '/functions.php');
+
+if ($functions === false) {
+	fwrite(STDERR, "Failed to load functions.php\n");
+	exit(1);
+}
+
+/* All three information_schema queries must be prepared statements
+ * scoped to the requested table via a placeholder. */
+if (preg_match_all('/syslog_db_fetch_(?:row|assoc|cell)_prepared\s*\(/', $functions) < 3) {
+	fwrite(STDERR, "Partition queries are not consistently scoped to the requested table.\n");
+	exit(1);
+}
+
+if (!preg_match('/SELECT\s+GET_LOCK\s*\(\s*\?\s*,\s*10\s*\)/', $functions)) {
+	fwrite(STDERR, "Partition create lock acquisition is missing.\n");
+	exit(1);
+}
+
+if (!preg_match('/SELECT\s+RELEASE_LOCK\s*\(\s*\?\s*\)/', $functions)) {
+	fwrite(STDERR, "Partition create lock release is missing.\n");
+	exit(1);
+}
+
+if (!preg_match('/function\s+syslog_partition_table_allowed\s*\(/', $functions)) {
+	fwrite(STDERR, "Partition table validation helper is missing.\n");
+	exit(1);
+}
+
+echo "issue254_partition_table_locking_test passed\n";