From 51e488eaf31307319e34c8b56a85c019016c7871 Mon Sep 17 00:00:00 2001 From: Josh Willcock Date: Wed, 20 May 2015 13:08:19 +0100 Subject: [PATCH 1/5] MDL-50287 completion: Separated slow task out from cron The mark started function for the completions scheduled task can be very slow and causes performance issues when running. By splitting this into it's own task it can be managed independantly from the other parts which update regularly. --- completion/cron.php | 24 +++++++- lang/en/admin.php | 2 + ...ask.php => completion_cron_daily_task.php} | 12 ++-- .../task/completion_cron_regular_task.php | 56 +++++++++++++++++++ lib/db/tasks.php | 11 +++- 5 files changed, 96 insertions(+), 9 deletions(-) rename lib/classes/task/{completion_cron_task.php => completion_cron_daily_task.php} (77%) create mode 100644 lib/classes/task/completion_cron_regular_task.php diff --git a/completion/cron.php b/completion/cron.php index 23e682c84c6..c252de4ed3a 100644 --- a/completion/cron.php +++ b/completion/cron.php @@ -28,20 +28,38 @@ defined('MOODLE_INTERNAL') || die(); require_once($CFG->libdir.'/completionlib.php'); /** + * Legacy - here for plugin use only, not used in scheduled tasks. * Update user's course completion statuses * * First update all criteria completions, then aggregate all criteria completions - * and update overall course completions + * and update overall course completions. */ function completion_cron() { - completion_cron_mark_started(); completion_cron_criteria(); completion_cron_completions(); } - +/** + * Update user's course completion statuses + * + * First update all criteria completions, then aggregate all criteria completions + * and update overall course completions + */ +function completion_regular_cron() { + // Two Functions which check criteria and learner completions regularly for accurate data. + completion_cron_criteria(); + completion_cron_completions(); +} +/** + * Marks users as started if the config option is set. + * + * One function which takes a long time 60+ minutes to enrol users onto completion started. + */ +function completion_daily_cron() { + completion_cron_mark_started(); +} /** * Mark users as started if the config option is set * diff --git a/lang/en/admin.php b/lang/en/admin.php index 7e7f4c71476..de8b022f37f 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -1020,6 +1020,8 @@ $string['taskcachecron'] = 'Background processing for caches'; $string['taskcalendarcron'] = 'Send calendar notifications'; $string['taskcheckforupdates'] = 'Check for updates'; $string['taskcompletioncron'] = 'Calculate completion data'; +$string['taskcompletioncronregular'] = 'Calculate regular completion data'; +$string['taskcompletioncrondaily'] = 'Completion mark as started'; $string['taskcontextcleanup'] = 'Cleanup contexts'; $string['taskcreatecontexts'] = 'Create missing contexts'; $string['taskdeletecachetext'] = 'Delete old text cache records'; diff --git a/lib/classes/task/completion_cron_task.php b/lib/classes/task/completion_cron_daily_task.php similarity index 77% rename from lib/classes/task/completion_cron_task.php rename to lib/classes/task/completion_cron_daily_task.php index b34860a8014..ace3a5bbada 100644 --- a/lib/classes/task/completion_cron_task.php +++ b/lib/classes/task/completion_cron_daily_task.php @@ -24,9 +24,11 @@ namespace core\task; /** - * Simple task to run the completion cron. + * Simple task to run the daily completion cron. + * @copyright 2013 onwards Martin Dougiamas http://dougiamas.com. + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. */ -class completion_cron_task extends scheduled_task { +class completion_cron_daily_task extends scheduled_task { /** * Get a descriptive name for this task (shown to admins). @@ -34,7 +36,7 @@ class completion_cron_task extends scheduled_task { * @return string */ public function get_name() { - return get_string('taskcompletioncron', 'admin'); + return get_string('taskcompletioncrondaily', 'admin'); } /** @@ -45,9 +47,9 @@ class completion_cron_task extends scheduled_task { global $CFG; if ($CFG->enablecompletion) { - // Completion cron. + // Daily Completion cron. require_once($CFG->dirroot.'/completion/cron.php'); - completion_cron(); + completion_daily_cron(); } } diff --git a/lib/classes/task/completion_cron_regular_task.php b/lib/classes/task/completion_cron_regular_task.php new file mode 100644 index 00000000000..29a0c527442 --- /dev/null +++ b/lib/classes/task/completion_cron_regular_task.php @@ -0,0 +1,56 @@ +. + +/** + * A scheduled task. + * + * @package core + * @copyright 2013 onwards Martin Dougiamas http://dougiamas.com + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +namespace core\task; + +/** + * Simple task to run the regular completion cron. + * @copyright 2013 onwards Martin Dougiamas http://dougiamas.com. + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. + */ +class completion_cron_regular_task extends scheduled_task { + + /** + * Get a descriptive name for this task (shown to admins). + * + * @return string + */ + public function get_name() { + return get_string('taskcompletioncronregular', 'admin'); + } + + /** + * Do the job. + * Throw exceptions on errors (the job will be retried). + */ + public function execute() { + global $CFG; + + if ($CFG->enablecompletion) { + // Regular Completion cron. + require_once($CFG->dirroot.'/completion/cron.php'); + completion_regular_cron(); + } + } + +} diff --git a/lib/db/tasks.php b/lib/db/tasks.php index 6c4d7d203da..9954d5926a8 100644 --- a/lib/db/tasks.php +++ b/lib/db/tasks.php @@ -159,7 +159,7 @@ $tasks = array( 'month' => '*' ), array( - 'classname' => 'core\task\completion_cron_task', + 'classname' => 'core\task\completion_cron_regular_task', 'blocking' => 0, 'minute' => '*', 'hour' => '*', @@ -167,6 +167,15 @@ $tasks = array( 'dayofweek' => '*', 'month' => '*' ), + array( + 'classname' => 'core\task\completion_cron_daily_task', + 'blocking' => 0, + 'minute' => 'R', + 'hour' => 'R', + 'day' => '*', + 'dayofweek' => '*', + 'month' => '*' + ), array( 'classname' => 'core\task\portfolio_cron_task', 'blocking' => 0, From 0628925bf1a7d982166509344eac8ff941e5bc63 Mon Sep 17 00:00:00 2001 From: Dan Poltawski Date: Thu, 27 Aug 2015 14:33:10 +0100 Subject: [PATCH 2/5] MDL-50287 completion: avoid introducing uncessary functions Also fix copyright --- completion/cron.php | 35 +------------------ .../task/completion_cron_daily_task.php | 2 +- .../task/completion_cron_regular_task.php | 7 ++-- lib/deprecatedlib.php | 23 +++++++++++- 4 files changed, 28 insertions(+), 39 deletions(-) diff --git a/completion/cron.php b/completion/cron.php index c252de4ed3a..f56e5dddb91 100644 --- a/completion/cron.php +++ b/completion/cron.php @@ -15,7 +15,7 @@ // along with Moodle. If not, see . /** - * Cron job for reviewing and aggregating course completion criteria + * Code used by scheduled tasks for reviewing and aggregating course completion criteria. * * @package core_completion * @category completion @@ -27,39 +27,6 @@ defined('MOODLE_INTERNAL') || die(); require_once($CFG->libdir.'/completionlib.php'); -/** - * Legacy - here for plugin use only, not used in scheduled tasks. - * Update user's course completion statuses - * - * First update all criteria completions, then aggregate all criteria completions - * and update overall course completions. - */ -function completion_cron() { - completion_cron_mark_started(); - - completion_cron_criteria(); - - completion_cron_completions(); -} -/** - * Update user's course completion statuses - * - * First update all criteria completions, then aggregate all criteria completions - * and update overall course completions - */ -function completion_regular_cron() { - // Two Functions which check criteria and learner completions regularly for accurate data. - completion_cron_criteria(); - completion_cron_completions(); -} -/** - * Marks users as started if the config option is set. - * - * One function which takes a long time 60+ minutes to enrol users onto completion started. - */ -function completion_daily_cron() { - completion_cron_mark_started(); -} /** * Mark users as started if the config option is set * diff --git a/lib/classes/task/completion_cron_daily_task.php b/lib/classes/task/completion_cron_daily_task.php index ace3a5bbada..fa014b1c888 100644 --- a/lib/classes/task/completion_cron_daily_task.php +++ b/lib/classes/task/completion_cron_daily_task.php @@ -49,7 +49,7 @@ class completion_cron_daily_task extends scheduled_task { if ($CFG->enablecompletion) { // Daily Completion cron. require_once($CFG->dirroot.'/completion/cron.php'); - completion_daily_cron(); + completion_cron_mark_started(); } } diff --git a/lib/classes/task/completion_cron_regular_task.php b/lib/classes/task/completion_cron_regular_task.php index 29a0c527442..215679c30f2 100644 --- a/lib/classes/task/completion_cron_regular_task.php +++ b/lib/classes/task/completion_cron_regular_task.php @@ -18,14 +18,14 @@ * A scheduled task. * * @package core - * @copyright 2013 onwards Martin Dougiamas http://dougiamas.com + * @copyright 2015 Josh Willcock * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ namespace core\task; /** * Simple task to run the regular completion cron. - * @copyright 2013 onwards Martin Dougiamas http://dougiamas.com. + * @copyright 2015 Josh Willcock * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. */ class completion_cron_regular_task extends scheduled_task { @@ -49,7 +49,8 @@ class completion_cron_regular_task extends scheduled_task { if ($CFG->enablecompletion) { // Regular Completion cron. require_once($CFG->dirroot.'/completion/cron.php'); - completion_regular_cron(); + completion_cron_criteria(); + completion_cron_completions(); } } diff --git a/lib/deprecatedlib.php b/lib/deprecatedlib.php index 8c85c9d60bc..2758fee0c56 100644 --- a/lib/deprecatedlib.php +++ b/lib/deprecatedlib.php @@ -2378,4 +2378,25 @@ function get_referer($stripquery = true) { } else { return ''; } -} \ No newline at end of file +} + +/** + * Update user's course completion statuses + * + * First update all criteria completions, then aggregate all criteria completions + * and update overall course completions. + * + * @deprecated since Moodle 3.0 MDL-50287 - please do not use this function any more. + * @todo Remove this function in Moodle 3.2 MDL-51226. + */ +function completion_cron() { + global $CFG; + require_once($CFG->dirroot.'/completion/cron.php'); + + debugging('completion_cron() is deprecated. Functionality has been moved to scheduled tasks.', DEBUG_DEVELOPER); + completion_cron_mark_started(); + + completion_cron_criteria(); + + completion_cron_completions(); +} From 160ccd3d09c2514c8f6a6ca9d218bd18f6653663 Mon Sep 17 00:00:00 2001 From: Dan Poltawski Date: Thu, 27 Aug 2015 14:36:38 +0100 Subject: [PATCH 3/5] MDL-50287 completion: Rename tasks to remove the word 'cron' --- lang/en/admin.php | 4 ++-- ...mpletion_cron_daily_task.php => completion_daily_task.php} | 4 ++-- ...tion_cron_regular_task.php => completion_regular_task.php} | 4 ++-- lib/db/tasks.php | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) rename lib/classes/task/{completion_cron_daily_task.php => completion_daily_task.php} (92%) rename lib/classes/task/{completion_cron_regular_task.php => completion_regular_task.php} (92%) diff --git a/lang/en/admin.php b/lang/en/admin.php index de8b022f37f..90e2c99ea63 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -1020,8 +1020,8 @@ $string['taskcachecron'] = 'Background processing for caches'; $string['taskcalendarcron'] = 'Send calendar notifications'; $string['taskcheckforupdates'] = 'Check for updates'; $string['taskcompletioncron'] = 'Calculate completion data'; -$string['taskcompletioncronregular'] = 'Calculate regular completion data'; -$string['taskcompletioncrondaily'] = 'Completion mark as started'; +$string['taskcompletionregular'] = 'Calculate regular completion data'; +$string['taskcompletiondaily'] = 'Completion mark as started'; $string['taskcontextcleanup'] = 'Cleanup contexts'; $string['taskcreatecontexts'] = 'Create missing contexts'; $string['taskdeletecachetext'] = 'Delete old text cache records'; diff --git a/lib/classes/task/completion_cron_daily_task.php b/lib/classes/task/completion_daily_task.php similarity index 92% rename from lib/classes/task/completion_cron_daily_task.php rename to lib/classes/task/completion_daily_task.php index fa014b1c888..a6597d85bfa 100644 --- a/lib/classes/task/completion_cron_daily_task.php +++ b/lib/classes/task/completion_daily_task.php @@ -28,7 +28,7 @@ namespace core\task; * @copyright 2013 onwards Martin Dougiamas http://dougiamas.com. * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. */ -class completion_cron_daily_task extends scheduled_task { +class completion_daily_task extends scheduled_task { /** * Get a descriptive name for this task (shown to admins). @@ -36,7 +36,7 @@ class completion_cron_daily_task extends scheduled_task { * @return string */ public function get_name() { - return get_string('taskcompletioncrondaily', 'admin'); + return get_string('taskcompletiondaily', 'admin'); } /** diff --git a/lib/classes/task/completion_cron_regular_task.php b/lib/classes/task/completion_regular_task.php similarity index 92% rename from lib/classes/task/completion_cron_regular_task.php rename to lib/classes/task/completion_regular_task.php index 215679c30f2..19be43f035a 100644 --- a/lib/classes/task/completion_cron_regular_task.php +++ b/lib/classes/task/completion_regular_task.php @@ -28,7 +28,7 @@ namespace core\task; * @copyright 2015 Josh Willcock * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. */ -class completion_cron_regular_task extends scheduled_task { +class completion_regular_task extends scheduled_task { /** * Get a descriptive name for this task (shown to admins). @@ -36,7 +36,7 @@ class completion_cron_regular_task extends scheduled_task { * @return string */ public function get_name() { - return get_string('taskcompletioncronregular', 'admin'); + return get_string('taskcompletionregular', 'admin'); } /** diff --git a/lib/db/tasks.php b/lib/db/tasks.php index 9954d5926a8..9723d334beb 100644 --- a/lib/db/tasks.php +++ b/lib/db/tasks.php @@ -159,7 +159,7 @@ $tasks = array( 'month' => '*' ), array( - 'classname' => 'core\task\completion_cron_regular_task', + 'classname' => 'core\task\completion_regular_task', 'blocking' => 0, 'minute' => '*', 'hour' => '*', @@ -168,7 +168,7 @@ $tasks = array( 'month' => '*' ), array( - 'classname' => 'core\task\completion_cron_daily_task', + 'classname' => 'core\task\completion_daily_task', 'blocking' => 0, 'minute' => 'R', 'hour' => 'R', From 88d350eab46b646fd96fa56dcdb41190670697f5 Mon Sep 17 00:00:00 2001 From: Dan Poltawski Date: Thu, 27 Aug 2015 14:51:14 +0100 Subject: [PATCH 4/5] MDL-50287 lang: remove unused string (I don't think this string warrants deprecation) --- lang/en/admin.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lang/en/admin.php b/lang/en/admin.php index 90e2c99ea63..9b4543ffa51 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -1019,7 +1019,6 @@ $string['taskcachecleanup'] = 'Remove expired cache entries'; $string['taskcachecron'] = 'Background processing for caches'; $string['taskcalendarcron'] = 'Send calendar notifications'; $string['taskcheckforupdates'] = 'Check for updates'; -$string['taskcompletioncron'] = 'Calculate completion data'; $string['taskcompletionregular'] = 'Calculate regular completion data'; $string['taskcompletiondaily'] = 'Completion mark as started'; $string['taskcontextcleanup'] = 'Cleanup contexts'; From 12fb029c67b91504c77b0507119a9407a7ed38e8 Mon Sep 17 00:00:00 2001 From: Dan Poltawski Date: Thu, 27 Aug 2015 14:41:38 +0100 Subject: [PATCH 5/5] MDL-50287 completion: version bump --- version.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.php b/version.php index 822072bd1ef..c562c0d231a 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2015082000.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2015082700.00; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes.