Skip to content

Commit

Permalink
Enhance task locks and events
Browse files Browse the repository at this point in the history
- Updates task locks (the `locked` column) so they now use a timestamp
  to allow for recovery from a fatal failure. Includes updates to SQL
  scripts.
- Dispatch a new onTaskRecoverFailure event on "recovery" from a fatal
  failure.
- Actually dispatch task exit events (oops!)
- Update the event class declaration for onExecuteTask to be more
  elegant, readable. [1]

[1]: http://#35143#discussion_r700897628 / @Denitz
  • Loading branch information
ditsuke committed Sep 5, 2021
1 parent 39e29c5 commit 7b945bf
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 13 deletions.
Expand Up @@ -18,7 +18,7 @@ CREATE TABLE IF NOT EXISTS `#__scheduler_tasks` (
`next_execution` datetime COMMENT 'Timestamp of next (planned) run, referred for execution on trigger',
`times_executed` int DEFAULT '0' COMMENT 'Count of successful triggers',
`times_failed` int DEFAULT '0' COMMENT 'Count of failures',
`locked` tinyint NOT NULL DEFAULT 0,
`locked` datetime,
`ordering` int NOT NULL DEFAULT 0 COMMENT 'Configurable list ordering',
`params` text NOT NULL,
`note` text,
Expand Down
Expand Up @@ -17,7 +17,7 @@ CREATE TABLE IF NOT EXISTS "#__scheduler_tasks"
"next_execution" timestamp without time zone,
"times_executed" int NOT NULL DEFAULT '0',
"times_failed" int DEFAULT '0',
"locked" smallint default 0 NOT NULL,
"locked" timestamp without time zone,
"ordering" bigint DEFAULT 0 NOT NULL,
"params" text NOT NULL,
"note" text DEFAULT '',
Expand Down
46 changes: 37 additions & 9 deletions administrator/components/com_scheduler/src/Task/Task.php
Expand Up @@ -16,6 +16,7 @@

use Assert\Assertion;
use Assert\AssertionFailedException;
use DateInterval;
use Exception;
use Joomla\CMS\Application\CMSApplication;
use Joomla\CMS\Event\AbstractEvent;
Expand Down Expand Up @@ -130,6 +131,20 @@ public function getRecord(): object
*/
public function run(): bool
{
/*
* Dispatch event if task lock is not released (NULL). This only happens if the task execution
* was interrupted by a fatal failure.
* @todo add listeners. We can have Email, PushBullet, etc
*/
if ($this->get('locked') === null)
{
$event = AbstractEvent::create('onTaskRecoverFailure', [
'subject' => $this
]
);
$this->app->getDispatcher()->dispatch($event);
}

if (!$this->acquireLock())
{
$this->snapshot['status'] = Status::NO_LOCK;
Expand All @@ -145,7 +160,7 @@ public function run(): bool
$event = AbstractEvent::create(
'onExecuteTask',
[
'eventClass' => 'Joomla\Component\Scheduler\Administrator\Event\ExecuteTaskEvent',
'eventClass' => ExecuteTaskEvent::class,
'subject' => $this,
'routineId' => $this->get('type'),
'langConstPrefix' => $this->get('taskOption')->langConstPrefix,
Expand Down Expand Up @@ -185,12 +200,24 @@ public function acquireLock(): bool
$db = $this->db;
$query = $db->getQuery(true);
$id = $this->get('id');
$now = Factory::getDate('now', 'GMT');

// @todo Get timeout in seconds from component configuration
$timeout = new DateInterval('PT100S');
$timeoutThreshold = $now->sub($timeout)->toSql();
$now = $now->toSql();

$query->update($db->qn('#__scheduler_tasks', 't'))
->set('t.locked = 1')
->set('t.locked = :now')
->where($db->qn('t.id') . ' = :taskId')
->where($db->qn('t.locked') . ' = 0')
->bind(':taskId', $id, ParameterType::INTEGER);
->extendWhere('AND', [
$db->qn('t.locked') . ' < :threshold',
$db->qn('t.locked') . 'IS NULL'],
'OR'
)
->bind(':taskId', $id, ParameterType::INTEGER)
->bind(':now', $now)
->bind(':threshold', $timeoutThreshold);

try
{
Expand All @@ -206,7 +233,7 @@ public function acquireLock(): bool
return false;
}

$this->set('locked', 1);
$this->set('locked', $now);

return true;
}
Expand All @@ -229,9 +256,9 @@ public function releaseLock(bool $update = true): bool
$id = $this->get('id');

$query->update($db->qn('#__scheduler_tasks', 't'))
->set('t.locked = 0')
->set('t.locked = NULL')
->where($db->qn('t.id') . ' = :taskId')
->where($db->qn('t.locked') . ' = 1')
->where($db->qn('t.locked') . ' IS NOT NULL')
->bind(':taskId', $id, ParameterType::INTEGER);

if ($update)
Expand Down Expand Up @@ -275,7 +302,7 @@ public function releaseLock(bool $update = true): bool
return false;
}

$this->set('locked', 0);
$this->set('locked', null);

return true;
}
Expand Down Expand Up @@ -306,10 +333,11 @@ private function handleExit(bool $success = true): bool
{
$eventName = $success ? 'onTaskExecuteSuccess' : 'onTaskExecuteFailure';

AbstractEvent::create($eventName, [
$event = AbstractEvent::create($eventName, [
'subject' => $this
]
);
$this->app->getDispatcher()->dispatch($event);

return $success;
}
Expand Down
2 changes: 1 addition & 1 deletion installation/sql/mysql/extensions.sql
Expand Up @@ -906,7 +906,7 @@ CREATE TABLE IF NOT EXISTS `#__scheduler_tasks` (
`next_execution` datetime COMMENT 'Timestamp of next (planned) run, referred for execution on trigger',
`times_executed` int DEFAULT '0' COMMENT 'Count of successful triggers',
`times_failed` int DEFAULT '0' COMMENT 'Count of failures',
`locked` tinyint NOT NULL DEFAULT 0,
`locked` datetime,
`ordering` int NOT NULL DEFAULT 0 COMMENT 'Configurable list ordering',
`params` text NOT NULL,
`note` text,
Expand Down
2 changes: 1 addition & 1 deletion installation/sql/postgresql/extensions.sql
Expand Up @@ -869,7 +869,7 @@ CREATE TABLE IF NOT EXISTS "#__scheduler_tasks"
"next_execution" timestamp without time zone,
"times_executed" int NOT NULL DEFAULT '0',
"times_failed" int DEFAULT '0',
"locked" smallint default 0 NOT NULL,
"locked" timestamp without time zone,
"ordering" bigint DEFAULT 0 NOT NULL,
"params" text NOT NULL,
"note" text DEFAULT '',
Expand Down

0 comments on commit 7b945bf

Please sign in to comment.