From cd1a7291eaf814b1276f6f1a1e5e2cd7864861b1 Mon Sep 17 00:00:00 2001 From: Cameron Ball Date: Fri, 11 Mar 2022 09:50:45 +0800 Subject: [PATCH] MDL-73293 core_task: Cleanup stale adhoc task metadata --- lang/en/admin.php | 1 + lib/classes/task/manager.php | 65 +++++++++++++++++++++ lib/classes/task/task_lock_cleanup_task.php | 53 +++++++++++++++++ lib/db/tasks.php | 9 +++ version.php | 2 +- 5 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 lib/classes/task/task_lock_cleanup_task.php diff --git a/lang/en/admin.php b/lang/en/admin.php index 677b2c542d100..efd4a1b0fc946 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -1327,6 +1327,7 @@ $string['task_dbstats'] = 'Database'; $string['task_result'] = 'Result'; $string['tasktype'] = 'Type'; +$string['tasklockcleanuptask'] = 'Clean up ad hoc task metadata'; $string['taskadmintitle'] = 'Tasks'; $string['taskanalyticscleanup'] = 'Analytics cleanup'; $string['taskautomatedbackup'] = 'Automated backups'; diff --git a/lib/classes/task/manager.php b/lib/classes/task/manager.php index 889307002ca08..eb63d23f337da 100644 --- a/lib/classes/task/manager.php +++ b/lib/classes/task/manager.php @@ -1206,6 +1206,71 @@ public static function get_running_tasks($sort = ''): array { return $DB->get_records_sql($sql, $params); } + /** + * Cleanup stale task metadata. + */ + public static function cleanup_metadata() { + global $DB; + + $cronlockfactory = \core\lock\lock_config::get_lock_factory('cron'); + $runningtasks = self::get_running_tasks(); + + foreach ($runningtasks as $taskrecord) { + if ($taskrecord->timestarted > time() - HOURSECS) { + continue; + } + + if ($taskrecord->type == 'adhoc') { + $lock = $cronlockfactory->get_lock('adhoc_' . $taskrecord->id, 0); + } + + if ($taskrecord->type == 'scheduled') { + $lock = $cronlockfactory->get_lock($taskrecord->classname, 0); + } + + // If we got this lock it means one of three things: + // + // 1. The task was stopped abnormally and the metadata was not cleaned up + // 2. This is the process running the cleanup task + // 3. We took so long getting to it in this loop that it did finish, and we now have the lock + // + // In the case of 1. we need to make the task as failed, in the case of 2. and 3. we do nothing. + if (!empty($lock)) { + if ($taskrecord->classname == "\\" . \core\task\task_lock_cleanup_task::class) { + $lock->release(); + continue; + } + + // We need to get the record again to verify whether or not we are dealing with case 3. + $taskrecord = $DB->get_record('task_' . $taskrecord->type, ['id' => $taskrecord->id]); + + if ($taskrecord->type == 'scheduled') { + // Empty timestarted indicates that this task finished (case 3) and was properly cleaned up. + if (empty($taskrecord->timestarted)) { + $lock->release(); + continue; + } + + $task = self::scheduled_task_from_record($taskrecord); + $task->set_lock($lock); + self::scheduled_task_failed($task); + } else if ($taskrecord->type == 'adhoc') { + // Ad hoc tasks are removed from the DB if they finish successfully. + // If we can't re-get this task, that means it finished and was properly + // cleaned up. + if (!$taskrecord) { + $lock->release(); + continue; + } + + $task = self::adhoc_task_from_record($taskrecord); + $task->set_lock($lock); + self::adhoc_task_failed($task); + } + } + } + } + /** * This function is used to indicate that any long running cron processes should exit at the * next opportunity and restart. This is because something (e.g. DB changes) has changed and diff --git a/lib/classes/task/task_lock_cleanup_task.php b/lib/classes/task/task_lock_cleanup_task.php new file mode 100644 index 0000000000000..5ca5c398c3f43 --- /dev/null +++ b/lib/classes/task/task_lock_cleanup_task.php @@ -0,0 +1,53 @@ +. + +/** + * Cleanup adhoc task metadata. + * + * @package core + * @copyright 2022 Catalyst IT Australia Pty Ltd + * @author Cameron Ball + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace core\task; + +/** + * Adhoc task metadata cleanup task. + * + * @package core + * @copyright 2022 Catalyst IT Australia Pty Ltd + * @author Cameron Ball + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class task_lock_cleanup_task extends scheduled_task { + + /** + * Get a descriptive name for this task. + * + * @return string + */ + public function get_name() { + return get_string('tasklockcleanuptask', 'admin'); + } + + /** + * Processes task. + */ + public function execute() { + \core\task\manager::cleanup_metadata(); + } +} diff --git a/lib/db/tasks.php b/lib/db/tasks.php index 7a3021e92b06f..b7eaebfe355fe 100644 --- a/lib/db/tasks.php +++ b/lib/db/tasks.php @@ -428,4 +428,13 @@ 'month' => '*', 'dayofweek' => '*', ), + [ + 'classname' => 'core\task\task_lock_cleanup_task', + 'blocking' => 0, + 'minute' => 'R', + 'hour' => '0', + 'day' => '*', + 'dayofweek' => '*', + 'month' => '*' + ] ); diff --git a/version.php b/version.php index c9f8a0a5ca65b..0209b551c2b51 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2022060300.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2022060300.01; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. $release = '4.1dev (Build: 20220603)'; // Human-friendly version name