From 53f45fa008e6671898d84fe75aa73adf6e92ea19 Mon Sep 17 00:00:00 2001 From: johnnyq Date: Fri, 31 Jul 2026 16:35:39 -0400 Subject: [PATCH] Fix queued backups never building, per-type retention, and cron connection handling --- admin/backup.php | 4 +- admin/post/backup.php | 11 +++- cron/backup.php | 10 ++++ cron/cron.php | 14 ++++++ functions/backup.php | 113 +++++++++++++++++++++++++----------------- 5 files changed, 103 insertions(+), 49 deletions(-) diff --git a/admin/backup.php b/admin/backup.php index 51244262..6740ab1f 100644 --- a/admin/backup.php +++ b/admin/backup.php @@ -223,9 +223,9 @@ if (!empty($_SESSION['backup_master_key_reveal'])) { 0 disables age-based deletion.
- + - The newest is never deleted. + Counted separately for each type. The newest of each is never deleted.
diff --git a/admin/post/backup.php b/admin/post/backup.php index ff94fbeb..bc43741e 100644 --- a/admin/post/backup.php +++ b/admin/post/backup.php @@ -20,7 +20,16 @@ if (isset($_GET['queue_backup'])) { if ($backup_id > 0) { logAudit("Backup", "Queue", ($session_name ?? 'Unknown User') . " queued a " . backupTypeLabel($type)); - flashAlert(backupTypeLabel($type) . " queued - it will start within a minute and you will be notified when it is ready."); + + // Saying "it will start within a minute" when the master cron switch is off is a lie + // the user only discovers by waiting, so check before promising. + $cron_row = mysqli_fetch_assoc(mysqli_query($mysqli, "SELECT config_enable_cron FROM settings WHERE company_id = 1")); + + if (intval($cron_row['config_enable_cron']) === 0) { + flashAlert(backupTypeLabel($type) . " queued, but cron is switched off in Settings > Notifications, so it will not start until that is enabled.", 'error'); + } else { + flashAlert(backupTypeLabel($type) . " queued - it will start within a minute and you will be notified when it is ready."); + } } else { flashAlert($error ?? "Could not queue the backup.", 'error'); } diff --git a/cron/backup.php b/cron/backup.php index fad9ae2d..0bfd6c7f 100644 --- a/cron/backup.php +++ b/cron/backup.php @@ -46,6 +46,16 @@ if ($queued > 0) { * CONTRIBUTING's cron rules. A second dispatch in the same day (a manual Run Now, a catch-up * after downtime) must not produce a second scheduled archive. */ +$backup_job_row = mysqli_fetch_assoc(mysqli_query($mysqli, "SELECT cron_job_enabled FROM cron_jobs WHERE cron_job_name = 'backup'")); + +if (empty($backup_job_row['cron_job_enabled'])) { + // Reached by a Run Now, which is how a queued backup gets built while the schedule is + // off. Building the scheduled archive as well would hand somebody who asked for a + // database backup an unasked-for full one. Scheduled work belongs to the schedule. + echo "Scheduled backups are switched off - built queued work only\n"; + return; +} + $type = in_array($config_backup_cron_type, backupUnattendedTypes(), true) ? $config_backup_cron_type : BACKUP_TYPE_FULL; $type_esc = escapeSql($type); diff --git a/cron/cron.php b/cron/cron.php index 4fa25c63..722aeaba 100644 --- a/cron/cron.php +++ b/cron/cron.php @@ -183,6 +183,11 @@ function cronJobFinished($mysqli, string $job_name, string $status, ?float $dura // "no job was due" apart from "nothing has run this since the server was rebuilt". mysqli_query($mysqli, "UPDATE settings SET config_cron_last_dispatch_at = '" . date('Y-m-d H:i:s') . "' WHERE company_id = 1"); +// Best effort: ask the server not to hang up while a long job is quiet. +if (function_exists('backupDbHoldOpen')) { + backupDbHoldOpen($mysqli); +} + // A fatal error inside a job cannot be caught, and it takes the rest of the cycle with it. // Recording which job was running at the time is the only trace of that left behind. $cron_dispatch_running = null; @@ -224,6 +229,15 @@ foreach (cronJobRegistry() as $cron_dispatch_job) { $cron_dispatch_running = $cron_dispatch_job['name']; $cron_dispatch_started = microtime(true); + // Every job runs in this one process on this one connection, and a job that spends + // minutes on network or file work without querying leaves it idle long enough for a + // server with a short wait_timeout to close it. The next job then dies on a connection + // it never touched. Re-establish before each one so a quiet job cannot poison the rest + // of the cycle. + if (function_exists('backupDbEnsure')) { + $mysqli = backupDbEnsure($mysqli); + } + try { require_once $cron_dispatch_path; cronJobFinished($mysqli, $cron_dispatch_job['name'], 'Completed', microtime(true) - $cron_dispatch_started); diff --git a/functions/backup.php b/functions/backup.php index 09c0e0eb..0955339a 100644 --- a/functions/backup.php +++ b/functions/backup.php @@ -532,7 +532,15 @@ function backupQueue(mysqli $mysqli, string $type, string $created_by, ?string & mysqli_query($mysqli, "INSERT INTO backups SET backup_type = '$type_esc', backup_file_name = '', backup_status = 'Pending', backup_source = 'Manual', backup_created_by = '$created_by_esc'"); - return intval(mysqli_insert_id($mysqli)); + $backup_id = intval(mysqli_insert_id($mysqli)); + + // Ask the dispatcher to run the backup job on its next pass. Without this the row sits + // Pending for ever on a default install: the job ships disabled, so the schedule never + // calls it and nothing ever builds what the button just queued. run_now is honoured + // whether or not a job is enabled, which is exactly the case this needs. + mysqli_query($mysqli, "UPDATE cron_jobs SET cron_job_run_now = 1 WHERE cron_job_name = 'backup'"); + + return $backup_id; } /** @@ -841,51 +849,10 @@ function backupRunRetention(mysqli $mysqli): array } - // Keep the newest $count complete backups regardless of age - $keep = []; - $keep_res = mysqli_query($mysqli, "SELECT backup_id FROM backups WHERE backup_status = 'Complete' ORDER BY backup_created_at DESC LIMIT " . max(1, $count)); - if ($keep_res) { - while ($row = mysqli_fetch_assoc($keep_res)) { - $keep[] = intval($row['backup_id']); - } - } - - $keep_clause = empty($keep) ? "" : " AND backup_id NOT IN (" . implode(",", $keep) . ")"; - - // Age-based removal - if ($days > 0) { - $old = mysqli_query($mysqli, "SELECT backup_id FROM backups WHERE backup_created_at < CURDATE() - INTERVAL $days DAY $keep_clause"); - if ($old) { - while ($row = mysqli_fetch_assoc($old)) { - if (backupDeleteById($mysqli, intval($row['backup_id']))) { - $result['deleted']++; - } - } - } - } - - // Count-based removal - if ($count > 0 && !empty($keep)) { - $surplus = mysqli_query($mysqli, "SELECT backup_id FROM backups WHERE backup_status = 'Complete' $keep_clause"); - if ($surplus) { - while ($row = mysqli_fetch_assoc($surplus)) { - if (backupDeleteById($mysqli, intval($row['backup_id']))) { - $result['deleted']++; - } - } - } - } - - // Failed rows never had a usable file - $failed = mysqli_query($mysqli, "SELECT backup_id FROM backups WHERE backup_status = 'Failed' AND backup_created_at < CURDATE() - INTERVAL 7 DAY"); - if ($failed) { - while ($row = mysqli_fetch_assoc($failed)) { - if (backupDeleteById($mysqli, intval($row['backup_id']))) { - $result['deleted']++; - } - } - } - + // Reconciliation runs BEFORE the retention maths, not after. A row this rescues or + // adopts has to be counted by the same pass that decides what to delete - otherwise it + // escapes retention until tomorrow, and a second run on the same day is not the no-op + // CONTRIBUTING's third cron rule asks for. // Orphans: rows whose file is gone, and files with no row $known = []; $rows = mysqli_query($mysqli, "SELECT backup_id, backup_file_name, backup_status FROM backups"); @@ -942,6 +909,60 @@ function backupRunRetention(mysqli $mysqli): array $result['orphan_files']++; } + + // Keep the newest $count complete backups OF EACH TYPE, regardless of age. + // + // Per type rather than one shared pool: a master key export is a few hundred bytes and + // a full backup is gigabytes, so counting them together means five master key exports + // silently evict every real backup on the box. They are also the artefact you would + // least want retention to quietly remove. + $keep = []; + foreach (backupAllTypes() as $keep_type) { + $keep_type_esc = escapeSql($keep_type); + $keep_res = mysqli_query($mysqli, "SELECT backup_id FROM backups WHERE backup_status = 'Complete' AND backup_type = '$keep_type_esc' ORDER BY backup_created_at DESC LIMIT " . max(1, $count)); + if ($keep_res) { + while ($row = mysqli_fetch_assoc($keep_res)) { + $keep[] = intval($row['backup_id']); + } + } + } + + $keep_clause = empty($keep) ? "" : " AND backup_id NOT IN (" . implode(",", $keep) . ")"; + + // Age-based removal + if ($days > 0) { + $old = mysqli_query($mysqli, "SELECT backup_id FROM backups WHERE backup_created_at < CURDATE() - INTERVAL $days DAY $keep_clause"); + if ($old) { + while ($row = mysqli_fetch_assoc($old)) { + if (backupDeleteById($mysqli, intval($row['backup_id']))) { + $result['deleted']++; + } + } + } + } + + // Count-based removal + if ($count > 0 && !empty($keep)) { + $surplus = mysqli_query($mysqli, "SELECT backup_id FROM backups WHERE backup_status = 'Complete' $keep_clause"); + if ($surplus) { + while ($row = mysqli_fetch_assoc($surplus)) { + if (backupDeleteById($mysqli, intval($row['backup_id']))) { + $result['deleted']++; + } + } + } + } + + // Failed rows never had a usable file + $failed = mysqli_query($mysqli, "SELECT backup_id FROM backups WHERE backup_status = 'Failed' AND backup_created_at < CURDATE() - INTERVAL 7 DAY"); + if ($failed) { + while ($row = mysqli_fetch_assoc($failed)) { + if (backupDeleteById($mysqli, intval($row['backup_id']))) { + $result['deleted']++; + } + } + } + return $result; }