fix(poller): evaluate the right thresholds and clear only what was checked

[somethingwithproof]

Fixes #783. Three defects in includes/polling.php.

The poller_id alternation was unparenthesised

AND h.poller_id = 1 OR h.poller_id IS NULL
AND td.tcheck = 1
AND h.status = 3

AND binds tighter than OR, so this reads as

( thold_per_enabled='on' AND (...) AND h.poller_id = 1 )
OR
( h.poller_id IS NULL AND td.tcheck = 1 AND h.status = 3 )

The first branch carries neither the dirty-flag nor the device-status filter. The second cannot match at all, because the LEFT JOIN gives a NULL host a NULL status.

So on poller 1, every enabled threshold is re-evaluated on every run against whatever lastread happens to be sitting there — including for devices that are down. Fail counters advance on stale readings, and a threshold that breached once keeps counting until it crosses its trigger and alerts on data that may be hours old.

Confirmed against MariaDB 10.6 rather than by reading it. Three thresholds: one with new data on an up device, one with no new data, one on a down device.

--- current (unparenthesised) ---   --- parenthesised ---
id                                  id
1                                   1
2       <- tcheck = 0
3       <- device status != 3

The poller_id != 1 and non-remote variants of the same query are already parenthesised correctly, which is what makes this look like a typo rather than intent.

The dirty flag was cleared for more than was evaluated

After the loop, tcheck was cleared by poller, or across the whole table:

db_execute('UPDATE thold_data AS td SET td.tcheck = 0');

Any thold_poller_output() write landing between the select and that update was discarded without ever being evaluated. With multiple pollers or a remote collector, thresholds whose data arrives late in the cycle are simply skipped.

It now clears exactly the identifiers that were visited. That also collapses the three branches into one statement: the select already scopes to this poller, so the identifiers carry that scoping with them.

array_chunk was given a count where a size belongs

$chunks = ceil(sizeof($rrd_update_array) / 50);
$rrd_update_array_chunks = array_chunk($rrd_update_array, $chunks, true);

The second argument is the size of each chunk. With 500 readings this produced 50 chunks of 10 where 10 chunks of 50 was intended; with 25 readings, 25 chunks of one. Each chunk costs a select plus a bulk insert.

Tests

Six tests. The dirty-flag behaviour is checked by driving thold_check_all_thresholds() and asserting on the statement it issues — that it clears the identifiers it visited, that it issues nothing when it visited nothing, and that no statement clears table-wide.

The precedence and chunking fixes are guarded structurally, and the test says so in as many words. Proving a SQL precedence bug needs a database to run the query against, which the unit workflow has none of; the evidence above is the proof, and the guard is there so the parentheses cannot quietly go missing again.

Coverage is now scoped to includes/polling.php as well as thold_functions.php, since this is the first change to it.

[Copilot]

Pull request overview

This PR fixes three scheduling/selection defects in the Thold poller pipeline (includes/polling.php) so the poller evaluates the intended threshold set (correct SQL precedence), batches daemon inserts correctly, and only clears the dirty flag (tcheck) for thresholds that were actually checked. It also adds a PHPUnit-based unit test harness plus CI workflow to prevent regressions.

Changes:

• Fix SQL AND/OR precedence for poller 1 remote-storage threshold selection.
• Fix daemon batching by using the correct array_chunk() chunk size (50).
• Clear tcheck only for the evaluated threshold IDs, and add unit tests + CI (including changed-lines coverage enforcement) to guard these behaviors.

Reviewed changes

Copilot reviewed 13 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
includes/polling.php	Fixes SQL precedence, daemon chunking, and makes tcheck clearing ID-scoped.
tests/Unit/PollerSchedulingTest.php	Adds unit tests covering poller scheduling behavior and structural guards.
tests/TestCase.php	Introduces a base PHPUnit TestCase with common reset/setup helpers.
tests/Helpers/CactiStubs.php	Adds programmable stubs/recording for Cacti global helper functions.
tests/fixtures/optional-core-functions.php	Provides optional Cacti core functions for tests that need them.
tests/fixtures/cacti-lib/variables.php	Fixture file to satisfy runtime include_once() expectations in plugin code.
tests/docker/Dockerfile	Adds a reproducible local/CI unit-test runner image (PHP 8.1 + pcov).
tests/docker/docker-compose.yml	Provides a local docker compose run mirror of the CI unit job.
tests/bootstrap-unit.php	Adds PHPUnit bootstrap that stubs Cacti framework functions and sets up globals.
tests/bin/patch-coverage.php	Adds changed-lines coverage gate helper for CI PR enforcement.
phpunit.xml	Adds PHPUnit configuration and scopes coverage to relevant plugin files.
composer.json	Adds dev dependencies and scripts for linting/testing (including docker runner).
.gitignore	Ignores vendor, coverage outputs, and PHPUnit caches for the new test tooling.
.github/workflows/php-unit-tests.yml	Adds GitHub Actions workflow to run lint + unit tests + coverage gates in Docker.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.