From 27a2e4969cee9f345e6f7de3608c1fc773c7df53 Mon Sep 17 00:00:00 2001 From: raortegar Date: Wed, 20 Mar 2024 09:24:10 +0100 Subject: [PATCH 1/2] MDL-81000 core: increase size of "attemptsavailable" on task_adhoc table --- lib/db/install.xml | 4 ++-- lib/db/upgrade.php | 17 ++++++++++++++++- version.php | 2 +- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/lib/db/install.xml b/lib/db/install.xml index 802a5f592b0a1..41345e8e814fe 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -1,5 +1,5 @@ - @@ -3509,7 +3509,7 @@ - + diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 13bd8b7e6dd97..c04f124cbb1f9 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -870,7 +870,7 @@ function xmldb_main_upgrade($oldversion) { $field = new xmldb_field( name: 'attemptsavailable', type: XMLDB_TYPE_INTEGER, - precision: '1', + precision: '2', unsigned: null, notnull: null, sequence: null, @@ -1129,5 +1129,20 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2024030500.02); } + if ($oldversion < 2024032600.01) { + + // Changing precision of field attemptsavailable on table task_adhoc to (2). + $table = new xmldb_table('task_adhoc'); + $field = new xmldb_field('attemptsavailable', XMLDB_TYPE_INTEGER, '2', null, null, null, null, 'pid'); + + // Launch change of precision for field. + if (!$dbman->field_exists($table, $field)) { + $dbman->change_field_precision($table, $field); + } + + // Main savepoint reached. + upgrade_main_savepoint(true, 2024032600.01); + } + return true; } diff --git a/version.php b/version.php index 91e89cdda3ae2..1b45fa5e76137 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2024032600.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2024032600.01; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. $release = '4.4dev+ (Build: 20240326)'; // Human-friendly version name From 8698a9a2293c29e66885aaabd8b40df76a5d3b1e Mon Sep 17 00:00:00 2001 From: raortegar Date: Wed, 20 Mar 2024 08:52:55 +0100 Subject: [PATCH 2/2] MDL-81000 core: Update "attemptsavailable" value and remove "MAX_RETRY" --- lib/classes/task/adhoc_task.php | 12 +++++------ lib/classes/task/manager.php | 7 +----- lib/tests/task/adhoc_task_test.php | 34 ++++++++++++++++++++++++++---- 3 files changed, 37 insertions(+), 16 deletions(-) diff --git a/lib/classes/task/adhoc_task.php b/lib/classes/task/adhoc_task.php index 69c7db08ca956..d537dd9f76e44 100644 --- a/lib/classes/task/adhoc_task.php +++ b/lib/classes/task/adhoc_task.php @@ -46,8 +46,8 @@ abstract class adhoc_task extends task_base { /** @var \core\lock\lock The concurrency task lock for this task. */ private $concurrencylock = null; - /** @var integer|null $attemptsavailable - The remaining attempts of the task, or null for unlimited. */ - private $attemptsavailable = null; + /** @var int $attemptsavailable - The remaining attempts of the task. */ + private $attemptsavailable = 12; /** * Provide default implementation of the task name for backward compatibility. Extending classes are expected to implement @@ -178,18 +178,18 @@ final public function release_concurrency_lock(): void { /** * Set the remaining attempts of the task. * - * @param int|null $attemptsavailable Number of the remaining attempts of the task, or null for unlimited. + * @param int $attemptsavailable Number of the remaining attempts of the task. */ - public function set_attempts_available(?int $attemptsavailable): void { + public function set_attempts_available(int $attemptsavailable): void { $this->attemptsavailable = $attemptsavailable; } /** * Get the remaining attempts of the task. * - * @return int|null Number of the remaining attempts of the task, or null for unlimited. + * @return int Number of the remaining attempts of the task. */ - public function get_attempts_available(): ?int { + public function get_attempts_available(): int { return $this->attemptsavailable; } diff --git a/lib/classes/task/manager.php b/lib/classes/task/manager.php index 7d70d8014ea56..1a036c814bf53 100644 --- a/lib/classes/task/manager.php +++ b/lib/classes/task/manager.php @@ -56,11 +56,6 @@ class manager { */ const ADHOC_TASK_FAILED_RETENTION = 4 * WEEKSECS; - /** - * @var int Used for max retry. - */ - const MAX_RETRY = 9; - /** * @var ?task_base $runningtask Used to tell what is the current running task in this process. */ @@ -261,7 +256,7 @@ public static function queue_adhoc_task(adhoc_task $task, $checkforexisting = fa } // Check if the task is allowed to be retried or not. - $record->attemptsavailable = $task->retry_until_success() ? self::MAX_RETRY : 1; + $record->attemptsavailable = $task->retry_until_success() ? $record->attemptsavailable : 1; // Set the time the task was created. $record->timecreated = time(); diff --git a/lib/tests/task/adhoc_task_test.php b/lib/tests/task/adhoc_task_test.php index fdef01e05f47c..f3e93da3549bf 100644 --- a/lib/tests/task/adhoc_task_test.php +++ b/lib/tests/task/adhoc_task_test.php @@ -136,6 +136,32 @@ public function test_get_next_adhoc_task_fail_retry(): void { $this->assertNull(manager::get_next_adhoc_task($now)); } + /** + * Test that failed tasks eventually hit the maximum delay. + * + * @covers \core\task\adhoc_task + */ + public function test_get_next_adhoc_task_maximum_fail_delay(): void { + $this->resetAfterTest(true); + + $now = time(); + + // Create an adhoc task. + $task = new adhoc_test_task(); + $attemptsavailable = $task->get_attempts_available(); + manager::queue_adhoc_task($task); + + // Exhaust all attempts available. + for ($x = 0; $x < $attemptsavailable; $x++) { + $delay = $task->get_fail_delay() * 2; + $task = manager::get_next_adhoc_task($now + $delay); + $task->execute(); + manager::adhoc_task_failed($task); + } + // Check that the fail delay is now set to 24 hours (the maximum amount of times). + $this->assertEquals(DAYSECS, $task->get_fail_delay()); + } + /** * Test adhoc task failure retry backoff. */ @@ -148,13 +174,13 @@ public function test_adhoc_task_with_retry_flag(): void { $task = new adhoc_test_task(); $taskid1 = manager::queue_adhoc_task(task: $task); - // This is a normal task, so it should have unlimited attempts. The remaining available attempts should be null. + // This is a normal task, so it should have limited attempts. $attemptsavailable = $DB->get_field( table: 'task_adhoc', return: 'attemptsavailable', conditions: ['id' => $taskid1] ); - $this->assertEquals(expected: manager::MAX_RETRY, actual: $attemptsavailable); + $this->assertEquals(expected: 12, actual: $attemptsavailable); // Get the task from the scheduler, execute it, and mark it as failed. $task = manager::get_next_adhoc_task(timestart: $now); @@ -162,13 +188,13 @@ public function test_adhoc_task_with_retry_flag(): void { $task->execute(); manager::adhoc_task_failed(task: $task); - // This is a normal task, so it should have unlimited attempts. The remaining available attempts should be null. + // Now that the task has failed, there should be one less attempt available. $attemptsavailable = $DB->get_field( table: 'task_adhoc', return: 'attemptsavailable', conditions: ['id' => $taskid1] ); - $this->assertEquals(expected: manager::MAX_RETRY - 1, actual: $attemptsavailable); + $this->assertEquals(expected: 12 - 1, actual: $attemptsavailable); // Create a no-retry adhoc task. $now = time();