Files
cacti/tests/integration/BoostFlushOutputBatchPacketLimitTest.php
Thomas VincentandTheWitness 3c20f4ce7e Harden Boost data handoffs and graph cache writes (#7729)
* 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>
2026-09-12 20:17:35 -04:00

145 lines
5.1 KiB
PHP

<?php
/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2026 The Cacti Group |
+-------------------------------------------------------------------------+
| Cacti: The Complete RRDtool-based Graphing Solution |
+-------------------------------------------------------------------------+
*/
/*
* Integration coverage for boost_flush_output_batch()'s max_allowed_packet
* cache.
*
* The value used to be memoized in one static shared by every caller, but
* the helper is invoked with two different connections -- the default one
* for cmd.php's local writes and $remote_db_cnn_id from a remote poller --
* and max_allowed_packet is a per-server setting. Whichever connection
* called first decided the chunk size for all the others: too small wastes
* round trips, too large builds an INSERT the second server rejects. The
* cache is now keyed per connection, so each one is looked up exactly once
* against its own server.
*/
require_once dirname(__DIR__) . '/Helpers/UnitStubs.php';
require_once dirname(__DIR__) . '/Helpers/FakeMySQLPDO.php';
require_once dirname(__DIR__, 2) . '/include/vendor/autoload.php';
require_once dirname(__DIR__, 2) . '/lib/database.php';
require_once dirname(__DIR__, 2) . '/lib/boost.php';
/**
* Counts the max_allowed_packet lookups this connection is asked for, so
* the cache key can be observed per connection instead of per process.
*/
class BoostPacketLookupPDO extends FakeMySQLPDO {
public int $packet_lookups = 0;
public function __construct() {
parent::__construct();
$this->exec('CREATE TABLE poller_output_boost (
local_data_id INTEGER NOT NULL,
rrd_name TEXT NOT NULL,
time TEXT NOT NULL,
output TEXT NOT NULL,
PRIMARY KEY (local_data_id, rrd_name, time)
)');
}
public function prepare(string $query, array $options = []): PDOStatement|false {
if (stripos($query, 'max_allowed_packet') !== false) {
$this->packet_lookups++;
}
return parent::prepare($query, $options);
}
}
class BoostFailingChunkPDO extends BoostPacketLookupPDO {
public int $insert_attempts = 0;
public function prepare(string $query, array $options = []): PDOStatement|false {
if (stripos($query, 'max_allowed_packet') !== false) {
$this->packet_lookups++;
return parent::prepare("SELECT 'max_allowed_packet' AS Variable_name, '512' AS Value", $options);
}
if (stripos(ltrim($query), 'INSERT IGNORE INTO poller_output_boost') === 0) {
$this->insert_attempts++;
if ($this->insert_attempts === 2) {
throw new RuntimeException('injected second-chunk failure');
}
}
return parent::prepare($query, $options);
}
}
beforeEach(function () {
global $database_sessions, $database_hostname, $database_port, $database_default;
$this->db_globals = [$database_sessions, $database_hostname, $database_port, $database_default];
});
// Put the default db_* connection back. Left cleared, every later
// read_config_option() in the run loses its connection and the suite aborts.
afterEach(function () {
global $database_sessions, $database_hostname, $database_port, $database_default;
[$database_sessions, $database_hostname, $database_port, $database_default] = $this->db_globals;
});
test('max_allowed_packet is read once per connection, not once per process', function () {
global $database_sessions, $database_hostname, $database_port, $database_default;
// no default connection is registered, so both writes have to go
// through the connection they were handed
$database_hostname = 'unit_test_host';
$database_port = '0';
$database_default = 'unit_test_db';
$database_sessions = [];
$local = new BoostPacketLookupPDO();
$remote = new BoostPacketLookupPDO();
boost_flush_output_batch(["(1,'ds','2024-01-01 00:00:00','10')"], $local);
// the second connection is a different server: it must be asked for its
// own limit rather than inheriting the one cached for $local
boost_flush_output_batch(["(2,'ds','2024-01-01 00:00:00','20')"], $remote);
expect($local->packet_lookups)->toBe(1);
expect($remote->packet_lookups)->toBe(1);
// the per-connection value is still cached: no repeat lookup on reuse
boost_flush_output_batch(["(3,'ds','2024-01-01 00:00:00','30')"], $local);
expect($local->packet_lookups)->toBe(1);
// and each connection got its own rows
foreach ([[$local, 2], [$remote, 1]] as [$conn, $expected]) {
$count = (int) $conn->query('SELECT COUNT(*) AS c FROM poller_output_boost')->fetch(PDO::FETCH_ASSOC)['c'];
expect($count)->toBe($expected);
}
});
test('a failed chunk stops the batch before later tuples are staged', function () {
$conn = new BoostFailingChunkPDO();
$output = str_repeat('x', 300);
$result = boost_flush_output_batch([
"(11,'first','2024-01-01 00:00:00','{$output}')",
"(12,'failed','2024-01-01 00:00:00','{$output}')",
"(13,'later','2024-01-01 00:00:00','{$output}')",
], $conn);
$ids = $conn->query('SELECT local_data_id FROM poller_output_boost ORDER BY local_data_id')->fetchAll(PDO::FETCH_COLUMN);
expect($result)->toBeFalse()
->and($conn->insert_attempts)->toBe(2)
->and($ids)->toBe([11]);
});