-
Bug
-
Resolution: Fixed
-
Major
-
4.4
-
MOODLE_404_STABLE
-
MOODLE_404_STABLE
-
MDL-81000-main2 -
-
2
-
Team Hedgehog 2024 Sprint 1.3
There are a few problems with the current implementation of max fail delays:
- There was a maths error in the original issue requirements implementing the maximum attempts value (MAX_RETRY), so instead of the intended outcome of stopping retries after 24+ hours, it stops after just 4 hours. This causes some unnecessary complexity (or breaks the implementation) when trying to implement the new max fail notification (
MDL-79131), because the max retry time doesn't match the 24 hour notification requirement. - There are references to the null option being "unlimited retries", which was designed as a temporary measure while "no retry" was being implemented prior to a proper max fail delay being implemented, so should not still be usable.
- The combination of MAX_RETRY when setting a task's initial attempts available means we are bypassing the task object's own attemptsavailable property, which adds unnecessary complexity and limitations on that functionality (it locks tasks into two possible starting values, preventing tasks from properly using set_attempts_available()).
These issues can all be solved as follows:
- In adhoc_task.php, change the default value for the attemptsavailable property from null to 12 (which is the number of attempts required to hit the maximum 24 hour backoff), and in the docblock remove "or null for unlimited" as well as null as a data type (it should just be int).
- In the same file, update both the set_attempts_available and get_attempts_available so the imports/exports only allow int (not nullable), and update their docblocks to remove references to null being a valid value/meaning unlimited.
- In manager.php, change this:
$record->attemptsavailable = $task->retry_until_success() ? self::MAX_RETRY : 1;
To this:
$record->attemptsavailable = $task->retry_until_success() ? $record->attemptsavailable : 1;
- Completely remove all references to the MAX_RETRY constant (including removing the constant itself), because it is no longer needed.
The result of the above is that the correct number of retries are attempted, they are properly fetched from the task object (which removes the need for the constant), we now support (and require) a valid, finite number of retries to be defined, and we still ensure that if a task has defined that it's a "no retry" task, that will still override the number of attempts to 1.
Steps to reproduce:
Prerequisites.
Install the testing plugin moodle-tool_test_tasks.zip
It can be found attached to this issue, and it can be installed with the zip package from "site admin" / "Plugins" / "Install plugins"
Adhoc task - Available attempts
- Locate the file test_MDL-81000.phpin your root folder
- Execute it with the following command from root:
php test_MDL-82000.php
- Check these values are in the results
Initial values:
- Attempts available = 12 - Fail delay = 0
Run task:
- Attempts available = 11 - Fail delay = 60
- Attempts available = 10 - Fail delay = 120
- Attempts available = 9 - Fail delay = 240
- Attempts available = 8 - Fail delay = 480
- Attempts available = 7 - Fail delay = 960
- Attempts available = 6 - Fail delay = 1920
- Attempts available = 5 - Fail delay = 3840
- Attempts available = 4 - Fail delay = 7680
- Attempts available = 3 - Fail delay = 15360
- Attempts available = 2 - Fail delay = 30720
- Attempts available = 1 - Fail delay = 61440
- Attempts available = 0 - Fail delay = 86400
- blocks
-
MDL-71857 Add option to create new indexes for existing tables in an online fashion
- Reopened
-
MDL-79131 Tasks: Max fail delay admin notification
- Closed
- has been marked as being related by
-
MDL-79130 Tasks: Ad-hoc tasks shouldn't retry after max fail delay
- Closed
- is duplicated by
-
MDL-81201 Ad-hoc task maximum retries value set too low
- Closed