Welcome to little lamb

Code » slicd » commit 93edd2a

Fix DST handling

author Olivier Brunel
2016-04-03 17:54:01 UTC
committer Olivier Brunel
2016-04-30 12:30:10 UTC
parent b740d3e824e0bc4c05747994e8ba2cc568cd69ef

Fix DST handling

doc/slicd-sched.pod +26 -6
src/libslicd/slicd_job_next_run.c +107 -35
src/slicd/slicd-sched.c +10 -49

diff --git a/doc/slicd-sched.pod b/doc/slicd-sched.pod
index 65d9935..48bd3f7 100644
--- a/doc/slicd-sched.pod
+++ b/doc/slicd-sched.pod
@@ -44,13 +44,33 @@ check if a job definition matches & must be run; Instead, it calculate the next
 runtime for each job, and waits until the closest one.
 
 It does so via a timer set on the real-time system clock, which means any time
-changes (manual, NTP, daylight savings, etc) is handled properly. Specifically,
-if the time jumps backward, nothing happens (jobs are not re-run). If the time
-jumps forward, it will process every job that should have run during the
-(missed) interval, but only once.
+changes (manual, NTP, etc) is handled properly.
 
-So a job said to run every minute would only be processed once, even for a jump
-forward of many hours.
+Specifically, if the time jumps backward, nothing happens (jobs are not re-run),
+as B<slicd-sched>(1) still waits for the time previously calculated. If after
+setting the time backward you want jobs to be processed, you'll need to have
+B<slicd-sched>(1) re-process jobs at the (new) current minute, which can be done
+simply by sending it a signal USR1.
+
+If the time jumps forward and the timer has expired, it will process every job
+that should run in the current minute, and again set a new timer for the closest
+next run. (There's no attempt to run "missed" jobs.)
+
+=head2 Daylight Saving Time
+
+Time changes due to DST are also handled properly, though differently since they
+are another beast.
+
+Specifically, when time jumps backward then jobs that run during the "repeated"
+interval will indeed run again. For example, if a job runs every hour at minute
+30, and the clock jumps from 02:59 (DST activated) to 02:00 (DST deactivated),
+then the job will run at 02:30 (DST on), then an hour later at 02:30 (DST off).
+
+When time jumps forward then jobs that would have run during the "missed"
+interval will simply not run, since that time didn't happen; there's no attempt
+to run "missed" jobs either. For example our job set to run every hour at minute
+30, when the clock goes from 01:59 (DST off) to 03:00 (DST on), would simply run
+at 01:30 (DST off) then an hour later at 03:30 (DST on).
 
 =head1 SIGNALS
 
diff --git a/src/libslicd/slicd_job_next_run.c b/src/libslicd/slicd_job_next_run.c
index aceaa98..66421b3 100644
--- a/src/libslicd/slicd_job_next_run.c
+++ b/src/libslicd/slicd_job_next_run.c
@@ -23,95 +23,167 @@
 #include <skalibs/bytestr.h>
 #include <slicd/sched.h>
 
+#define ONE_MINUTE      60
+#define ONE_HOUR        60 * ONE_MINUTE
+#define ONE_DAY         24 * ONE_HOUR
+
 /* source: http://stackoverflow.com/a/11595914 */
 #define is_leap(year) \
     ((year & 3) == 0 && ((year % 25) != 0 || (year & 15) == 0))
 
+static time_t
+first_minute_of_dst (time_t time, struct tm *time_tm)
+{
+    int interval = 10 * ONE_DAY;
+    int back = 1;
+
+    for (;;)
+    {
+        struct tm *tm;
+
+        if (back)
+            time -= interval;
+        else
+            time += interval;
+
+        tm = localtime (&time);
+        if (!tm)
+            return (time_t) -1;
+        if (back)
+        {
+            if (tm->tm_isdst != time_tm->tm_isdst)
+            {
+                back = 0;
+                if (interval == 10 * ONE_DAY)
+                    interval = ONE_DAY;
+                else
+                    interval = ONE_MINUTE;
+            }
+        }
+        else if (tm->tm_isdst == time_tm->tm_isdst)
+        {
+            back = 1;
+            if (interval == ONE_DAY)
+                interval = ONE_HOUR;
+            else
+            {
+                memcpy (time_tm, tm, sizeof (struct tm));
+                return time;
+            }
+        }
+    }
+}
+
 time_t
 slicd_job_next_run (slicd_job_t *job, struct tm *next)
 {
     int days[12] = { 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 };
     time_t time;
+    int dst = -1;
     int n;
 
+again:
+    /* set tm_wday & tm_isdst, plus return value on success */
+    time = mktime (next);
+    if (time == (time_t) -1)
+        return time;
+    else if (dst == -1)
+        dst = next->tm_isdst;
+    else if (next->tm_isdst != dst)
+    {
+        /* we changed DST state, set next to the first minute with this new DST
+         * state, and move on from here (to properly handle DST changes, i.e.
+         * skip the "unexisting" interval when jumping forward, or find both
+         * times (w/ & w/out DST) when jumping backward, since that's what
+         * actually happens */
+        time = first_minute_of_dst (time, next);
+        if (time == (time_t) -1)
+            return time;
+        dst = next->tm_isdst;
+    }
+
     days[1] = is_leap (next->tm_year) ? 29 : 28;
 
 month:
     n = slicd_job_first (job, SLICD_MONTHS, next->tm_mon + 1, 12, 1);
     if (n > 12)
     {
-bump_year:
         ++next->tm_year;
-        days[1] = is_leap (next->tm_year) ? 29 : 28;
         next->tm_mon = 0;
         next->tm_mday = 1;
         next->tm_hour = 0;
         next->tm_min = 0;
-        goto month;
+        /* we know nothing possibly matches in the remaining months of the year,
+         * so a DST change doesn't matter and can be safely ignored */
+        dst = -1;
+        goto again;
     }
-    else if (n > next->tm_mon + 1)
+    else if (n > next->tm_mon + 1 || dst == -1)
     {
         next->tm_mon = n - 1;
         next->tm_mday = 1;
         next->tm_hour = 0;
         next->tm_min = 0;
+        dst = -1;
+        goto again;
     }
 
-day:
-    if (slicd_job_has_days_combo (job))
-        goto hour;
-
-    n = slicd_job_first (job, SLICD_DAYS, next->tm_mday, days[next->tm_mon], 1);
-    if (n > days[next->tm_mon])
-    {
-bump_month:
-        if (++next->tm_mon > 11)
-            goto bump_year;
-        next->tm_mday = 1;
-        next->tm_hour = 0;
-        next->tm_min = 0;
-        goto month;
-    }
-    else if (n > next->tm_mday)
+    if (!slicd_job_has_days_combo (job))
     {
-        next->tm_mday = n;
-        next->tm_hour = 0;
-        next->tm_min = 0;
+day:
+        n = slicd_job_first (job, SLICD_DAYS, next->tm_mday, days[next->tm_mon], 1);
+        if (n > days[next->tm_mon])
+        {
+            ++next->tm_mon;
+            dst = -1;
+            if (next->tm_mon <= 11)
+                goto month;
+            else
+                goto again;
+        }
+        else if (n > next->tm_mday || dst == -1)
+        {
+            next->tm_mday = n;
+            next->tm_hour = 0;
+            next->tm_min = 0;
+            dst = -1;
+            goto again;
+        }
     }
 
-hour:
     n = slicd_job_first (job, SLICD_HOURS, next->tm_hour, 23, 1);
     if (n > 23)
     {
 bump_day:
-        if (++next->tm_mday > days[next->tm_mon])
-            goto bump_month;
+        ++next->tm_mday;
+        dst = -1;
+        if (!slicd_job_has_days_combo (job) && next->tm_mday <= days[next->tm_mon])
+            goto day;
         next->tm_hour = 0;
         next->tm_min = 0;
-        goto day;
+        goto again;
     }
     else if (n > next->tm_hour)
     {
+        /* from here on, a DST change matters & must be accounted for. */
         next->tm_hour = n;
         next->tm_min = 0;
+        goto again;
     }
 
-/* minute: */
     n = slicd_job_first (job, SLICD_MINUTES, next->tm_min, 59, 1);
     if (n > 59)
     {
-        if (++next->tm_hour > 23)
-            goto bump_day;
+        ++next->tm_hour;
         next->tm_min = 0;
-        goto hour;
+        goto again;
     }
     else if (n > next->tm_min)
+    {
         next->tm_min = n;
+        goto again;
+    }
 
-    /* get tm_wday, plus return value on success */
-    time = mktime (next);
-    if (time == (time_t) -1)
-        return time;
 
     if (!slicd_job_has (job, SLICD_DOW, next->tm_wday))
         goto bump_day;
diff --git a/src/slicd/slicd-sched.c b/src/slicd/slicd-sched.c
index 9296746..ccde9c2 100644
--- a/src/slicd/slicd-sched.c
+++ b/src/slicd/slicd-sched.c
@@ -204,8 +204,8 @@ main (int argc, char * const argv[])
     {
         if (iop[1].revents & IOPAUSE_READ)
         {
-            struct tm cur_tm, next_tm, since_tm, *tm;
-            time_t cur_time, next_time, since_time;
+            struct tm cur_tm, next_tm, *tm;
+            time_t cur_time, next_time;
             int i;
 
             if (clock_gettime (CLOCK_REALTIME, &its.it_interval) < 0)
@@ -221,41 +221,12 @@ main (int argc, char * const argv[])
             cur_time -= cur_tm.tm_sec;
             cur_tm.tm_sec = 0;
 
-            since_time = cur_time;
-            byte_copy (&since_tm, sizeof (since_tm), &cur_tm);
-
-            /* is there a timerfd set? i.e. did we wait for some time, if so we
-             * might need to handle some time changes.
-             * Any time change shouldn't affect us, since our timerfd is based
-             * on CLOCK_REALTIME and simply "adjust" as needed. But, time might
-             * have jumped forwad and we "missed" our deadline...
-             * We'll also account for the case of the time going back right
-             * after our timerfd expired, just in case.
-             */
-            if (its.it_value.tv_sec > 0)
+            /* post-DO_RELOAD, wait for next minute to avoid rerunning jobs */
+            if (its.it_value.tv_sec == 1 && its.it_value.tv_nsec == 1)
             {
-                /* post-DO_RELOAD, wait for next minute to avoid rerunning jobs */
-                if (its.it_value.tv_nsec > 0)
-                {
-                    its.it_value.tv_nsec = 0;
-                    next_time = cur_time + 60;
-                    goto timer;
-                }
-                else if (cur_time > its.it_value.tv_sec)
-                {
-                    /* we're ahead of the plan, we need to trigger anything that
-                     * we "missed" */
-                    since_time = its.it_value.tv_sec;
-                    tm = localtime (&since_time);
-                    if (!tm)
-                        strerr_diefu1x (RC_OTHER, "break down time");
-                    byte_copy (&since_tm, sizeof (since_tm), tm);
-                }
-                else if (cur_time < its.it_value.tv_sec)
-                {
-                    /* we're early somehow; just wait it out... */
-                    goto pause;
-                }
+                its.it_value.tv_nsec = 0;
+                next_time = cur_time + 60;
+                goto timer;
             }
 
             next_time = (time_t) -1;
@@ -265,7 +236,7 @@ main (int argc, char * const argv[])
                 struct tm job_tm;
                 time_t job_time;
 
-                byte_copy (&job_tm, sizeof (job_tm), &since_tm);
+                byte_copy (&job_tm, sizeof (job_tm), &cur_tm);
 next_run:
                 job_time = slicd_job_next_run (job, &job_tm);
                 if (job_time == (time_t) -1)
@@ -274,23 +245,14 @@ next_run:
                     continue;
                 }
 
-                if (job_time <= cur_time)
+                if (job_time == cur_time)
                 {
                     buffer_puts (buffer_1small, job_str (job));
                     if (!buffer_putsflush (buffer_1small, "\n"))
                         strerr_diefu1sys (RC_IO, "write to stdout");
 
-                    /* now calculate the "actual" next run, using cur_tm in case
-                     * since_tm was set earlier (time jump) */
-                    byte_copy (&job_tm, sizeof (job_tm), &cur_tm);
+                    /* now calculate the "actual" next run */
                     ++job_tm.tm_min;
-                    /* call mktime() to update job_tm, as one more minute
-                     * might be a new hour, day, etc */
-                    if (mktime (&job_tm) == (time_t) -1)
-                    {
-                        strerr_warnwu3x ("get next run for job '", job_str (job), "'");
-                        continue;
-                    }
                     goto next_run;
                 }
                 else if (next_time == (time_t) -1 || job_time < next_time)
@@ -309,7 +271,6 @@ timer:
         else if (iop[1].revents & IOPAUSE_EXCEPT)
             strerr_dief1sys (RC_IO, "trouble with timerfd");
 
-pause:
         r = iopause_g (iop, 2, NULL);
         if (r < 0)
             strerr_diefu1sys (RC_IO, "iopause");