Notification queue: process claim is never applied, and failed sends are discarded

[somethingwithproof]

thold_notify.php claims rows from the notification queue but never filters on the claim, so concurrent runs can both send the same notification. A failed send is also discarded rather than retried.

The claim is written but not read back

thold_notify.php:111 marks the rows it intends to handle:

UPDATE notification_queue SET process_id = ? WHERE event_processed = 0

Then :148 drains them:

thold_notification_execute();

with no arguments. $pid defaults to 0, so $sql_where stays empty (thold_functions.php:7184, :7229, :7449) and the drain queries select every unprocessed row regardless of which process claimed it.

poller_thold.php:142-150 launches thold_notify.php on every poller cycle with no check that the previous run has finished, so two instances can overlap and both call mailer() on the same rows.

Passing $pid through to thold_notification_execute($pid) restores the intended behaviour.

The claim happens before the running-instance check

The claiming UPDATE at :111 runs before register_process_start() at :121, so a second instance overwrites the first instance's process_id even in the case where it goes on to exit.

Making the claim conditional (WHERE event_processed = 0 AND process_id = 0) and checking db_affected_rows() would make it atomic.

The guard is skipped entirely on non-unix

if ($config['cacti_server_os'] == 'unix') {
    ... register_process_start() ... exit(1);
}

On Windows a false from register_process_start() has no effect and the process continues. Same shape at thold_process.php:119.

A failed send is not retried

When mailer() fails, the row is still marked event_processed = 1 with error_code = 1 (thold_functions.php:7305-7310, :7500-7506). The notification is lost, with no backoff and no operator-visible queue of failures — which matches the symptom reported in #754.

Leaving event_processed = 0, incrementing an attempt counter and backing off would make a transient SMTP outage recoverable.

[somethingwithproof]

Implementation of the claim half in #804: the identifier now reaches the drain, the claim follows the process registration and takes only unheld rows, and an instance that cannot check for a live peer stands down instead of draining the queue a second time.

The retry half is deliberately not in it. notification_queue has nowhere to count attempts, and leaving event_processed = 0 without a counter turns a permanently failing notification into one that retries every cycle forever. That needs a schema change and belongs in its own PR.

[somethingwithproof]

Fixed in #804 and #811.