Skip to content

Commit cadac72

Browse files
authored
Fix job behavior during summer and winter time change (#2954)
ref/IP/52977 `ts_last_attempt`, which is used in the callback function of `React\EventLoop\LoopInterface::addPeriodicTimer`, had data type timestamp and was saved as UTC time but retrieved as system time, and during DST where the clock is moved forward or set back during DST, you could expect time jumps in the retrieved `ts_last_attempt` value. And the `React\EventLoop\LoopInterface::addPeriodicTimer` expects the time source to be monotonous without any such time jumps. This causes the jobs to be erratically scheduled after DST. And this happens mostly when the `ts_last_attempt` saved time time falls in the skipped or time range that was reset. To avoid such time jumps the `ts_last_attempt` datatype is changed to `BIGINT` and the values are saved as Unix time stamp.
2 parents 5f4ad0f + 4c283a2 commit cadac72

File tree

7 files changed

+54
-15
lines changed

7 files changed

+54
-15
lines changed

library/Director/Objects/DirectorJob.php

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace Icinga\Module\Director\Objects;
44

55
use Icinga\Exception\NotFoundError;
6+
use Icinga\Module\Director\Daemon\DaemonUtil;
67
use Icinga\Module\Director\Daemon\Logger;
78
use Icinga\Module\Director\Data\Db\DbObjectWithSettings;
89
use Icinga\Module\Director\Db;
@@ -84,15 +85,16 @@ public function getInstance()
8485
public function run()
8586
{
8687
$job = $this->getInstance();
87-
$this->set('ts_last_attempt', date('Y-m-d H:i:s'));
88+
$currentTimestamp = DaemonUtil::timestampWithMilliseconds();
89+
$this->set('ts_last_attempt', $currentTimestamp);
8890

8991
try {
9092
$job->run();
9193
$this->set('last_attempt_succeeded', 'y');
9294
$success = true;
9395
} catch (Exception $e) {
9496
Logger::error($e->getMessage());
95-
$this->set('ts_last_error', date('Y-m-d H:i:s'));
97+
$this->set('ts_last_error', $currentTimestamp);
9698
$this->set('last_error_message', $e->getMessage());
9799
$this->set('last_attempt_succeeded', 'n');
98100
$success = false;
@@ -127,8 +129,8 @@ public function isOverdue()
127129
}
128130

129131
return (
130-
strtotime($this->get('ts_last_attempt')) + $this->get('run_interval') * 2
131-
) < time();
132+
$this->get('ts_last_attempt') + $this->get('run_interval') * 2 * 1000
133+
) < DaemonUtil::timestampWithMilliseconds();
132134
}
133135

134136
public function hasBeenDisabled()
@@ -145,7 +147,9 @@ public function isPending()
145147
return $this->isWithinTimeperiod();
146148
}
147149

148-
if (strtotime($this->get('ts_last_attempt')) + $this->get('run_interval') < time()) {
150+
if (
151+
$this->get('ts_last_attempt') + $this->get('run_interval') * 1000 < DaemonUtil::timestampWithMilliseconds()
152+
) {
149153
return $this->isWithinTimeperiod();
150154
}
151155

library/Director/Web/Table/JobTable.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use gipfl\IcingaWeb2\Link;
66
use gipfl\IcingaWeb2\Table\ZfQueryBasedTable;
7+
use Icinga\Module\Director\Daemon\DaemonUtil;
78

89
class JobTable extends ZfQueryBasedTable
910
{
@@ -37,11 +38,11 @@ public function renderRow($row)
3738

3839
protected function getJobClasses($row)
3940
{
40-
if ($row->unixts_last_attempt === null) {
41+
if ($row->ts_last_attempt === null) {
4142
return 'pending';
4243
}
4344

44-
if ($row->unixts_last_attempt + $row->run_interval < time()) {
45+
if ($row->ts_last_attempt + $row->run_interval * 1000 < DaemonUtil::timestampWithMilliseconds()) {
4546
return 'pending';
4647
}
4748

@@ -73,7 +74,6 @@ public function prepareQuery()
7374
'run_interval' => 'j.run_interval',
7475
'last_attempt_succeeded' => 'j.last_attempt_succeeded',
7576
'ts_last_attempt' => 'j.ts_last_attempt',
76-
'unixts_last_attempt' => 'UNIX_TIMESTAMP(j.ts_last_attempt)',
7777
'ts_last_error' => 'j.ts_last_error',
7878
'last_error_message' => 'j.last_error_message',
7979
]

library/Director/Web/Widget/JobDetails.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public function __construct(DirectorJob $job)
4545

4646
$tsLastAttempt = $job->get('ts_last_attempt');
4747
if ($tsLastAttempt) {
48-
$ts = \strtotime($tsLastAttempt);
48+
$ts = $tsLastAttempt / 1000;
4949
$timeAgo = Html::tag('span', [
5050
'class' => 'time-ago',
5151
'title' => DateFormatter::formatDateTime($ts)
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
ALTER TABLE director_job ADD COLUMN ts_last_attempt_tmp BIGINT(20) DEFAULT NULL;
2+
ALTER TABLE director_job ADD COLUMN ts_last_error_tmp BIGINT(20) DEFAULT NULL;
3+
4+
5+
UPDATE director_job
6+
SET ts_last_attempt_tmp = UNIX_TIMESTAMP(ts_last_attempt) * 1000,
7+
ts_last_error_tmp = UNIX_TIMESTAMP(ts_last_error) * 1000;
8+
9+
ALTER TABLE director_job
10+
DROP COLUMN ts_last_attempt,
11+
DROP COLUMN ts_last_error,
12+
CHANGE ts_last_attempt_tmp ts_last_attempt BIGINT(20) DEFAULT NULL,
13+
CHANGE ts_last_error_tmp ts_last_error BIGINT(20) DEFAULT NULL;
14+
15+
INSERT INTO director_schema_migration
16+
(schema_version, migration_time)
17+
VALUES (189, NOW());

schema/mysql.sql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -347,8 +347,8 @@ CREATE TABLE director_job (
347347
run_interval INT(10) UNSIGNED NOT NULL, -- seconds
348348
timeperiod_id INT(10) UNSIGNED DEFAULT NULL,
349349
last_attempt_succeeded ENUM('y', 'n') DEFAULT NULL,
350-
ts_last_attempt TIMESTAMP NULL DEFAULT NULL,
351-
ts_last_error TIMESTAMP NULL DEFAULT NULL,
350+
ts_last_attempt BIGINT(20) NULL DEFAULT NULL,
351+
ts_last_error BIGINT(20) NULL DEFAULT NULL,
352352
last_error_message TEXT DEFAULT NULL,
353353
PRIMARY KEY (id),
354354
UNIQUE KEY (job_name),
@@ -2446,4 +2446,4 @@ CREATE TABLE branched_icinga_dependency (
24462446

24472447
INSERT INTO director_schema_migration
24482448
(schema_version, migration_time)
2449-
VALUES (188, NOW());
2449+
VALUES (189, NOW());
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
ALTER TABLE director_job ADD COLUMN ts_last_attempt_tmp bigint DEFAULT NULL;
2+
ALTER TABLE director_job ADD COLUMN ts_last_error_tmp bigint DEFAULT NULL;
3+
4+
5+
UPDATE director_job
6+
SET ts_last_attempt_tmp = UNIX_TIMESTAMP(ts_last_attempt) * 1000,
7+
ts_last_error_tmp = UNIX_TIMESTAMP(ts_last_error) * 1000;
8+
9+
ALTER TABLE director_job
10+
DROP COLUMN ts_last_attempt,
11+
DROP COLUMN ts_last_error;
12+
13+
ALTER TABLE director_job RENAME COLUMN ts_last_attempt_tmp TO ts_last_attempt;
14+
ALTER TABLE director_job RENAME COLUMN ts_last_error_tmp TO ts_last_error;
15+
16+
INSERT INTO director_schema_migration
17+
(schema_version, migration_time)
18+
VALUES (189, NOW());

schema/pgsql.sql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -448,8 +448,8 @@ CREATE TABLE director_job (
448448
run_interval integer NOT NULL, -- seconds
449449
timeperiod_id integer DEFAULT NULL,
450450
last_attempt_succeeded enum_boolean DEFAULT NULL,
451-
ts_last_attempt timestamp with time zone DEFAULT NULL,
452-
ts_last_error timestamp with time zone DEFAULT NULL,
451+
ts_last_attempt bigint DEFAULT NULL,
452+
ts_last_error bigint DEFAULT NULL,
453453
last_error_message text NULL DEFAULT NULL,
454454
CONSTRAINT director_job_period
455455
FOREIGN KEY (timeperiod_id)
@@ -2781,4 +2781,4 @@ CREATE INDEX branched_dependency_search_object_name ON branched_icinga_dependenc
27812781

27822782
INSERT INTO director_schema_migration
27832783
(schema_version, migration_time)
2784-
VALUES (187, NOW());
2784+
VALUES (189, NOW());

0 commit comments

Comments
 (0)