diff --git a/lib/classes/task/adhoc_task.php b/lib/classes/task/adhoc_task.php index 69c7db08ca9..d537dd9f76e 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 @@ abstract class adhoc_task extends task_base { /** * 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 7d70d8014ea..1a036c814bf 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 @@ class manager { } // 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 fdef01e05f4..f3e93da3549 100644 --- a/lib/tests/task/adhoc_task_test.php +++ b/lib/tests/task/adhoc_task_test.php @@ -136,6 +136,32 @@ final class adhoc_task_test extends \advanced_testcase { $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 @@ final class adhoc_task_test extends \advanced_testcase { $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 @@ final class adhoc_task_test extends \advanced_testcase { $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();