From a35335a9c43bd63b0f58bb79cfb26908a935b269 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Tue, 4 Mar 2025 15:46:49 +0700 Subject: [PATCH] MDL-84729 core: Use `core\clock` for Tasks This will help to solve the random failure on "test_adhoc_task_get_first_starting_time" --- lib/classes/task/manager.php | 53 ++++++++++++++----- lib/tests/task/adhoc_task_test.php | 84 ++++++++++++++++++++---------- 2 files changed, 97 insertions(+), 40 deletions(-) diff --git a/lib/classes/task/manager.php b/lib/classes/task/manager.php index 26625d20485..ac564611f17 100644 --- a/lib/classes/task/manager.php +++ b/lib/classes/task/manager.php @@ -244,6 +244,8 @@ class manager { public static function queue_adhoc_task(adhoc_task $task, $checkforexisting = false) { global $DB; + $clock = \core\di::get(\core\clock::class); + // Don't queue tasks for deprecated components. if (self::task_component_is_deprecated($task)) { return false; @@ -257,13 +259,13 @@ class manager { $record = self::record_from_adhoc_task($task); // Schedule it immediately if nextruntime not explicitly set. if (!$task->get_next_run_time()) { - $record->nextruntime = time() - 1; + $record->nextruntime = $clock->time() - 1; } // Check if the task is allowed to be retried or not. $record->attemptsavailable = $task->retry_until_success() ? $record->attemptsavailable : 1; // Set the time the task was created. - $record->timecreated = time(); + $record->timecreated = $clock->time(); // Check if the same task is already scheduled. if ($checkforexisting && self::task_is_scheduled($task)) { @@ -556,7 +558,9 @@ class manager { public static function get_adhoc_tasks_summary(): array { global $DB; - $now = time(); + $clock = \core\di::get(\core\clock::class); + + $now = $clock->time(); $records = $DB->get_records('task_adhoc'); $summary = []; foreach ($records as $r) { @@ -1127,6 +1131,9 @@ class manager { */ public static function adhoc_task_failed(adhoc_task $task) { global $DB; + + $clock = \core\di::get(\core\clock::class); + // Finalise the log output. logmanager::finalise_log(true); @@ -1154,7 +1161,7 @@ class manager { $task->set_timestarted(); $task->set_hostname(); $task->set_pid(); - $task->set_next_run_time(time() + $delay); + $task->set_next_run_time($clock->time() + $delay); $task->set_fail_delay($delay); if ($task->get_attempts_available() > 0) { $task->set_attempts_available($task->get_attempts_available() - 1); @@ -1182,7 +1189,8 @@ class manager { $hostname = (string)gethostname(); if (empty($time)) { - $time = time(); + $clock = \core\di::get(\core\clock::class); + $time = $clock->time(); } $task->set_timestarted($time); @@ -1233,6 +1241,9 @@ class manager { */ public static function scheduled_task_failed(scheduled_task $task) { global $DB; + + $clock = \core\di::get(\core\clock::class); + // Finalise the log output. logmanager::finalise_log(true); @@ -1263,7 +1274,7 @@ class manager { $classname = self::get_canonical_class_name($task); $record = $DB->get_record('task_scheduled', array('classname' => $classname)); - $record->nextruntime = time() + $delay; + $record->nextruntime = $clock->time() + $delay; $record->faildelay = $delay; $record->timestarted = null; $record->hostname = null; @@ -1301,11 +1312,14 @@ class manager { */ public static function scheduled_task_starting(scheduled_task $task, int $time = 0) { global $DB; + + $clock = \core\di::get(\core\clock::class); + $pid = (int)getmypid(); $hostname = (string)gethostname(); if (!$time) { - $time = time(); + $time = $clock->time(); } $task->set_timestarted($time); @@ -1330,6 +1344,8 @@ class manager { public static function scheduled_task_complete(scheduled_task $task) { global $DB; + $clock = \core\di::get(\core\clock::class); + // Finalise the log output. logmanager::finalise_log(); $task->set_timestarted(); @@ -1339,7 +1355,7 @@ class manager { $classname = self::get_canonical_class_name($task); $record = $DB->get_record('task_scheduled', array('classname' => $classname)); if ($record) { - $record->lastruntime = time(); + $record->lastruntime = $clock->time(); $record->faildelay = 0; $record->nextruntime = $task->get_next_scheduled_time(); $record->timestarted = null; @@ -1364,10 +1380,13 @@ class manager { */ public static function get_running_tasks($sort = ''): array { global $DB; + + $clock = \core\di::get(\core\clock::class); + if (empty($sort)) { $sort = 'timestarted ASC, classname ASC'; } - $params = ['now1' => time(), 'now2' => time()]; + $params = ['now1' => $clock->time(), 'now2' => $clock->time()]; $sql = "SELECT subquery.* FROM (SELECT " . $DB->sql_concat("'s'", 'ts.id') . " as uniqueid, @@ -1402,11 +1421,13 @@ class manager { public static function cleanup_metadata() { global $DB; + $clock = \core\di::get(\core\clock::class); + $cronlockfactory = \core\lock\lock_config::get_lock_factory('cron'); $runningtasks = self::get_running_tasks(); foreach ($runningtasks as $runningtask) { - if ($runningtask->timestarted > time() - HOURSECS) { + if ($runningtask->timestarted > $clock->time() - HOURSECS) { continue; } @@ -1468,15 +1489,18 @@ class manager { */ public static function clear_static_caches() { global $DB; + + $clock = \core\di::get(\core\clock::class); + // Do not use get/set config here because the caches cannot be relied on. $record = $DB->get_record('config', array('name'=>'scheduledtaskreset')); if ($record) { - $record->value = time(); + $record->value = $clock->time(); $DB->update_record('config', $record); } else { $record = new \stdClass(); $record->name = 'scheduledtaskreset'; - $record->value = time(); + $record->value = $clock->time(); $DB->insert_record('config', $record); } } @@ -1776,12 +1800,15 @@ class manager { */ public static function clean_failed_adhoc_tasks(): void { global $CFG, $DB; + + $clock = \core\di::get(\core\clock::class); + $difftime = !empty($CFG->task_adhoc_failed_retention) ? $CFG->task_adhoc_failed_retention : static::ADHOC_TASK_FAILED_RETENTION; $DB->delete_records_select( table: 'task_adhoc', select: 'attemptsavailable = 0 AND firststartingtime < :time', - params: ['time' => time() - $difftime], + params: ['time' => $clock->time() - $difftime], ); } } diff --git a/lib/tests/task/adhoc_task_test.php b/lib/tests/task/adhoc_task_test.php index df0cd83c962..4cc320dc5c1 100644 --- a/lib/tests/task/adhoc_task_test.php +++ b/lib/tests/task/adhoc_task_test.php @@ -60,15 +60,16 @@ final class adhoc_task_test extends \advanced_testcase { public function test_get_next_adhoc_task_now(): void { $this->resetAfterTest(true); + $clock = \core\di::get(\core\clock::class); + // Create an adhoc task. $task = new adhoc_test_task(); // Queue it. manager::queue_adhoc_task($task); - $now = time(); // Get it from the scheduler. - $task = manager::get_next_adhoc_task($now); + $task = manager::get_next_adhoc_task($clock->time()); $this->assertInstanceOf('\\core\\task\\adhoc_test_task', $task); $task->execute(); manager::adhoc_task_complete($task); @@ -80,20 +81,21 @@ final class adhoc_task_test extends \advanced_testcase { public function test_get_next_adhoc_task_class(): void { $this->resetAfterTest(true); + $clock = $this->mock_clock_with_frozen(); + // Create an adhoc task. $task = new \core\task\adhoc_test_task(); // Queue it. manager::queue_adhoc_task($task); - $now = time(); $classname = get_class($task); // The task will not be returned. - $this->assertNull(manager::get_next_adhoc_task($now, true, "{$classname}notexists")); + $this->assertNull(manager::get_next_adhoc_task($clock->time(), true, "{$classname}notexists")); // Get it from the scheduler. - $task = manager::get_next_adhoc_task($now, true, $classname); + $task = manager::get_next_adhoc_task($clock->time(), true, $classname); $this->assertInstanceOf('\\core\\task\\adhoc_test_task', $task); $task->execute(); manager::adhoc_task_complete($task); @@ -105,11 +107,13 @@ final class adhoc_task_test extends \advanced_testcase { public function test_get_next_adhoc_task_fail_retry(): void { $this->resetAfterTest(true); + $clock = $this->mock_clock_with_frozen(); + // Create an adhoc task. $task = new adhoc_test_task(); manager::queue_adhoc_task($task); - $now = time(); + $now = $clock->time(); // Get it from the scheduler, execute it, and mark it as failed. $task = manager::get_next_adhoc_task($now); @@ -121,7 +125,8 @@ final class adhoc_task_test extends \advanced_testcase { $this->assertNull(manager::get_next_adhoc_task($now)); // Should get the adhoc task (retry after delay). Fail it again. - $task = manager::get_next_adhoc_task($now + 120); + $clock->bump(120); + $task = manager::get_next_adhoc_task($clock->time()); $this->assertInstanceOf('\\core\\task\\adhoc_test_task', $task); $this->assertEquals($taskid, $task->get_id()); $task->execute(); @@ -146,7 +151,8 @@ final class adhoc_task_test extends \advanced_testcase { public function test_get_next_adhoc_task_maximum_fail_delay(): void { $this->resetAfterTest(true); - $now = time(); + $clock = \core\di::get(\core\clock::class); + $now = $clock->time(); // Create an adhoc task. $task = new adhoc_test_task(); @@ -171,7 +177,9 @@ final class adhoc_task_test extends \advanced_testcase { global $DB; $this->resetAfterTest(); - $now = time(); + $clock = \core\di::get(\core\clock::class); + + $now = $clock->time(); // Create a normal adhoc task. $task = new adhoc_test_task(); $taskid1 = manager::queue_adhoc_task(task: $task); @@ -199,7 +207,7 @@ final class adhoc_task_test extends \advanced_testcase { $this->assertEquals(expected: 12 - 1, actual: $attemptsavailable); // Create a no-retry adhoc task. - $now = time(); + $now = $clock->time(); $task = new no_retry_adhoc_task(); $taskid2 = manager::queue_adhoc_task(task: $task); @@ -270,6 +278,8 @@ final class adhoc_task_test extends \advanced_testcase { global $DB, $CFG; $this->resetAfterTest(); + $clock = \core\di::get(\core\clock::class); + // Create two no-retry adhoc tasks. $task1 = new no_retry_adhoc_task(); $taskid1 = manager::queue_adhoc_task(task: $task1); @@ -319,7 +329,7 @@ final class adhoc_task_test extends \advanced_testcase { $DB->set_field( table: 'task_adhoc', newfield: 'firststartingtime', - newvalue: time() - (DAYSECS * 2) - 10, // Plus 10 seconds to make sure it is older than 2 days. + newvalue: $clock->time() - (DAYSECS * 2) - 10, // Plus 10 seconds to make sure it is older than 2 days. conditions: ['id' => $taskid2], ); @@ -337,7 +347,7 @@ final class adhoc_task_test extends \advanced_testcase { table: 'task_adhoc', newfield: 'firststartingtime', // Plus 10 seconds to make sure it is older than the retention time. - newvalue: time() - $CFG->task_adhoc_failed_retention - 10, + newvalue: $clock->time() - $CFG->task_adhoc_failed_retention - 10, conditions: ['id' => $taskid1], ); @@ -370,7 +380,9 @@ final class adhoc_task_test extends \advanced_testcase { global $DB; $this->resetAfterTest(); - $now = time(); + $clock = \core\di::get(\core\clock::class); + + $now = $clock->time(); // Create an adhoc task. $task = new adhoc_test_task(); // Queue it. @@ -380,7 +392,7 @@ final class adhoc_task_test extends \advanced_testcase { $DB->set_field( table: 'task_adhoc', newfield: 'timecreated', - newvalue: time() - DAYSECS, + newvalue: $clock->time() - DAYSECS, conditions: ['id' => $taskid], ); @@ -415,7 +427,9 @@ final class adhoc_task_test extends \advanced_testcase { public function test_get_next_adhoc_task_future(): void { $this->resetAfterTest(true); - $now = time(); + $clock = \core\di::get(\core\clock::class); + + $now = $clock->time(); // Create an adhoc task in future. $task = new adhoc_test_task(); $task->set_next_run_time($now + 1000); @@ -576,10 +590,12 @@ final class adhoc_task_test extends \advanced_testcase { public function test_reschedule_or_queue_adhoc_task_match_no_change(): void { $this->resetAfterTest(true); + $clock = \core\di::get(\core\clock::class); + // Schedule adhoc task. $task = new adhoc_test_task(); $task->set_custom_data(['courseid' => 10]); - $task->set_next_run_time(time() + DAYSECS); + $task->set_next_run_time($clock->time() + DAYSECS); manager::reschedule_or_queue_adhoc_task($task); $before = manager::get_adhoc_tasks('core\task\adhoc_test_task'); @@ -598,8 +614,11 @@ final class adhoc_task_test extends \advanced_testcase { */ public function test_reschedule_or_queue_adhoc_task_match_update_runtime(): void { $this->resetAfterTest(true); - $initialruntime = time() + DAYSECS; - $newruntime = time() + WEEKSECS; + + $clock = \core\di::get(\core\clock::class); + + $initialruntime = $clock->time() + DAYSECS; + $newruntime = $clock->time() + WEEKSECS; // Schedule adhoc task. $task = new adhoc_test_task(); @@ -695,12 +714,14 @@ final class adhoc_task_test extends \advanced_testcase { public function test_adhoc_task_user_empty(): void { $this->resetAfterTest(true); + $clock = \core\di::get(\core\clock::class); + // Create an adhoc task in future. $task = new adhoc_test_task(); manager::queue_adhoc_task($task); // Get it back from the scheduler. - $now = time(); + $now = $clock->time(); $task = manager::get_next_adhoc_task($now); manager::adhoc_task_complete($task); @@ -723,7 +744,8 @@ final class adhoc_task_test extends \advanced_testcase { manager::queue_adhoc_task($task); // Get it back from the scheduler. - $now = time(); + $clock = \core\di::get(\core\clock::class); + $now = $clock->time(); $task = manager::get_next_adhoc_task($now); manager::adhoc_task_complete($task); @@ -737,7 +759,8 @@ final class adhoc_task_test extends \advanced_testcase { global $DB; $this->resetAfterTest(true); - $now = time(); + $clock = $this->mock_clock_with_frozen(); + $now = $clock->time(); // Create an adhoc task. $task = new adhoc_test_task(); @@ -753,7 +776,7 @@ final class adhoc_task_test extends \advanced_testcase { $this->assertNull($firststartingtime); // This will make sure that the task will be started after the $now value. - sleep(3); + $clock->bump(5); // Get the task from the scheduler. $task = manager::get_next_adhoc_task(timestart: $now); @@ -773,8 +796,11 @@ final class adhoc_task_test extends \advanced_testcase { $this->assertNotNull($origintimestarted); $this->assertGreaterThan($now, $origintimestarted); + // Time travel 24 hours into the future. + $clock->bump(DAYSECS * 3); + $now = $clock->time(); // Get the task from the scheduler. - $task = manager::get_next_adhoc_task(timestart: $now + 86400); + $task = manager::get_next_adhoc_task(timestart: $now); // Mark the task as starting. manager::adhoc_task_starting($task); // Execute the task. @@ -837,6 +863,8 @@ final class adhoc_task_test extends \advanced_testcase { public function test_get_next_adhoc_task_sorting(): void { $this->resetAfterTest(true); + $clock = \core\di::get(\core\clock::class); + // Create adhoc tasks. $task1 = new adhoc_test_task(); $task1->set_next_run_time(1510000000); @@ -864,15 +892,15 @@ final class adhoc_task_test extends \advanced_testcase { manager::reschedule_or_queue_adhoc_task($task2); // Confirm, that tasks are sorted by nextruntime and then by id (ascending). - $task = manager::get_next_adhoc_task(time()); + $task = manager::get_next_adhoc_task($clock->time()); $this->assertEquals('Task 2', $task->get_custom_data_as_string()); manager::adhoc_task_complete($task); - $task = manager::get_next_adhoc_task(time()); + $task = manager::get_next_adhoc_task($clock->time()); $this->assertEquals('Task 3', $task->get_custom_data_as_string()); manager::adhoc_task_complete($task); - $task = manager::get_next_adhoc_task(time()); + $task = manager::get_next_adhoc_task($clock->time()); $this->assertEquals('Task 1', $task->get_custom_data_as_string()); manager::adhoc_task_complete($task); } @@ -932,6 +960,8 @@ final class adhoc_task_test extends \advanced_testcase { $this->resetAfterTest(); $this->setAdminUser(); + $clock = \core\di::get(\core\clock::class); + // Redirect messages. $messagesink = $this->redirectMessages(); @@ -939,7 +969,7 @@ final class adhoc_task_test extends \advanced_testcase { $task = new adhoc_test_task(); manager::queue_adhoc_task($task); - $now = time(); + $now = $clock->time(); // Get it from the scheduler, execute it, and mark it as failed. $task = manager::get_next_adhoc_task($now); -- 2.43.0