diff options
author | Ben Burgess <88810029+bx80@users.noreply.github.com> | 2022-02-02 02:19:23 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-02-02 02:19:23 +0300 |
commit | f20c4cbed3de7f3fcc49810d5721b257d5a56d43 (patch) | |
tree | da362660970e784654bf9f5ec1d4a55755ed1322 /core | |
parent | cbb297de4aed974716dd03acfee8832565ebf133 (diff) |
Retry scheduled tasks on failure (#18335)
* Catch and log errors for scheduled tasks
* Added retry on exception mechanism for scheduled tasks
* Replace error code with custom exception
* Fix/workaround for SchedulerTest mock breaking retry list option loading
* Log warning instead of info if a task has failed three times
* Added basic tests, minor code improvements
* Test fix
* Fix for test
* Added integration test to check that only tasks that throw exceptions are scheduled for retry
Diffstat (limited to 'core')
-rw-r--r-- | core/Scheduler/RetryableException.php | 9 | ||||
-rw-r--r-- | core/Scheduler/Schedule/SpecificTime.php | 6 | ||||
-rw-r--r-- | core/Scheduler/Scheduler.php | 44 | ||||
-rw-r--r-- | core/Scheduler/Timetable.php | 88 |
4 files changed, 145 insertions, 2 deletions
diff --git a/core/Scheduler/RetryableException.php b/core/Scheduler/RetryableException.php new file mode 100644 index 0000000000..70650089a8 --- /dev/null +++ b/core/Scheduler/RetryableException.php @@ -0,0 +1,9 @@ +<?php + +namespace Piwik\Scheduler; + +use Piwik\Exception\Exception; + +class RetryableException extends Exception +{ +} diff --git a/core/Scheduler/Schedule/SpecificTime.php b/core/Scheduler/Schedule/SpecificTime.php index bc4c5bc66e..1d5263ec7c 100644 --- a/core/Scheduler/Schedule/SpecificTime.php +++ b/core/Scheduler/Schedule/SpecificTime.php @@ -30,4 +30,10 @@ class SpecificTime extends Schedule { throw new \Exception('not supported'); } + + public function setScheduledTime($scheduledTime) + { + $this->scheduledTime = $scheduledTime; + } + }
\ No newline at end of file diff --git a/core/Scheduler/Scheduler.php b/core/Scheduler/Scheduler.php index b126ccf7b2..0d7b8296bb 100644 --- a/core/Scheduler/Scheduler.php +++ b/core/Scheduler/Scheduler.php @@ -8,7 +8,6 @@ namespace Piwik\Scheduler; -use Exception; use Piwik\Piwik; use Piwik\Timer; use Psr\Log\LoggerInterface; @@ -55,6 +54,12 @@ class Scheduler private $isRunningTask = false; /** + * Should the last run task be scheduled for a retry + * @var bool + */ + private $scheduleRetry = false; + + /** * @var Timetable */ private $timetable; @@ -145,8 +150,36 @@ class Scheduler if ($shouldExecuteTask) { $readFromOption = true; + $this->scheduleRetry = false; $message = $this->executeTask($task); + // Task has thrown an exception and should be scheduled for a retry + if ($this->scheduleRetry) { + + if($this->timetable->getRetryCount($task->getName()) == 3) { + + // Task has already been retried three times, give up + $this->timetable->clearRetryCount($task->getName()); + + $this->logger->warning("Scheduler: '{task}' has already been retried three times, giving up", + ['task' => $task->getName()]); + + } else { + + $readFromOption = true; + $rescheduledDate = $this->timetable->rescheduleTaskAndRunInOneHour($task); + $this->timetable->incrementRetryCount($task->getName()); + + $this->logger->info("Scheduler: '{task}' retry scheduled for {date}", + ['task' => $task->getName(), 'date' => $rescheduledDate]); + } + $this->scheduleRetry = false; + } else { + if ($this->timetable->getRetryCount($task->getName()) > 0) { + $this->timetable->clearRetryCount($task->getName()); + } + } + $executionResults[] = array('task' => $taskName, 'output' => $message); } } @@ -275,8 +308,15 @@ class Scheduler $callable = array($task->getObjectInstance(), $task->getMethodName()); call_user_func($callable, $task->getMethodParameter()); $message = $timer->__toString(); - } catch (Exception $e) { + } catch (\Exception $e) { + $this->logger->error("Scheduler: Error {errorMessage} for task '{task}'", + ['errorMessage' => $e->getMessage(), 'task' => $task->getName()]); $message = 'ERROR: ' . $e->getMessage(); + + // If the task has indicated that retrying on exception is safe then flag for rescheduling + if ($e instanceof RetryableException) { + $this->scheduleRetry = true; + } } $this->isRunningTask = false; diff --git a/core/Scheduler/Timetable.php b/core/Scheduler/Timetable.php index 47e6b094d7..9073b73dc6 100644 --- a/core/Scheduler/Timetable.php +++ b/core/Scheduler/Timetable.php @@ -19,8 +19,10 @@ use Piwik\Date; class Timetable { const TIMETABLE_OPTION_STRING = "TaskScheduler.timetable"; + const RETRY_OPTION_STRING = "TaskScheduler.retryList"; private $timetable; + private $retryList; public function __construct() { @@ -37,6 +39,11 @@ class Timetable $this->timetable = $timetable; } + public function setRetryList($retryList) + { + $this->retryList = $retryList; + } + /** * @param Task[] $activeTasks */ @@ -124,6 +131,17 @@ class Timetable return $tomorrow; } + public function rescheduleTaskAndRunInOneHour(Task $task) + { + $oneHourFromNow = Date::factory('now')->addHour(1); + + // update the scheduled time + $this->timetable[$task->getName()] = $oneHourFromNow->getTimestamp(); + $this->save(); + + return $oneHourFromNow; + } + public function save() { Option::set(self::TIMETABLE_OPTION_STRING, serialize($this->timetable)); @@ -149,4 +167,74 @@ class Timetable $this->timetable = $unserializedTimetable === false ? array() : $unserializedTimetable; } + + /** + * Read the retry list option from the database + * + * @throws \Throwable + */ + private function readRetryList() + { + Option::clearCachedOption(self::RETRY_OPTION_STRING); + $retryData = Option::get(self::RETRY_OPTION_STRING); + $unserializedRetryList = Common::safe_unserialize($retryData); + + $this->retryList = $unserializedRetryList === false ? array() : $unserializedRetryList; + } + + /** + * Save the retry list option to the database + */ + public function saveRetryList() + { + Option::set(self::RETRY_OPTION_STRING, serialize($this->retryList)); + } + + /** + * Remove a task from the retry list + * + * @param string $taskName + */ + public function clearRetryCount(string $taskName) + { + if (isset($this->retryList[$taskName])) { + unset($this->retryList[$taskName]); + $this->saveRetryList(); + } + } + + /** + * Increment the retry counter for a task + * + * @param string $taskName + */ + public function incrementRetryCount(string $taskName) + { + $this->readRetryList(); + if (!isset($this->retryList[$taskName])) { + $this->retryList[$taskName] = 0; + } + $this->retryList[$taskName]++; + $this->saveRetryList(); + } + + /** + * Return the current number of retries for a task + * + * @param string $taskName + * + * @return int + */ + public function getRetryCount(string $taskName) : int + { + $this->readRetryList(); + + // Ignore excessive retry counts, workaround for SchedulerTest mock + if (!isset($this->retryList[$taskName]) || $this->retryList[$taskName] > 10000) { + return 0; + } + + return $this->retryList[$taskName]; + } + } |