mirror of
https://github.com/Cacti/cacti.git
synced 2026-10-31 00:57:11 +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>
155 lines
7.5 KiB
PHP
155 lines
7.5 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 |
|
|
+-------------------------------------------------------------------------+
|
|
*/
|
|
|
|
/*
|
|
* Regression tests for two bugs in boost_process_poller_output() (lib/boost.php):
|
|
*
|
|
* 1. (#7519) The archive-table cleanup deleted every row for $local_data_id
|
|
* unconditionally, even rows with time >= $timestamp that belong to a
|
|
* still-open poll round and were therefore excluded from the RRD write a
|
|
* few lines above. Those rows are now carried forward into the live
|
|
* poller_output_boost table (INSERT IGNORE, time >= $timestamp) before
|
|
* the archive delete runs.
|
|
*
|
|
* The delete itself stays an unconditional-by-local_data_id delete on
|
|
* purpose, not narrowed to time < $timestamp: the archive-side SELECT
|
|
* that seeds boost_process_poller_output()'s own $temp_table on a later
|
|
* call for this local_data_id has no time filter (see the "INSERT INTO
|
|
* `{$temp_table}` SELECT * FROM `{$table}` WHERE local_data_id = ?" a few
|
|
* lines above in the same function) and is not INSERT IGNORE. Leaving a
|
|
* forwarded row behind in the archive table would let that later call
|
|
* pull the same row in twice -- once from the archive copy, once from
|
|
* the live table it was forwarded to -- and collide on
|
|
* poller_output_boost's own primary key (local_data_id, time, rrd_name)
|
|
* when both land in $temp_table. Deleting the whole slice once it is
|
|
* known to be safely represented elsewhere (RRD-written or forwarded)
|
|
* avoids that.
|
|
*
|
|
* 2. (#7523) $last_item was initialized once before the results loop and
|
|
* never reassigned, so the duplicate-record guard compared every row
|
|
* against the sentinel {-1, -1, ''} and could never fire. $last_item is
|
|
* now updated at the end of each processed iteration.
|
|
*/
|
|
|
|
$boostLibPath = CACTI_PATH_LIBRARY . '/boost.php';
|
|
|
|
if (!function_exists('boost_handoff_extract_function')) {
|
|
function boost_handoff_extract_function(string $contents, string $signature) : string {
|
|
$func_pos = strpos($contents, $signature);
|
|
|
|
if ($func_pos === false) {
|
|
return '';
|
|
}
|
|
|
|
$func_end = strpos($contents, "\nfunction ", $func_pos + 1);
|
|
|
|
if ($func_end === false) {
|
|
$func_end = strlen($contents);
|
|
}
|
|
|
|
return substr($contents, $func_pos, $func_end - $func_pos);
|
|
}
|
|
}
|
|
|
|
test('archive-table cleanup forwards still-open-round rows before deleting', function () use ($boostLibPath) {
|
|
$contents = file_get_contents($boostLibPath);
|
|
$func_body = boost_handoff_extract_function($contents, 'function boost_process_poller_output(');
|
|
|
|
expect($func_body)->not->toBe('');
|
|
|
|
// The forwarding INSERT must exist, target the live table, use IGNORE
|
|
// (defensive against a row already re-inserted there by a concurrent
|
|
// path), and copy exactly the still-open-round rows: time >= $timestamp.
|
|
$insert_pos = strpos($func_body, 'INSERT IGNORE INTO poller_output_boost');
|
|
expect($insert_pos)->not->toBeFalse('forwarding INSERT is missing');
|
|
|
|
$insert_segment = substr($func_body, $insert_pos, 200);
|
|
expect($insert_segment)->toContain('time >= FROM_UNIXTIME(?)');
|
|
|
|
// The archive DELETE (the second db_execute_prepared after the archive
|
|
// foreach) must run after the forwarding INSERT.
|
|
$delete_pos = strpos($func_body, 'DELETE FROM `$table`', $insert_pos);
|
|
expect($delete_pos)->not->toBeFalse('archive DELETE is missing');
|
|
expect($insert_pos)->toBeLessThan($delete_pos);
|
|
});
|
|
|
|
test('archive-table delete removes the whole local_data_id slice once forwarding has run, not just the closed rounds', function () use ($boostLibPath) {
|
|
$contents = file_get_contents($boostLibPath);
|
|
$func_body = boost_handoff_extract_function($contents, 'function boost_process_poller_output(');
|
|
|
|
$insert_pos = strpos($func_body, 'INSERT IGNORE INTO poller_output_boost');
|
|
$delete_pos = strpos($func_body, 'DELETE FROM `$table`', $insert_pos);
|
|
expect($delete_pos)->not->toBeFalse();
|
|
|
|
$delete_segment = substr($func_body, $delete_pos, 200);
|
|
|
|
// A forwarded row must not be left behind in the archive table: the
|
|
// archive-side temp-table seed a few lines above (INSERT IGNORE INTO
|
|
// `{$temp_table}` SELECT * FROM `{$table}` WHERE local_data_id = ?) has
|
|
// no time filter, so a leftover forwarded row
|
|
// would collide with its own live-table copy on a later call for the
|
|
// same local_data_id. The delete must therefore not be narrowed to
|
|
// "time < FROM_UNIXTIME(?)" -- it must clear everything for this
|
|
// local_data_id now that RRD-written and forwarded rows are both
|
|
// accounted for elsewhere.
|
|
expect($delete_segment)->not->toContain('time <');
|
|
expect($delete_segment)->toContain('WHERE local_data_id = ?');
|
|
});
|
|
|
|
test('the archive-side temp-table seed that forwarded rows must not collide with has no time filter', function () use ($boostLibPath) {
|
|
$contents = file_get_contents($boostLibPath);
|
|
$func_body = boost_handoff_extract_function($contents, 'function boost_process_poller_output(');
|
|
|
|
// This is the query the delete-scope test above is protecting against:
|
|
// confirms the hazard this fix accounts for is still actually present
|
|
// in the source, so the reasoning stays tied to real code rather than a
|
|
// stale assumption.
|
|
$seed_pos = strpos($func_body, 'INSERT IGNORE INTO `{$temp_table}`');
|
|
expect($seed_pos)->not->toBeFalse();
|
|
|
|
$seed_segment = substr($func_body, $seed_pos, 200);
|
|
expect($seed_segment)->toContain('FROM `{$table}`');
|
|
expect($seed_segment)->toContain('WHERE local_data_id = ?"');
|
|
expect($seed_segment)->not->toContain('time');
|
|
});
|
|
|
|
test('duplicate-record guard reassigns $last_item after processing each row', function () use ($boostLibPath) {
|
|
$contents = file_get_contents($boostLibPath);
|
|
$func_body = boost_handoff_extract_function($contents, 'function boost_process_poller_output(');
|
|
|
|
$guard_pos = strpos($func_body, "if (\$last_item['timestamp'] == \$item['timestamp']");
|
|
expect($guard_pos)->not->toBeFalse('duplicate guard not found');
|
|
|
|
// $last_item = $item; must appear somewhere after the guard, inside the
|
|
// same foreach loop over $results.
|
|
$reassign_pos = strpos($func_body, '$last_item = $item;', $guard_pos);
|
|
expect($reassign_pos)->not->toBeFalse('$last_item is never reassigned -- guard is still dead');
|
|
|
|
// The reassignment must be the last thing that happens to $last_item in
|
|
// the function; there must be no earlier "$last_item = " (other than the
|
|
// initial sentinel array literal) between the guard and this point that
|
|
// would make the placement redundant or misplaced.
|
|
$between = substr($func_body, $guard_pos, $reassign_pos - $guard_pos);
|
|
expect(substr_count($between, '$last_item = ['))->toBe(0);
|
|
});
|
|
|
|
test('$last_item sentinel starts at values no real row can match', function () use ($boostLibPath) {
|
|
$contents = file_get_contents($boostLibPath);
|
|
$func_body = boost_handoff_extract_function($contents, 'function boost_process_poller_output(');
|
|
|
|
expect($func_body)->toContain("'local_data_id' => -1");
|
|
expect($func_body)->toContain("'timestamp' => -1");
|
|
expect($func_body)->toContain("'rrd_name' => ''");
|
|
});
|