mirror of
https://github.com/Cacti/cacti.git
synced 2026-10-24 03:57:32 +09:00
* fix(boost): forward still-open-round archive rows instead of dropping them boost_process_poller_output() correctly excluded rows from an in-progress poll round when building the RRD write, but the archive-table cleanup right after deleted every row for the local_data_id anyway, with no time filter. Those rows now get INSERT IGNORE'd into the live poller_output_boost table before the archive table is cleared, so a still-open round's data survives. Also fixes the duplicate-record guard in the same function: $last_item was initialized once before the results loop and never reassigned, so the comparison against it could never be true. It is now updated after each processed row. Fixes #7519 Fixes #7523 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): gate archive-table drop on shard completeness poller_boost.php decided whether to drop a run's archive tables using SELECT SUM(status) FROM poller_output_boost_processes, an aggregate over whatever completion rows landed. If one child crashed before writing its completion row, the sum from the surviving shards was still positive and every archive table for the run got dropped, including the crashed shard's unprocessed data. The drain loop already computes boost_completed_children() vs $expected_children to detect a crashed child; the drop now uses that same completeness check instead, and leaves the archive tables in place with a warning when it is not met. Fixes #7520 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(dsstats,rrdcheck): apply Boost's startup barrier to launch loops dsstats_launch_children()/rrdcheck_launch_children() fork children via exec_background() (non-blocking) and the callers then wait with a bare while ($running = ..._processes_running($type)) { sleep(2); }. If the parent evaluates that before any child has registered in the processes table, $running reads 0 and the loop body never runs, treating "not started yet" as "already done" -- the same race Boost's boost_all_children_registered() barrier was added to close. dsstats_launch_children()/rrdcheck_launch_children() now return the launched count, and dsstats_wait_for_children()/ rrdcheck_wait_for_children() wait for the running count to reach it (bounded by a deadline) before trusting the drain loop's running-count poll. Fixes #7526 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * refactor(poller): consolidate poller_item deletes behind a shared helper Extract poller_item_delete_for_data_source() and poller_item_delete_for_host() in lib/poller.php and migrate the existing local+remote delete pairs in lib/api_device.php and lib/api_data_source.php onto them, so the local-delete-plus-remote-mirror pattern lives in one place instead of eight. Refs #7535 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(poller): mirror poller_item deletes to the remote poller on handoff api_data_source_change_host(), data_query_remove_disabled_items(), and data_source_to_data_template() each moved or removed a data source's poller_item row locally but never on the remote Data Collector that still owned it, so that Collector kept polling an item it no longer should. All three now resolve the owning remote poller before the local change and purge the row there too via the shared helper. Fixes #7521 Fixes #7524 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(poller): correct poller_item join semantics and cron reset in cmd.php poller_item and host share ~13 identically-named snmp_*/hostname columns; SELECT * across the join let PDO FETCH_ASSOC silently collapse them to host's live values instead of poller_item's cached copies. Select pi.* plus explicitly aliased host/site disabled columns instead. The now already-fetched $poller_items also makes the separate script-server-call COUNT(*) re-join redundant, replaced with an in-PHP count. Also apply the same cron_interval-aware rrd_next_step reset used by the ranged branch to the --allhost branch, which previously ignored it. Fixes #7522, #7525, #7528. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(poller): thread known poller_id through update_poller_cache() callers update_poller_cache() re-derived poller_id via a host/data_local JOIN on every call, even when the caller already had it: repopulate_poller_cache() and push_out_data_input_method() both already select h.poller_id and discarded it, push_out_host() has the host row cached per data source, and api_device_enable_devices()/update_poller_cache_from_query() compute it once outside their loops. Add an optional $poller_id argument (and honor an already-present 'poller_id' array key) so these callers skip the redundant query. Fixes #7527. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(poller): scope global aggregate scans and make process-leveling sargable Three poller.php queries (total_snmp_ports and the mutually-exclusive active_profiles pair) had no poller_id filter, unlike the rest of the file's central-only blocks, so every remote poller redundantly recomputed and overwrote the same global config options; the snmp_port scan is also an unindexed full table scan. Gate them behind poller_id == 1, with remote pollers reading back the value the central poller already stored. Also rewrite the process-leveling $sql_where's rrd_next_step predicate from the non-sargable `rrd_next_step - $poller_interval <= 0` to the algebraically equivalent `rrd_next_step <= $poller_interval`, restoring use of the poller_id_rrd_next_step index. Fixes #7529, #7530. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(poller): batch update_data_query_cache()'s poller cache writes Each changed data source called update_poller_cache($data_source, true), which flushes immediately via poller_update_poller_cache_from_buffer() -- its own present=0 UPDATE, INSERT ON DUPLICATE KEY UPDATE, and stale-row DELETE -- on every iteration. Re-indexing a host where every port changed issued up to 3x as many statements as ports instead of one batched flush. Accumulate changed rows across the loop and call update_poller_cache() with commit=false, then flush once via poller_update_poller_cache_from_buffer() after the loop, matching the batched pattern already used by push_out_data_input_method(). Fixes #7531. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): add USING BTREE to poller_output_boost_local_data_ids indexes MEMORY-engine indexes default to HASH unless USING BTREE is specified, unlike the sibling poller_output/poller_output_boost tables in the same schema file. boost_output_rrd_data()'s ORDER BY/LIMIT query and boost_process_local_data_ids()'s ranged DELETE both need to scan local_data_id in order, which a HASH index can't serve, forcing a filesort on every boost worker pass. Verified against a real MariaDB 11 instance: EXPLAIN on the ORDER BY query shows "Using filesort" with the old HASH schema and no filesort with USING BTREE, using the primary key for ordering instead. Also aligns boost_prepare_process_table()'s dead CREATE TABLE IF NOT EXISTS fallback with the corrected schema, and adds an upgrade step that rebuilds both indexes as BTREE on existing installs (checked via information_schema.STATISTICS, since db_install_add_key() only compares index columns, not algorithm, and would otherwise skip an existing HASH index with the right column list). Closes #7532 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): skip the UNION ALL wrapper for the single-archive-table case boost_process_local_data_ids() always wrapped its flush query in "SELECT * FROM (... UNION ALL ...) t ORDER BY ...", even when there was only one archive table -- the steady-state case, since old archive tables are dropped at the end of each cycle. UNION ALL results wrapped in a derived table are one of the documented cases MySQL/MariaDB's derived-table merging doesn't apply to, so every pass paid for a filesort over an indexless derived table regardless of how well-ordered the underlying archive table's rows already were. Extracts the per-table SELECT into boost_archive_select_sql() and uses it directly, without the wrapper, when there's exactly one archive table. The UNION ALL/derived-table shape is kept as the fallback for the genuinely rare case where a prior crashed run left more than one archive table behind. Verified against a real MariaDB 11 instance and via a SQLite-backed integration test that both shapes return correctly filtered and sorted rows. Closes #7533 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): batch-prefetch RRD field name lookups instead of per-row queries Two N+1 patterns in the Boost hot paths: boost_process_local_data_ids() (poller_boost.php) re-ran the unused_data_source_names/nt_rrd_field_names lookups on every local_data_id boundary in the sorted flush result set. process_poller_output() (lib/poller.php) had the equivalent lookup, but without even a boundary check -- it re-ran per matching row whenever the output contained a ':', including consecutive rows for the same data source. Adds poller_prefetch_rrd_field_names() (lib/poller.php, shared by both callers since poller_boost.php already requires lib/poller.php) to batch-fetch these mappings once per invocation via IN()-batched queries keyed by the full set of local_data_ids in the batch, chunked at 1000 ids to match the array_chunk(..., 1000) convention used elsewhere (lib/api_data_source.php, lib/api_graph.php). Both call sites now look up the prefetched maps in-memory inside their loops. While consolidating the lookups, fixes a latent bug the old process_poller_output() code had in its non-templated fallback branch: a query referencing gti.task_item_id without joining graph_templates_item, which throws an unknown-column error whenever that path is hit. The batched query always joins correctly. Integration test proves both correctness (against a real SQLite-backed schema) and the query-count reduction: 5 local_data_ids that would previously cost up to 10 queries for these two lookups now cost exactly 3, regardless of how many ids are in the batch. Closes #7534 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * refactor(boost): consolidate poller_output_boost inserts into one helper The INSERT INTO poller_output_boost (...) VALUES ... statement was hand-written four times: three in cmd.php (behind the same boost_redirect == 'on' && boost_rrd_update_enable == 'on' guard) using INSERT IGNORE, and once in boost_poller_on_demand() (lib/boost.php) using ON DUPLICATE KEY UPDATE. Beyond the duplication, that was a real data-integrity inconsistency between two paths writing the same shape of data: silently-drop vs last-write-wins on the same key. Since boost_redirect gates cmd.php's writes and boost_poller_on_demand() returns early when boost_redirect is on, the two paths never run concurrently for the same install; they're alternate write paths selected by that one setting, not competing writers. A genuine collision on (local_data_id, rrd_name, time) in either path represents a retry of an already-collected sample -- a batch resent after a partial failure, or two writers racing on the same rounded second -- not two independently meaningful readings, since RRD only stores one value per timestamp regardless. Standardizes on INSERT IGNORE (first write wins), matching the sibling poller_output insert this table shadows, which has always used INSERT IGNORE, and 3 of the 4 existing call sites. Extracts boost_flush_output_batch() (lib/boost.php) as the one shared helper, handling both the batched INSERT and the max_allowed_packet-safe chunking cmd.php's three sites previously didn't do at all (they only chunked by item count). Integration test exercises a genuine duplicate-key collision against a real (SQLite-backed) table and asserts the chosen first-write-wins behavior. Closes #7536 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(tests): resolve cross-branch test interactions found by full-suite verification Two test-only bugs surfaced only when all four fix branches for the poller_item/Boost hardening pass ran together, not when each ran in isolation: - BoostArchiveHandoffIntegrationTest extracted the archive-forward INSERT IGNORE by searching lib/boost.php for the first occurrence of "INSERT IGNORE INTO poller_output_boost". The Boost SQL-optimization branch adds a second, earlier occurrence (the new boost_flush_output_batch() helper's single-line prefix), so the naive search now grabbed the wrong snippet. Anchor on the multi-line "...poller_output_boost\n\tSELECT *" shape that only the archive-forward statement has. - PollerItemRemotePurgeHandOffTest calls push_out_host() (via api_data_source_change_host()), which needs CACTI_PATH_LIBRARY. The test never requires include/global.php (which normally defines it), and was relying on some other file in a full multi-thousand-file suite run having defined it first as a process-wide side effect. Define it directly instead of depending on unguaranteed load order. No production code changed. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * style(poller): satisfy php-cs-fixer strict_param in the script-server count Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): order the single-archive-table flush by the table's PK columns Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): key the max_allowed_packet cache per connection Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(tests): restore the default db_* session after swapping it for a fake handle Left swapped, the fake handle answered every later read_config_option() in the run; on PHP 8.4 the extra deprecation traffic routed through CactiErrorHandler turned that into an uncaught PDOException and killed the pest process. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net>
153 lines
6.8 KiB
PHP
153 lines
6.8 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 #7520: poller_boost.php decided whether to drop a
|
|
* run's archive tables using `SELECT SUM(status) FROM
|
|
* poller_output_boost_processes` -- an aggregate over whatever completion
|
|
* rows happened to land. If 3 of 4 shards succeed and the 4th child crashes
|
|
* before writing a completion row, that sum is still positive and every
|
|
* archive table for the run, including the crashed child's entire
|
|
* unprocessed shard, gets dropped.
|
|
*
|
|
* The fix reuses the drain loop's own completeness signal -- COUNT(*) FROM
|
|
* poller_output_boost_processes compared against $expected_children -- to
|
|
* gate the drop instead.
|
|
*
|
|
* This proves the discriminating case against a real server's SUM()/COUNT()
|
|
* semantics: exactly the scenario where the old gate and the new gate
|
|
* disagree, which is the entire point of the fix.
|
|
*
|
|
* Requires a MySQL/MariaDB instance; see BoostArchiveHandoffIntegrationTest.php
|
|
* for the same env vars / docker fallback. Skips (does not fail) if
|
|
* unreachable.
|
|
*/
|
|
|
|
require_once __DIR__ . '/../Helpers/UnitStubs.php';
|
|
require_once dirname(__DIR__, 2) . '/lib/database.php';
|
|
|
|
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;
|
|
}
|
|
|
|
global $database_sessions, $database_hostname, $database_port, $database_default;
|
|
|
|
$database_hostname = 'unit-test-host';
|
|
$database_port = 3306;
|
|
$database_default = 'cacti_boost_dropgate_test';
|
|
|
|
$database_sessions["$database_hostname:$database_port:$database_default"] = $this->pdo;
|
|
|
|
$this->pdo->exec('DROP TABLE IF EXISTS poller_output_boost_processes');
|
|
$this->pdo->exec('CREATE TABLE poller_output_boost_processes (status int(10) NOT NULL default 0)');
|
|
});
|
|
|
|
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_processes');
|
|
}
|
|
|
|
// 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('3 of 4 shards completing makes the old SUM(status)>0 gate say "drop", real data', function () {
|
|
if (!isset($this->pdo) || !$this->pdo instanceof PDO) {
|
|
return;
|
|
}
|
|
|
|
// Child 4 crashed before writing any completion row at all.
|
|
db_execute_prepared('INSERT INTO poller_output_boost_processes (status) VALUES (?)', [12]);
|
|
db_execute_prepared('INSERT INTO poller_output_boost_processes (status) VALUES (?)', [8]);
|
|
db_execute_prepared('INSERT INTO poller_output_boost_processes (status) VALUES (?)', [5]);
|
|
|
|
$rrd_updates = db_fetch_cell('SELECT SUM(status) FROM poller_output_boost_processes');
|
|
|
|
// This is exactly the old, buggy gate: positive sum from partial success.
|
|
expect((int) $rrd_updates)->toBeGreaterThan(0);
|
|
});
|
|
|
|
test('the same 3-of-4 scenario correctly withholds the drop under the completeness gate', function () {
|
|
if (!isset($this->pdo) || !$this->pdo instanceof PDO) {
|
|
return;
|
|
}
|
|
|
|
db_execute_prepared('INSERT INTO poller_output_boost_processes (status) VALUES (?)', [12]);
|
|
db_execute_prepared('INSERT INTO poller_output_boost_processes (status) VALUES (?)', [8]);
|
|
db_execute_prepared('INSERT INTO poller_output_boost_processes (status) VALUES (?)', [5]);
|
|
|
|
$expected_children = 4;
|
|
$boost_completed = (int) db_fetch_cell('SELECT COUNT(*) FROM poller_output_boost_processes');
|
|
|
|
// This is the exact expression the fix gates the DROP TABLE on
|
|
// (poller_boost.php: boost_completed_children() >= $expected_children).
|
|
expect($boost_completed >= $expected_children)->toBeFalse();
|
|
});
|
|
|
|
test('once all 4 shards report, both the old sum gate and the new completeness gate agree to drop', function () {
|
|
if (!isset($this->pdo) || !$this->pdo instanceof PDO) {
|
|
return;
|
|
}
|
|
|
|
db_execute_prepared('INSERT INTO poller_output_boost_processes (status) VALUES (?)', [12]);
|
|
db_execute_prepared('INSERT INTO poller_output_boost_processes (status) VALUES (?)', [8]);
|
|
db_execute_prepared('INSERT INTO poller_output_boost_processes (status) VALUES (?)', [5]);
|
|
db_execute_prepared('INSERT INTO poller_output_boost_processes (status) VALUES (?)', [3]);
|
|
|
|
$expected_children = 4;
|
|
|
|
$rrd_updates = (int) db_fetch_cell('SELECT SUM(status) FROM poller_output_boost_processes');
|
|
$boost_completed = (int) db_fetch_cell('SELECT COUNT(*) FROM poller_output_boost_processes');
|
|
|
|
expect($rrd_updates > 0)->toBeTrue();
|
|
expect($boost_completed >= $expected_children)->toBeTrue();
|
|
});
|
|
|
|
test('a shard that legitimately processed zero rows (status = 0) does not falsely block the drop', function () {
|
|
if (!isset($this->pdo) || !$this->pdo instanceof PDO) {
|
|
return;
|
|
}
|
|
|
|
// All 4 children completed, but one had nothing to update this cycle.
|
|
// COUNT(*) still correctly reflects 4 completions even though SUM(status)
|
|
// would undercount the "did work happen" question -- the completeness
|
|
// gate deliberately asks a different question than $rrd_updates does.
|
|
db_execute_prepared('INSERT INTO poller_output_boost_processes (status) VALUES (?)', [12]);
|
|
db_execute_prepared('INSERT INTO poller_output_boost_processes (status) VALUES (?)', [0]);
|
|
db_execute_prepared('INSERT INTO poller_output_boost_processes (status) VALUES (?)', [5]);
|
|
db_execute_prepared('INSERT INTO poller_output_boost_processes (status) VALUES (?)', [3]);
|
|
|
|
$expected_children = 4;
|
|
$boost_completed = (int) db_fetch_cell('SELECT COUNT(*) FROM poller_output_boost_processes');
|
|
|
|
expect($boost_completed >= $expected_children)->toBeTrue();
|
|
});
|