fix(notify): recover unfinished queue claims

CHANGELOG.md

@@ -7,6 +7,8 @@
 * issue#710: Fixing Typo in thold_daemons.service File
 * issue#714: Increase the Name column to 255 characters
 * issue#719: Plugin Disabled due to mix of string and int
+* issue#784: Retry failed queued email notifications with bounded exponential backoff
+* issue#812: Recover stale notification claims, scope worker drains, and deprecate the ignored notification --thread option
 * issue: All Columns checkd on Thresholds page
 * issue: Special character previous value handling broken on data query indexes with special characters
 

INFO

@@ -21,7 +21,7 @@
 
 [info]
 name = thold
-version = 1.8.2
+version = 1.8.3
 longname = Thresholds
 author = The Cacti Group
 email = 

README.md

@@ -27,6 +27,28 @@ and become familiar with its settings.  From there, you can provide overall
 control of thold, and set defaults for things like Email bodies, weekend
 exemptions, alert log retention, logging, etc.
 
+When the notification queue is enabled, transient email failures are retried
+up to five times with exponential backoff from one to eight minutes. The
+Notification Queue page shows the attempt count and next eligible retry time.
+After the fifth failed attempt the row becomes a terminal error so a permanent
+SMTP or address failure cannot retry forever.
+
+Notification workers claim queue rows with their process ID and drain only
+that claim. Unfinished rows are released when a worker stops or notifications
+are suspended, while claims left by a hard-killed worker are recovered after
+its process registration disappears. A cross-platform database advisory lease
+prevents a second worker from taking the same slot. Because a database
+reconnect can drop that lease while PHP is still running, a stale process row
+is reclaimed immediately when an operating-system probe confirms that the old
+PID is gone. Unknown liveness waits for one expired worker timeout before
+recovery. A live PID is compared with the process registration time before a
+stale worker is trusted, preventing a recycled operating-system PID from
+blocking notification processing indefinitely.
+
+The legacy `thold_notify.php --thread=N` option remains accepted for operator
+compatibility but is deprecated and ignored; queue ownership and worker
+serialization are automatic.
+
 As with much of Cacti, settings should be documented in line with the actual
 setting.  If you find that any of these settings are ambiguous, please create a
 pull request with your proposed changes.

includes/database.php

@@ -1755,6 +1755,27 @@ function thold_upgrade_database($force = false) {
 		db_execute('UPDATE plugin_notification_lists SET enabled = "on"');
 	}
 
+	if (cacti_version_compare($oldv, '1.8.3', '<')) {
+		db_add_column('notification_queue', [
+			'name'     => 'attempt_count',
+			'type'     => 'int',
+			'unsigned' => true,
+			'NULL'     => false,
+			'default'  => '0',
+			'after'    => 'error_message']
+		);
+
+		db_add_column('notification_queue', [
+			'name'    => 'next_attempt',
+			'type'    => 'timestamp',
+			'NULL'    => true,
+			'default' => null,
+			'after'   => 'attempt_count']
+		);
+
+		db_add_index('notification_queue', 'INDEX', 'retry_ready', ['event_processed', 'process_id', 'next_attempt']);
+	}
+
 	db_add_column('thold_data', [
 		'name'     => 'external_id',
 		'type'     => 'varchar(20)',
@@ -2123,12 +2144,15 @@ function thold_setup_database() {
 	$data['columns'][] = ['name' => 'event_data', 'type' => 'longblob', 'NULL' => false, 'default' => ''];
 	$data['columns'][] = ['name' => 'error_code', 'type' => 'int', 'NULL' => false, 'default' => '0'];
 	$data['columns'][] = ['name' => 'error_message', 'type' => 'varchar(128)', 'NULL' => false, 'default' => ''];
+	$data['columns'][] = ['name' => 'attempt_count', 'type' => 'int', 'unsigned' => true, 'NULL' => false, 'default' => '0'];
+	$data['columns'][] = ['name' => 'next_attempt', 'type' => 'timestamp', 'NULL' => true, 'default' => null];
 	$data['columns'][] = ['name' => 'process_id', 'type' => 'int', 'unsigned' => true, 'NULL' => false, 'default' => '0'];
 	$data['columns'][] = ['name' => 'event_processed', 'type' => 'tinyint', 'unsigned' => true, 'NULL' => false, 'default' => '0'];
 	$data['columns'][] = ['name' => 'event_processed_time', 'type' => 'timestamp', 'NULL' => false, 'default' => '0000-00-00'];
 	$data['columns'][] = ['name' => 'event_processed_runtime', 'type' => 'double', 'unsigned' => true, 'NULL' => false, 'default' => '0'];
 	$data['primary']   = 'id';
 	$data['keys'][]    = ['name' => 'topic_processed', 'columns' => 'topic`, `event_processed'];
+	$data['keys'][]    = ['name' => 'retry_ready', 'columns' => 'event_processed`, `process_id`, `next_attempt'];
 	$data['keys'][]    = ['name' => 'process_id', 'columns' => 'process_id'];
 	$data['keys'][]    = ['name' => 'object_id', 'columns' => 'object_id'];
 	$data['keys'][]    = ['name' => 'host_id', 'columns' => 'host_id'];

notify_queue.php

@@ -463,6 +463,18 @@ function clearFilter() {
 			'sort'    => 'DESC',
 			'tip'     => __('Did this notification result in an error.  Hover on the error column for details.', 'thold')
 		],
+		'attempt_count' => [
+			'display' => __('Attempts', 'thold'),
+			'align'   => 'right',
+			'sort'    => 'DESC',
+			'tip'     => __('The number of delivery attempts made for this notification.', 'thold')
+		],
+		'next_attempt' => [
+			'display' => __('Next Attempt', 'thold'),
+			'align'   => 'right',
+			'sort'    => 'DESC',
+			'tip'     => __('When a failed notification is eligible for its next retry.', 'thold')
+		],
 		'event_processed_runtime' => [
 			'display' => __('Run Time', 'thold'),
 			'align'   => 'right',
@@ -491,14 +503,9 @@ function clearFilter() {
 
 			form_selectable_cell($n['id'], $n['id'], '', 'right');
 			form_selectable_cell($n['event_time'], $n['id'], '', 'right');
-			form_selectable_cell($n['event_processed'] == 0 ? __('Pending', 'thold') : __('Done', 'thold'), $n['id'], '', 'right');
 
-			if ($n['event_processed'] > 0) {
-				form_selectable_cell($n['error_code'] > 0 ? __('Errored', 'thold') : __('Success', 'thold'), $n['id'], '', 'right');
-				form_selectable_cell(number_format_i18n($n['event_processed_runtime'], 2), $n['id'], '', 'right');
-			} else {
-				form_selectable_cell(__('N/A', 'thold'), $n['id'], '', 'right');
-				form_selectable_cell(__('N/A', 'thold'), $n['id'], '', 'right');
+			foreach (thold_notification_queue_status_cells($n) as $cell) {
+				form_selectable_cell($cell, $n['id'], '', 'right');
 			}
 
 			form_checkbox_cell($n['object_name'], $n['id']);

tests/Helpers/CactiStubs.php

@@ -105,8 +105,8 @@ public static function reset() {
 	/**
 	 * Record one Cacti function call.
 	 *
-	 * @param string            $fn     Cacti function name.
-	 * @param string            $sql    SQL text, or '' for non-query calls.
+	 * @param string                   $fn     Cacti function name.
+	 * @param string                   $sql    SQL text, or '' for non-query calls.
 	 * @param array<int|string, mixed> $params Bound parameters, if any.
 	 *
 	 * @return void

tests/Helpers/ThresholdScenario.php

@@ -260,19 +260,11 @@ public function inMaintenance() {
 		CactiStubs::willAlwaysReturn('plugin_maint_check_cacti_host', true);
 
 		/*
-		 * thold include_once()s the maint plugin when it reports enabled. The
-		 * fixture supplies an empty file so the include succeeds; the function
-		 * it would define is already stubbed.
+		 * thold include_once()s the maint plugin when it reports enabled. Point
+		 * base_path at a tracked, repository-contained Cacti root fixture so a
+		 * unit test never writes into the caller's Cacti checkout.
 		 */
-		$maint = dirname(__DIR__, 3) . '/maint';
-
-		if (!is_dir($maint)) {
-			mkdir($maint, 0755, true);
-		}
-
-		if (!file_exists($maint . '/functions.php')) {
-			file_put_contents($maint . '/functions.php', "<?php\n");
-		}
+		$GLOBALS['config']['base_path'] = dirname(__DIR__) . '/fixtures/cacti-root';
 
 		return $this;
 	}

tests/TestCase.php

@@ -33,6 +33,17 @@ protected function setUp(): void {
 		$GLOBALS['rpn_error'] = false;
 	}
 
+	/**
+	 * Restore global paths changed by filesystem-oriented fixtures.
+	 *
+	 * @return void
+	 */
+	protected function tearDown(): void {
+		$GLOBALS['config']['base_path'] = $GLOBALS['__test_original_base_path'];
+
+		parent::tearDown();
+	}
+
 	/**
 	 * Load a plugin source file once per process.
 	 *