mirror of
https://github.com/Cacti/cacti.git
synced 2026-10-25 12:37:47 +09:00
* fix(boost): harden data handoffs and cache writes Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Define $updates_ok when there is nothing to flush PHPStan level 6 also flags the poller ownership check, which is the POLLER_ID constant case the file already annotates above. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Use a line comment to match the fixer Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Record the boost process columns in the audit schema Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: resolve includes through Cacti path constants Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Fix Boost handoff review concerns Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: seed settings in Boost database fixture Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: refresh SQL interpolation baseline Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: record Boost handoff hardening Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix: stop boost discarding a cycle and wedging a shard Three defects in the handoff hardening. A rejected ownership handoff returned false, which the poller reads as boost having consumed the data, so it skipped the RRD write. The poller_output rows were already deleted by then, so a stale poller_item copy on a remote poller silently lost the whole cycle. Return true instead and let the poller write directly, matching the database failure path. Any non-OK RRDtool response retained the shard, and the retry then replayed rows already written, which RRDtool rejects as an illegal update time. That failed again on every pass, so the archive tables accumulated and boost never drained. Treat an at-or-behind timestamp as applied, since the sample is in the RRD; genuine errors still retain the shard. The two new poller_output_boost_processes columns passed 'null' where db_install_update_table reads 'NULL', so NOT NULL was never emitted and upgraded installs diverged from cacti.sql. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix: spell the NULL column key so upgrades emit NOT NULL db_add_column() reads $column['NULL'] and PHP array keys are case sensitive, so the thirteen definitions in the 1.3.0 upgrade that passed a lowercase 'null' key had the flag discarded and created nullable columns. Fresh installs built from cacti.sql got NOT NULL, upgraded installs did not, and Cacti's own schema audit reports the difference. The same file already spells the key in upper case 144 times, and the wrapper's own docblock example uses the upper case form. Guard the class with a test over every upgrade file rather than only the one that was wrong. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: assert the NULL key plainly toContain() takes needles rather than a failure message. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): close remaining handoff review gaps Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): guard nullable graph output Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): keep in-flight completion rows through the 1.3.0 upgrade The truncate only existed so the run_child UNIQUE key could build over rows that predate run_id; deleting just those rows leaves a running boost its completion records. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * refactor(boost): drop the poller ownership gate Its boost_poller_on_demand() call sits behind the boost_redirect early return that poller.php forces on for every multi-collector install, and its recovery call turns a device deletion on main into a recovery that never brings the collector back online. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): bound atomic cache write allocations Guard upgrade source reads so failures report clearly, write graph cache data in fixed-size chunks, and exercise a multi-chunk payload. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): support Windows cache replacement Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(boost): expand handoff failure coverage Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(boost): support ANSI_QUOTES upgrade checks Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): stop staging after chunk failure Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(recovery): use numeric poller status Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(sql): support ANSI_QUOTES in recovery paths Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): preserve cache on replacement failure Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): retain readable cache permissions Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): repair completion schema at runtime Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(recovery): bound timestamp batch fetches Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): satisfy runtime schema analysis Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(recovery): restore status after transfer failure Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(boost): rely on unit bootstrap helpers Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): refresh generated cache secret Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): normalize cache path separators Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(boost): fail clearly on unreadable fixtures * fix(boost): name ignored duplicate sample keys The staging warning only said rows were dropped, so a retry could not be told from a colliding writer. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Lack of pre-commit hook execution allowed lint failure --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net>
328 lines
14 KiB
PHP
328 lines
14 KiB
PHP
<?php
|
|
/*
|
|
+-------------------------------------------------------------------------+
|
|
| Copyright (C) 2004-2026 The Cacti Group |
|
|
| |
|
|
| This program is free software; you can redistribute it and/or |
|
|
| modify it under the terms of the GNU General Public License |
|
|
| as published by the Free Software Foundation; either version 2 |
|
|
| of the License, or (at your option) any later version. |
|
|
+-------------------------------------------------------------------------+
|
|
| Cacti: The Complete RRDtool-based Graphing Solution |
|
|
+-------------------------------------------------------------------------+
|
|
*/
|
|
|
|
/*
|
|
* Data hand-off proof for #7519: an archive-table row that belongs to a
|
|
* still-open poll round (time >= the safety cutoff) must survive
|
|
* boost_process_poller_output()'s archive-table cleanup by moving into the
|
|
* live poller_output_boost table, not be deleted out of an archive table
|
|
* that gets dropped wholesale once the run finishes.
|
|
*
|
|
* This executes the exact forward-then-narrow-delete SQL text shipped in
|
|
* lib/boost.php -- extracted from the live source at run time, not retyped
|
|
* -- through the real db_execute_prepared()/db_fetch_* call path in
|
|
* lib/database.php, against a real MySQL-compatible server. If the fix is
|
|
* ever reverted, this test starts running the reverted (unfiltered) SQL and
|
|
* fails on real, provable row loss instead of a mock agreeing with itself.
|
|
*
|
|
* Requires a MySQL/MariaDB instance. Set CACTI_TEST_MYSQL_DSN /
|
|
* CACTI_TEST_MYSQL_USER / CACTI_TEST_MYSQL_PASS to point at one; defaults to
|
|
* a local `docker run mariadb` instance on 127.0.0.1:33061. Skips (does not
|
|
* fail) if unreachable.
|
|
*/
|
|
|
|
require_once __DIR__ . '/../Helpers/UnitStubs.php';
|
|
require_once dirname(__DIR__, 2) . '/lib/database.php';
|
|
|
|
$boostLibPath = dirname(__DIR__, 2) . '/lib/boost.php';
|
|
|
|
if (!function_exists('boost_handoff_extract_sql')) {
|
|
function boost_handoff_extract_sql(string $contents, string $start_needle, string $end_needle) : string {
|
|
$start = strpos($contents, $start_needle);
|
|
|
|
if ($start === false) {
|
|
throw new RuntimeException("start needle not found: $start_needle");
|
|
}
|
|
|
|
$end = strpos($contents, $end_needle, $start);
|
|
|
|
if ($end === false) {
|
|
throw new RuntimeException("end needle not found: $end_needle");
|
|
}
|
|
|
|
return substr($contents, $start, $end - $start + strlen($end_needle));
|
|
}
|
|
}
|
|
|
|
if (!function_exists('boost_handoff_connect_default_session')) {
|
|
function boost_handoff_connect_default_session(PDO $pdo) : void {
|
|
global $database_sessions, $database_hostname, $database_port, $database_default;
|
|
|
|
$database_hostname = 'unit-test-host';
|
|
$database_port = 3306;
|
|
$database_default = 'cacti_boost_handoff_test';
|
|
|
|
$database_sessions["$database_hostname:$database_port:$database_default"] = $pdo;
|
|
}
|
|
}
|
|
|
|
beforeEach(function () {
|
|
global $database_sessions, $database_hostname, $database_port, $database_default;
|
|
|
|
$this->db_globals = [$database_sessions, $database_hostname, $database_port, $database_default];
|
|
|
|
$dsn = getenv('CACTI_TEST_MYSQL_DSN') ?: 'mysql:host=127.0.0.1;port=33061;dbname=cacti_test';
|
|
$user = getenv('CACTI_TEST_MYSQL_USER') ?: 'root';
|
|
$pass = getenv('CACTI_TEST_MYSQL_PASS') ?: 'root';
|
|
|
|
try {
|
|
$this->pdo = new PDO($dsn, $user, $pass, [PDO::ATTR_ERRMODE => PDO::ERRMODE_SILENT, PDO::ATTR_TIMEOUT => 3]);
|
|
} catch (\PDOException $e) {
|
|
$this->pdo = null;
|
|
test()->markTestSkipped('No MySQL/MariaDB test instance reachable at ' . $dsn . ': ' . $e->getMessage());
|
|
|
|
return;
|
|
}
|
|
|
|
boost_handoff_connect_default_session($this->pdo);
|
|
|
|
$this->pdo->exec('DROP TABLE IF EXISTS poller_output_boost');
|
|
$this->pdo->exec('DROP TABLE IF EXISTS poller_output_boost_arch_handoff_test');
|
|
|
|
$this->pdo->exec("CREATE TABLE poller_output_boost (
|
|
local_data_id int(10) unsigned NOT NULL default 0,
|
|
rrd_name varchar(19) NOT NULL default '',
|
|
time timestamp NOT NULL default '2000-01-01 00:00:00',
|
|
output varchar(512) NOT NULL,
|
|
last_updated timestamp NOT NULL default current_timestamp,
|
|
PRIMARY KEY (local_data_id, time, rrd_name)
|
|
) ENGINE=InnoDB");
|
|
|
|
$this->pdo->exec('CREATE TABLE poller_output_boost_arch_handoff_test LIKE poller_output_boost');
|
|
});
|
|
|
|
afterEach(function () {
|
|
global $database_sessions, $database_hostname, $database_port, $database_default;
|
|
|
|
if (isset($this->pdo) && $this->pdo instanceof PDO) {
|
|
$this->pdo->exec('DROP TABLE IF EXISTS poller_output_boost');
|
|
$this->pdo->exec('DROP TABLE IF EXISTS poller_output_boost_arch_handoff_test');
|
|
}
|
|
|
|
// Put the default db_* connection back so the test handle does not answer
|
|
// every later read_config_option() in the run.
|
|
[$database_sessions, $database_hostname, $database_port, $database_default] = $this->db_globals;
|
|
});
|
|
|
|
test('a row from a still-open poll round survives the archive cleanup by moving to the live table', function () use ($boostLibPath) {
|
|
if (!isset($this->pdo) || !$this->pdo instanceof PDO) {
|
|
return;
|
|
}
|
|
|
|
$contents = file_get_contents($boostLibPath);
|
|
|
|
$insert_sql = str_replace('$table', 'poller_output_boost_arch_handoff_test',
|
|
boost_handoff_extract_sql($contents, "INSERT IGNORE INTO poller_output_boost\n\t\t\t\tSELECT *", 'AND time >= FROM_UNIXTIME(?)'));
|
|
|
|
$delete_sql = str_replace('$table', 'poller_output_boost_arch_handoff_test',
|
|
rtrim(boost_handoff_extract_sql($contents, 'DELETE FROM `$table`', 'WHERE local_data_id = ?'), '"'));
|
|
|
|
$local_data_id = 42;
|
|
$cutoff = 1_700_000_000; // the in-progress-round safety cutoff ($timestamp)
|
|
|
|
// Seed the archive table like a mid-rotation poller_boost.php run would:
|
|
// two rows already belong to closed poll rounds (time < cutoff), one row
|
|
// landed after the still-open round started (time >= cutoff).
|
|
db_execute_prepared('INSERT INTO poller_output_boost_arch_handoff_test
|
|
(local_data_id, rrd_name, time, output) VALUES (?, ?, FROM_UNIXTIME(?), ?)',
|
|
[$local_data_id, 'traffic_in', $cutoff - 300, '123']);
|
|
|
|
db_execute_prepared('INSERT INTO poller_output_boost_arch_handoff_test
|
|
(local_data_id, rrd_name, time, output) VALUES (?, ?, FROM_UNIXTIME(?), ?)',
|
|
[$local_data_id, 'traffic_out', $cutoff - 300, '456']);
|
|
|
|
db_execute_prepared('INSERT INTO poller_output_boost_arch_handoff_test
|
|
(local_data_id, rrd_name, time, output) VALUES (?, ?, FROM_UNIXTIME(?), ?)',
|
|
[$local_data_id, 'traffic_in', $cutoff + 60, '999']); // still-open round
|
|
|
|
// Run the fixed sequence exactly as boost_process_poller_output() does:
|
|
// forward the still-open-round row, then delete the whole slice for
|
|
// this local_data_id now that every row is accounted for elsewhere.
|
|
$insert_ok = db_execute_prepared($insert_sql, [$local_data_id, $cutoff], false);
|
|
$delete_ok = db_execute_prepared($delete_sql, [$local_data_id], false);
|
|
|
|
expect($insert_ok)->not->toBeFalse();
|
|
expect($delete_ok)->not->toBeFalse();
|
|
|
|
// The archive table must be fully cleared for this local_data_id -- no
|
|
// leftover forwarded row that could collide with itself on a later run.
|
|
$remaining_in_archive = db_fetch_cell('SELECT COUNT(*) FROM poller_output_boost_arch_handoff_test');
|
|
expect((int) $remaining_in_archive)->toBe(0);
|
|
|
|
// ...and the still-open-round row is provably present in the live
|
|
// table, not lost.
|
|
$live_rows = db_fetch_assoc('SELECT rrd_name, output, UNIX_TIMESTAMP(time) AS ts FROM poller_output_boost');
|
|
expect($live_rows)->toHaveCount(1);
|
|
expect($live_rows[0]['rrd_name'])->toBe('traffic_in');
|
|
expect($live_rows[0]['output'])->toBe('999');
|
|
expect((int) $live_rows[0]['ts'])->toBe($cutoff + 60);
|
|
});
|
|
|
|
test('closed-round rows are removed from the archive table and not duplicated into the live table', function () use ($boostLibPath) {
|
|
if (!isset($this->pdo) || !$this->pdo instanceof PDO) {
|
|
return;
|
|
}
|
|
|
|
$contents = file_get_contents($boostLibPath);
|
|
|
|
$insert_sql = str_replace('$table', 'poller_output_boost_arch_handoff_test',
|
|
boost_handoff_extract_sql($contents, "INSERT IGNORE INTO poller_output_boost\n\t\t\t\tSELECT *", 'AND time >= FROM_UNIXTIME(?)'));
|
|
|
|
$delete_sql = str_replace('$table', 'poller_output_boost_arch_handoff_test',
|
|
rtrim(boost_handoff_extract_sql($contents, 'DELETE FROM `$table`', 'WHERE local_data_id = ?'), '"'));
|
|
|
|
$local_data_id = 9;
|
|
$cutoff = 1_700_000_000;
|
|
|
|
db_execute_prepared('INSERT INTO poller_output_boost_arch_handoff_test
|
|
(local_data_id, rrd_name, time, output) VALUES (?, ?, FROM_UNIXTIME(?), ?)',
|
|
[$local_data_id, 'ping', $cutoff - 60, '5']);
|
|
|
|
db_execute_prepared($insert_sql, [$local_data_id, $cutoff], false);
|
|
db_execute_prepared($delete_sql, [$local_data_id], false);
|
|
|
|
expect((int) db_fetch_cell('SELECT COUNT(*) FROM poller_output_boost_arch_handoff_test'))->toBe(0);
|
|
// This row's time was < cutoff, so it was already written to RRD by the
|
|
// caller before the archive cleanup ran; it must not also appear in the
|
|
// live table as a side effect of the cleanup.
|
|
expect((int) db_fetch_cell('SELECT COUNT(*) FROM poller_output_boost'))->toBe(0);
|
|
});
|
|
|
|
test('the temporary merge coalesces duplicate archive and live keys', function () use ($boostLibPath) {
|
|
if (!isset($this->pdo) || !$this->pdo instanceof PDO) {
|
|
return;
|
|
}
|
|
|
|
// Run both temp-table seed statements verbatim. They deliberately use
|
|
// INSERT IGNORE so a retry row present in both the archive and live tables
|
|
// is processed once instead of aborting the handoff on a primary-key clash.
|
|
$contents = file_get_contents($boostLibPath);
|
|
|
|
$first_seed_pos = strpos($contents, 'INSERT IGNORE INTO `{$temp_table}`');
|
|
$second_seed_pos = strpos($contents, 'INSERT IGNORE INTO `{$temp_table}`', $first_seed_pos + 1);
|
|
|
|
expect($first_seed_pos)->not->toBeFalse();
|
|
expect($second_seed_pos)->not->toBeFalse();
|
|
|
|
$seed_sql = rtrim(str_replace(['{$temp_table}', '{$table}'], ['poller_output_boost_temp_test', 'poller_output_boost_arch_handoff_test'],
|
|
boost_handoff_extract_sql(substr($contents, $first_seed_pos, $second_seed_pos - $first_seed_pos), 'INSERT IGNORE INTO `{$temp_table}`', 'WHERE local_data_id = ?"')), '"');
|
|
|
|
$live_seed_sql = rtrim(str_replace('{$temp_table}', 'poller_output_boost_temp_test',
|
|
boost_handoff_extract_sql(substr($contents, $second_seed_pos), 'INSERT IGNORE INTO `{$temp_table}`', 'AND time < FROM_UNIXTIME(?)"')), '"');
|
|
|
|
$this->pdo->exec('CREATE TEMPORARY TABLE poller_output_boost_temp_test LIKE poller_output_boost');
|
|
|
|
$local_data_id = 55;
|
|
$row_time = 1_700_000_060;
|
|
|
|
// The row exists in BOTH places: the archive table (a hypothetical
|
|
// leftover from a narrowed delete) and the live table (where it was
|
|
// forwarded to).
|
|
db_execute_prepared('INSERT INTO poller_output_boost_arch_handoff_test
|
|
(local_data_id, rrd_name, time, output) VALUES (?, ?, FROM_UNIXTIME(?), ?)',
|
|
[$local_data_id, 'ping', $row_time, 'from-archive']);
|
|
|
|
db_execute_prepared('INSERT INTO poller_output_boost
|
|
(local_data_id, rrd_name, time, output) VALUES (?, ?, FROM_UNIXTIME(?), ?)',
|
|
[$local_data_id, 'ping', $row_time, 'from-live']);
|
|
|
|
db_execute_prepared($seed_sql, [$local_data_id], false);
|
|
|
|
$next_cutoff = $row_time + 3600;
|
|
$merged = db_execute_prepared($live_seed_sql, [$local_data_id, $next_cutoff], false);
|
|
|
|
expect($merged)->not->toBeFalse();
|
|
|
|
$rows = db_fetch_assoc('SELECT output FROM poller_output_boost_temp_test');
|
|
expect($rows)->toHaveCount(1)
|
|
->and($rows[0]['output'])->toBe('from-archive');
|
|
|
|
$this->pdo->exec('DROP TEMPORARY TABLE IF EXISTS poller_output_boost_temp_test');
|
|
});
|
|
|
|
test('forwarding a row already present in the live table does not raise a PK collision (INSERT IGNORE)', function () use ($boostLibPath) {
|
|
if (!isset($this->pdo) || !$this->pdo instanceof PDO) {
|
|
return;
|
|
}
|
|
|
|
$contents = file_get_contents($boostLibPath);
|
|
|
|
$insert_sql = str_replace('$table', 'poller_output_boost_arch_handoff_test',
|
|
boost_handoff_extract_sql($contents, "INSERT IGNORE INTO poller_output_boost\n\t\t\t\tSELECT *", 'AND time >= FROM_UNIXTIME(?)'));
|
|
|
|
$local_data_id = 7;
|
|
$cutoff = 1_700_000_000;
|
|
|
|
// The exact same (local_data_id, time, rrd_name) key already exists in
|
|
// the live table -- e.g. re-inserted by a concurrent poller path -- and
|
|
// also sits in the archive table being forwarded.
|
|
db_execute_prepared('INSERT INTO poller_output_boost
|
|
(local_data_id, rrd_name, time, output) VALUES (?, ?, FROM_UNIXTIME(?), ?)',
|
|
[$local_data_id, 'ping', $cutoff + 10, 'live-value']);
|
|
|
|
db_execute_prepared('INSERT INTO poller_output_boost_arch_handoff_test
|
|
(local_data_id, rrd_name, time, output) VALUES (?, ?, FROM_UNIXTIME(?), ?)',
|
|
[$local_data_id, 'ping', $cutoff + 10, 'archive-value']);
|
|
|
|
$insert_ok = db_execute_prepared($insert_sql, [$local_data_id, $cutoff], false);
|
|
|
|
// INSERT IGNORE must not error on the PK collision, and must not clobber
|
|
// the row the live table already had.
|
|
expect($insert_ok)->not->toBeFalse();
|
|
|
|
$outputs = db_fetch_assoc("SELECT output FROM poller_output_boost WHERE local_data_id = $local_data_id");
|
|
expect($outputs)->toHaveCount(1);
|
|
expect($outputs[0]['output'])->toBe('live-value');
|
|
});
|
|
|
|
test('a genuine duplicate (timestamp, rrd_name) pair reaching $results is skipped by the reassigned $last_item guard', function () {
|
|
// #7523: this reproduces the scenario the issue describes as the reason
|
|
// the guard needed to work -- a stale leftover archive table and the
|
|
// current run's rows overlapping on the same (local_data_id, timestamp,
|
|
// rrd_name) key, both landing in $results because the forwarding fix in
|
|
// #7519 can legitimately place a row that already exists elsewhere in
|
|
// the query's result set for the same run.
|
|
//
|
|
// This exercises the *fixed* loop shape directly (not a mock of it): the
|
|
// same duplicate-detection condition and $last_item reassignment now
|
|
// shipped in boost_process_poller_output(), run against a synthetic
|
|
// $results array with an adjacent duplicate pair.
|
|
$results = [
|
|
['local_data_id' => 1, 'timestamp' => 1000, 'rrd_name' => 'traffic_in', 'output' => '10'],
|
|
['local_data_id' => 1, 'timestamp' => 1000, 'rrd_name' => 'traffic_in', 'output' => '10'], // duplicate
|
|
['local_data_id' => 1, 'timestamp' => 1060, 'rrd_name' => 'traffic_in', 'output' => '20'],
|
|
];
|
|
|
|
$last_item = ['local_data_id' => -1, 'timestamp' => -1, 'rrd_name' => ''];
|
|
$processed = [];
|
|
$skipped = 0;
|
|
|
|
foreach ($results as $item) {
|
|
if ($last_item['timestamp'] == $item['timestamp'] && $last_item['rrd_name'] == $item['rrd_name']) {
|
|
$skipped++;
|
|
|
|
continue;
|
|
}
|
|
|
|
$processed[] = $item;
|
|
|
|
$last_item = $item;
|
|
}
|
|
|
|
expect($skipped)->toBe(1);
|
|
expect($processed)->toHaveCount(2);
|
|
expect($processed[0]['output'])->toBe('10');
|
|
expect($processed[1]['output'])->toBe('20');
|
|
});
|