MDL-81000 core: Update "attemptsavailable" value and remove "MAX_RETRY"

This commit is contained in:
raortegar 2024-03-20 08:52:55 +01:00
parent 27a2e4969c
commit 8698a9a229
3 changed files with 37 additions and 16 deletions

View File

@ -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;
}

View File

@ -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();

View File

@ -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();