diff --git a/admin/cli/scheduled_task.php b/admin/cli/scheduled_task.php index 6c8144b45fb..d75b4f8e0fb 100644 --- a/admin/cli/scheduled_task.php +++ b/admin/cli/scheduled_task.php @@ -189,11 +189,7 @@ if ($disable = $options['disable']) { } $task->set_lock($lock); - if (!$task->is_blocking()) { - $cronlock->release(); - } else { - $task->set_cron_lock($cronlock); - } + $cronlock->release(); \core\cron::run_inner_scheduled_task($task); } diff --git a/auth/ldap/classes/task/asynchronous_sync_task.php b/auth/ldap/classes/task/asynchronous_sync_task.php index b36fb78ca6d..e4d03d0f338 100644 --- a/auth/ldap/classes/task/asynchronous_sync_task.php +++ b/auth/ldap/classes/task/asynchronous_sync_task.php @@ -34,7 +34,6 @@ use core\task\adhoc_task; * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class asynchronous_sync_task extends adhoc_task { - /** @var string Message prefix for mtrace */ protected const MTRACE_MSG = 'Synced ldap users'; @@ -42,7 +41,6 @@ class asynchronous_sync_task extends adhoc_task { * Constructor */ public function __construct() { - $this->set_blocking(false); $this->set_component('auth_ldap'); } diff --git a/backup/backup.php b/backup/backup.php index 4f2519572ad..b3f597fd9b9 100644 --- a/backup/backup.php +++ b/backup/backup.php @@ -203,7 +203,6 @@ if (!async_helper::is_async_pending($id, 'course', 'backup')) { // Create adhoc task for backup. $asynctask = new \core\task\asynchronous_backup_task(); - $asynctask->set_blocking(false); $asynctask->set_custom_data(array('backupid' => $backupid)); $asynctask->set_userid($USER->id); \core\task\manager::queue_adhoc_task($asynctask); diff --git a/backup/restore.php b/backup/restore.php index e6edbb4efbc..22f05c07d78 100644 --- a/backup/restore.php +++ b/backup/restore.php @@ -173,7 +173,6 @@ if ($restore->get_stage() != restore_ui::STAGE_PROCESS) { // Create adhoc task for restore. $restoreid = $restore->get_restoreid(); $asynctask = new \core\task\asynchronous_restore_task(); - $asynctask->set_blocking(false); $asynctask->set_userid($USER->id); $asynctask->set_custom_data(array('backupid' => $restoreid)); \core\task\manager::queue_adhoc_task($asynctask); diff --git a/backup/tests/async_backup_test.php b/backup/tests/async_backup_test.php index ebb5711ac69..03b412bb43d 100644 --- a/backup/tests/async_backup_test.php +++ b/backup/tests/async_backup_test.php @@ -105,7 +105,6 @@ class async_backup_test extends \advanced_testcase { // Create the adhoc task. $asynctask = new \core\task\asynchronous_backup_task(); - $asynctask->set_blocking(false); $asynctask->set_custom_data(['backupid' => $backupid]); $asynctask->set_userid($USER->id); \core\task\manager::queue_adhoc_task($asynctask); @@ -151,7 +150,6 @@ class async_backup_test extends \advanced_testcase { // Create the adhoc task. $asynctask = new \core\task\asynchronous_backup_task(); - $asynctask->set_blocking(false); $asynctask->set_custom_data(['backupid' => $backupid]); \core\task\manager::queue_adhoc_task($asynctask); @@ -242,7 +240,6 @@ class async_backup_test extends \advanced_testcase { // Now queue an adhoc task and check it handles and completes gracefully. $asynctask = new \core\task\asynchronous_backup_task(); - $asynctask->set_blocking(false); $asynctask->set_custom_data(array('backupid' => $backupid)); \core\task\manager::queue_adhoc_task($asynctask); diff --git a/backup/tests/async_restore_test.php b/backup/tests/async_restore_test.php index 01faf9a1731..f528f9b862d 100644 --- a/backup/tests/async_restore_test.php +++ b/backup/tests/async_restore_test.php @@ -116,7 +116,6 @@ class async_restore_test extends \advanced_testcase { // Create the adhoc task. $asynctask = new \core\task\asynchronous_restore_task(); - $asynctask->set_blocking(false); $asynctask->set_custom_data(array('backupid' => $restoreid)); $asynctask->set_userid($USER->id); \core\task\manager::queue_adhoc_task($asynctask); @@ -223,7 +222,6 @@ class async_restore_test extends \advanced_testcase { // Create the adhoc task. $asynctask = new \core\task\asynchronous_restore_task(); - $asynctask->set_blocking(false); $asynctask->set_custom_data(['backupid' => $restoreid]); \core\task\manager::queue_adhoc_task($asynctask); @@ -245,7 +243,6 @@ class async_restore_test extends \advanced_testcase { // Create the adhoc task. $asynctask = new \core\task\asynchronous_restore_task(); - $asynctask->set_blocking(false); $asynctask->set_custom_data(['backupid' => $restoreid]); \core\task\manager::queue_adhoc_task($asynctask); diff --git a/backup/util/helper/backup_cron_helper.class.php b/backup/util/helper/backup_cron_helper.class.php index ecd1617fa7e..3fdf7ac5770 100644 --- a/backup/util/helper/backup_cron_helper.class.php +++ b/backup/util/helper/backup_cron_helper.class.php @@ -378,7 +378,6 @@ abstract class backup_cron_automated_helper { global $DB; $asynctask = new \core\task\course_backup_task(); - $asynctask->set_blocking(false); $asynctask->set_custom_data(array( 'courseid' => $backupcourse->courseid, 'adminid' => $admin->id diff --git a/backup/util/helper/copy_helper.class.php b/backup/util/helper/copy_helper.class.php index 0bbd042424f..4cbd7d0d70a 100644 --- a/backup/util/helper/copy_helper.class.php +++ b/backup/util/helper/copy_helper.class.php @@ -96,7 +96,6 @@ final class copy_helper { // Create the ad-hoc task to perform the course copy. $asynctask = new \core\task\asynchronous_copy_task(); - $asynctask->set_blocking(false); $asynctask->set_custom_data($copyids); \core\task\manager::queue_adhoc_task($asynctask); diff --git a/backup/util/helper/tests/copy_helper_test.php b/backup/util/helper/tests/copy_helper_test.php index 1080fe56e66..7694343658b 100644 --- a/backup/util/helper/tests/copy_helper_test.php +++ b/backup/util/helper/tests/copy_helper_test.php @@ -333,7 +333,6 @@ class copy_helper_test extends \advanced_testcase { $this->assertInstanceOf('\\core\\task\\asynchronous_copy_task', $task); $this->assertEquals($result, (array)$task->get_custom_data()); - $this->assertFalse($task->is_blocking()); \core\task\manager::adhoc_task_complete($task); } diff --git a/cache/stores/file/tests/asyncpurge_test.php b/cache/stores/file/tests/asyncpurge_test.php index 6611aecf11e..79bb5dac3b5 100644 --- a/cache/stores/file/tests/asyncpurge_test.php +++ b/cache/stores/file/tests/asyncpurge_test.php @@ -86,7 +86,6 @@ class asyncpurge_test extends \advanced_testcase { // Create / execute adhoc task to delete cache revision directory. $asynctask = new cachestore_file\task\asyncpurge(); - $asynctask->set_blocking(false); $asynctask->set_custom_data(['path' => $cacherevdir]); $asynctask->set_userid($USER->id); \core\task\manager::queue_adhoc_task($asynctask); diff --git a/course/tests/backup/restore_test.php b/course/tests/backup/restore_test.php index 85bf9004d24..e364957740c 100644 --- a/course/tests/backup/restore_test.php +++ b/course/tests/backup/restore_test.php @@ -159,7 +159,6 @@ class restore_test extends \advanced_testcase { // Create the adhoc task. $asynctask = new \core\task\asynchronous_restore_task(); - $asynctask->set_blocking(false); $asynctask->set_custom_data(array('backupid' => $restoreid)); \core\task\manager::queue_adhoc_task($asynctask); diff --git a/lib/classes/task/manager.php b/lib/classes/task/manager.php index 8385172d9e0..5aba3f187c9 100644 --- a/lib/classes/task/manager.php +++ b/lib/classes/task/manager.php @@ -304,7 +304,6 @@ class manager { $record = new \stdClass(); $record->classname = self::get_canonical_class_name($task); $record->component = $task->get_component(); - $record->blocking = $task->is_blocking(); $record->customised = $task->is_customised(); $record->lastruntime = $task->get_last_run_time(); $record->nextruntime = $task->get_next_run_time(); @@ -333,7 +332,6 @@ class manager { $record->classname = self::get_canonical_class_name($task); $record->id = $task->get_id(); $record->component = $task->get_component(); - $record->blocking = $task->is_blocking(); $record->nextruntime = $task->get_next_run_time(); $record->faildelay = $task->get_fail_delay(); $record->customdata = $task->get_custom_data_as_string(); @@ -368,7 +366,6 @@ class manager { if (isset($record->component)) { $task->set_component($record->component); } - $task->set_blocking(!empty($record->blocking)); if (isset($record->faildelay)) { $task->set_fail_delay($record->faildelay); } @@ -430,7 +427,6 @@ class manager { if (isset($record->component)) { $task->set_component($record->component); } - $task->set_blocking(!empty($record->blocking)); if (isset($record->minute)) { $task->set_minute($record->minute, $expandr); } @@ -1040,11 +1036,7 @@ class manager { } $task->set_lock($lock); - if (!$task->is_blocking()) { - $cronlock->release(); - } else { - $task->set_cron_lock($cronlock); - } + $cronlock->release(); } /** @@ -1107,11 +1099,7 @@ class manager { throw new \moodle_exception('locktimeout'); } - if (!$task->is_blocking()) { - $cronlock->release(); - } else { - $task->set_cron_lock($cronlock); - } + $cronlock->release(); return $task; } } @@ -1198,9 +1186,6 @@ class manager { $DB->update_record('task_adhoc', $record); $task->release_concurrency_lock(); - if ($task->is_blocking()) { - $task->get_cron_lock()->release(); - } $task->get_lock()->release(); self::$runningtask = null; @@ -1259,9 +1244,6 @@ class manager { // Release the locks. $task->release_concurrency_lock(); - if ($task->is_blocking()) { - $task->get_cron_lock()->release(); - } $task->get_lock()->release(); self::$runningtask = null; @@ -1311,9 +1293,6 @@ class manager { $record->pid = null; $DB->update_record('task_scheduled', $record); - if ($task->is_blocking()) { - $task->get_cron_lock()->release(); - } $task->get_lock()->release(); self::$runningtask = null; @@ -1394,9 +1373,6 @@ class manager { } // Reschedule and then release the locks. - if ($task->is_blocking()) { - $task->get_cron_lock()->release(); - } $task->get_lock()->release(); self::$runningtask = null; diff --git a/lib/classes/task/task_base.php b/lib/classes/task/task_base.php index 1f3c0eed42f..8253389ef88 100644 --- a/lib/classes/task/task_base.php +++ b/lib/classes/task/task_base.php @@ -45,9 +45,6 @@ abstract class task_base { /** @var string $component - The component this task belongs to. */ private $component = ''; - /** @var bool $blocking - Does this task block the entire cron process. */ - private $blocking = false; - /** @var int $faildelay - Exponentially increasing fail delay */ private $faildelay = 0; @@ -120,10 +117,21 @@ abstract class task_base { /** * Setter for $blocking. - * @param bool $blocking + * + * Please note that task blocking is no longer supported. + * If you are using it in older versions of Moodle you are strongly advised to rewrite your code + * as has a detrimental impact upon performance. + * + * @deprecated since Moodle 4.4 See MDL-67667 + * @todo Remove in MDL-81509 */ + #[\core\attribute\deprecated( + replacement: null, + since: '4.4', + reason: 'Blocking tasks are no longer supported', + )] public function set_blocking($blocking) { - $this->blocking = $blocking; + \core\deprecation::emit_deprecation_if_present([$this, __FUNCTION__]); } /** @@ -131,16 +139,16 @@ abstract class task_base { * * @return bool * @deprecated since Moodle 4.4 See MDL-67667 + * @todo Remove in MDL-81509 */ #[\core\attribute\deprecated( replacement: null, since: '4.4', reason: 'Blocking tasks are no longer supported', - emit: false, )] public function is_blocking() { - \core\deprecation::emit_deprecation_if_present(([$this, __FUNCTION__]); - return $this->blocking; + \core\deprecation::emit_deprecation_if_present([$this, __FUNCTION__]); + return false; } /** diff --git a/lib/db/install.xml b/lib/db/install.xml index 41345e8e814..a2f52c5a996 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -1,5 +1,5 @@ - @@ -3475,7 +3475,6 @@ - @@ -3504,7 +3503,6 @@ - diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index c04f124cbb1..297e906c794 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -1144,5 +1144,28 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2024032600.01); } + if ($oldversion < 2024041200.00) { + // Define field blocking to be dropped from task_adhoc. + $table = new xmldb_table('task_adhoc'); + $field = new xmldb_field('blocking'); + + // Conditionally launch drop field customdata. + if ($dbman->field_exists($table, $field)) { + $dbman->drop_field($table, $field); + } + + // Define field blocking to be dropped from task_scheduled. + $table = new xmldb_table('task_scheduled'); + $field = new xmldb_field('blocking'); + + // Conditionally launch drop field customdata. + if ($dbman->field_exists($table, $field)) { + $dbman->drop_field($table, $field); + } + + // Main savepoint reached. + upgrade_main_savepoint(true, 2024041200.00); + } + return true; } diff --git a/lib/phpunit/classes/advanced_testcase.php b/lib/phpunit/classes/advanced_testcase.php index dca7c404c4f..2068ce650fd 100644 --- a/lib/phpunit/classes/advanced_testcase.php +++ b/lib/phpunit/classes/advanced_testcase.php @@ -705,11 +705,7 @@ abstract class advanced_testcase extends base_testcase { } $task->set_lock($lock); - if (!$task->is_blocking()) { - $cronlock->release(); - } else { - $task->set_cron_lock($cronlock); - } + $cronlock->release(); \core\cron::prepare_core_renderer(); \core\cron::setup_user($user); diff --git a/lib/tests/behat/behat_general.php b/lib/tests/behat/behat_general.php index a2468e1ca12..b38bd60956d 100644 --- a/lib/tests/behat/behat_general.php +++ b/lib/tests/behat/behat_general.php @@ -1176,11 +1176,7 @@ EOF; throw new DriverException('Unable to obtain task lock for scheduled task'); } $task->set_lock($lock); - if (!$task->is_blocking()) { - $cronlock->release(); - } else { - $task->set_cron_lock($cronlock); - } + $cronlock->release(); try { // Prepare the renderer. diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 09a2ec88eb5..9323bed124f 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -35,6 +35,9 @@ information provided here is intended especially for developers. By default, tasks will be retried until they succeed, other tasks can override this method to change this behaviour. - set_attempts_available(): Used to set the number of attempts available for the task - get_attempts_available(): Used to get the number of attempts available for the task. +* Support for blocking tasks has been removed. See MDL-67667 for further information. + Please note that this feature never worked correctly and can cause serious performance issues. + It is also not possible to deprecate this feature in a notifiable way. * There is a new DML method $DB->get_fieldset. For some reason, this did not exist even though get_fieldset_select etc. did. * The following callbacks have been migrated to hooks: - before_standard_html_head() -> core\hook\output\before_standard_head_html_generation diff --git a/version.php b/version.php index 57907b6b549..743f03c7b0c 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2024040900.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2024041200.00; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. $release = '4.4dev+ (Build: 20240409)'; // Human-friendly version name