From 69972a7636489a375d264c62450d59896a369a4e Mon Sep 17 00:00:00 2001 From: Matthew Hilton Date: Mon, 17 Jun 2024 11:25:42 +1000 Subject: [PATCH] [#14] bugfix: handle large numbers of logs to purge --- classes/helper.php | 35 ++++++++++++++++++------- classes/task/purge_log.php | 15 ++++++----- classes/task/purge_log_adhoc.php | 44 ++++++++++++++++++++++++++++++++ lang/en/local_maillog.php | 4 +-- version.php | 2 +- 5 files changed, 81 insertions(+), 19 deletions(-) create mode 100644 classes/task/purge_log_adhoc.php diff --git a/classes/helper.php b/classes/helper.php index e5dc3e4..3625865 100644 --- a/classes/helper.php +++ b/classes/helper.php @@ -34,19 +34,31 @@ class helper { + /** + * Purges emails logged older than $maxdays + * @param int $maxdays the number of days old a log should be before being purged + */ static function purge($maxdays) { - global $CFG, $DB; + global $DB; - $olderthantime = time() - $maxdays * 24 * 60 * 60; + $olderthantime = time() - $maxdays * DAYSECS; - // We can straight up delete records without attachments - $DB->delete_records_select('mail_log', 'timesent < ? AND attachment IS NULL', array($olderthantime)); + // We can straight up delete records without attachments. This is very efficient. + $DB->delete_records_select('mail_log', 'timesent < ? AND attachment IS NULL', [$olderthantime]); - // If there's any left to delete, these must have attachments - they should be removed one-by-one - $logitems = $DB->get_records_select('mail_log', 'timesent < ?', array($olderthantime)); - foreach ($logitems as $item) { - \local_maillog\helper::delete(array($item->id)); - } + // The remaining have attachments which need to also be deleted. + // Delete these in chunks of 1k, as to not exceed the query parameter limit. + // Do this until there are no more records to delete (or ran out of iterations as a fail safe); + for ($i = 0; $i < 1000; $i++) { + $records = $DB->get_records_select('mail_log', 'timesent < ?', [$olderthantime], '', 'id', 0, 1000); + $ids = array_column($records, 'id'); + + // Done - exit early. + if (empty($ids)) { + break; + } + \local_maillog\helper::delete($ids); + } } static function log_mail($success, $msg, $user, $from, $subject, $messagetext, $messagehtml, $attachment, $attachname, $usetrueaddress, $replyto, $replytoname, $wordwrapwidth, $queuestatus=0) { @@ -105,6 +117,11 @@ static function log_mail($success, $msg, $user, $from, $subject, $messagetext, $ $transaction->allow_commit(); } + /** + * Delete the logs with the given ids + * Note the size of the $ids param must not exceed the maximum db query parameter limit. + * @param array $ids + */ static function delete($ids) { global $DB; diff --git a/classes/task/purge_log.php b/classes/task/purge_log.php index 07fd780..ce404d7 100644 --- a/classes/task/purge_log.php +++ b/classes/task/purge_log.php @@ -39,13 +39,14 @@ public function get_name() { return get_string('task:purgelog', 'local_maillog'); } + /** + * Execute task + */ public function execute() { - // Purge old mail log entries - $maxdays = get_config('local_maillog', 'maxdays'); - if (empty($maxdays)) { - $maxdays = 7; - } - mtrace("Deleting log entries older than {$maxdays} days."); - \local_maillog\helper::purge($maxdays); + // Spawn adhoc task to do the purge, in case it fails it can be retried gracefully. + $task = new purge_log_adhoc(); + + // Deduplicate in case the previous one has not finished yet or one is already queued. + \core\task\manager::queue_adhoc_task($task, true); } } diff --git a/classes/task/purge_log_adhoc.php b/classes/task/purge_log_adhoc.php new file mode 100644 index 0000000..8c8f530 --- /dev/null +++ b/classes/task/purge_log_adhoc.php @@ -0,0 +1,44 @@ +. + +namespace local_maillog\task; + +use core\task\adhoc_task; +use local_maillog\helper; + +/** + * Mail log purge log task. + * + * @package local_maillog + * @author Matthew Hilton + * @copyright 2024 Catalyst IT Australia + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class purge_log_adhoc extends adhoc_task { + + /** + * Executes purge + */ + public function execute() { + // Purge old mail log entries + $maxdays = get_config('local_maillog', 'maxdays'); + if (empty($maxdays)) { + $maxdays = 7; + } + mtrace("Deleting log entries older than {$maxdays} days."); + helper::purge($maxdays); + } +} diff --git a/lang/en/local_maillog.php b/lang/en/local_maillog.php index 73dbcbd..a3e5660 100644 --- a/lang/en/local_maillog.php +++ b/lang/en/local_maillog.php @@ -50,7 +50,7 @@ $string['recordsshown'] = '{$a->countfiltered} of {$a->countall} records shown'; $string['sent'] = 'Sent'; $string['type_maillog'] = 'Mail log'; -$string['task:purgelog'] = 'Purge mail log'; +$string['task:purgelog'] = 'Queue adhoc purge of mail log'; $string['task:sendscheduled'] = 'Send scheduled emails'; $string['timesent'] = 'Time sent'; $string['withselected'] = 'With selected:'; @@ -58,4 +58,4 @@ // Privacy Strings. $string['privacy:metadata:mail_log:userid'] = 'The id that the email was sent to.'; $string['privacy:metadata:mail_log:toaddress'] = 'The email address that the email was sent to.'; -$string['privacy:metadata:mail_log'] = 'Stores information on sent emails from the system.'; \ No newline at end of file +$string['privacy:metadata:mail_log'] = 'Stores information on sent emails from the system.'; diff --git a/version.php b/version.php index 4ee82d5..05c838f 100644 --- a/version.php +++ b/version.php @@ -26,7 +26,7 @@ defined('MOODLE_INTERNAL') || die(); // Extra 0 due to broken previous version number. -$plugin->version = 20201106000; +$plugin->version = 20201106001; $plugin->requires = 2015051100; $plugin->cron = 0; $plugin->component = 'local_maillog';