From 6c4bf9098e14cbd318078fda70faad21e3ee1723 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 9bc136430ba..97741f6ae01 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); + if ($userid = $task->get_userid()) { // User found. Check that they are suitable. \core_user::require_active_user(\core_user::get_user($userid, '*', MUST_EXIST), true, true); @@ -252,13 +254,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)) { @@ -543,7 +545,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) { @@ -1153,6 +1157,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); @@ -1180,7 +1187,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); @@ -1208,7 +1215,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); @@ -1259,6 +1267,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); @@ -1289,7 +1300,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; @@ -1327,11 +1338,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); @@ -1356,6 +1370,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(); @@ -1365,7 +1381,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; @@ -1390,10 +1406,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, @@ -1428,11 +1447,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; } @@ -1494,15 +1515,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); } } @@ -1802,12 +1826,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 067e6c85b59..7e718794683 100644 --- a/lib/tests/task/adhoc_task_test.php +++ b/lib/tests/task/adhoc_task_test.php @@ -58,15 +58,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); @@ -78,20 +79,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); @@ -103,11 +105,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); @@ -119,7 +123,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(); @@ -144,7 +149,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(); @@ -169,7 +175,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); @@ -197,7 +205,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); @@ -268,6 +276,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); @@ -317,7 +327,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], ); @@ -335,7 +345,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], ); @@ -368,7 +378,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. @@ -378,7 +390,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], ); @@ -413,7 +425,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); @@ -571,10 +585,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'); @@ -593,8 +609,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(); @@ -690,12 +709,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); @@ -718,7 +739,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); @@ -732,7 +754,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(); @@ -748,7 +771,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); @@ -768,8 +791,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. @@ -832,6 +858,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); @@ -859,15 +887,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); } @@ -928,6 +956,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(); @@ -935,7 +965,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);