]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
core: let OnCalendar= timer units expire during suspend (#8231)
authorAlan Jenkins <alan.christopher.jenkins@gmail.com>
Wed, 28 Feb 2018 15:34:16 +0000 (15:34 +0000)
committerAlan Jenkins <alan.christopher.jenkins@gmail.com>
Wed, 28 Feb 2018 16:12:22 +0000 (16:12 +0000)
On timejumps, including suspend, timer_time_change() calls for a
re-calculation of the next elapse.  Sadly I'm not quite sure what the
intended effect of this was!  Because it was not managing to fire
OnCalendar= timers which fired during the suspend... unless the timer had
already fired once before.

Reported, entirely correctly as far as I can see, on stackexchange:
https://unix.stackexchange.com/questions/351829/systemd-timer-that-expired-while-suspended

 /* If we know the last time this was
  * triggered, schedule the job based relative
- * to that. If we don't just start from
- * now. */
+ * to that. If we don't, just start from
+ * the activation time. */

The same code is called for both the initial calculation and this
re-calculation.  If we're _not_ already active, then this is before the
activation time has been recorded in the unit, so just use the current
time as before.  The new code is mechanically adapted from the same
logic for `OnActiveSec=` (case TIMER_ACTIVE in the code which follows).

Tested with `date --set`.

Motivations:

* Rotate monitoring data from Atop into files which are named per-day.
  Fedora currently implements this with a cron job that runs at midnight,
  but that didn't handle suspend correctly either.

* unbound-anchor.timer on Fedora, is used to update DNSSEC "root trust
  anchor" daily, before the TTL expires.  It uses OnCalendar=daily
  AccuracySec=24h.  Which is a bit suspect because the TTL is 2 days, but I
  think it has the right general idea.

  None of the other timer settings are correct, because they would not
  account for time spent in suspend.  Unless you set WakeSystem
  (this feature is currently undocumented).

* So in general, we can expect to see people using OnCalendar= for the same
  cases as cron.daily and cron.monthly.  Which use anacron to keep track of
  jobs which should be run even if the system was down at the time.

  Timers which are configured to run more frequently than that, are
  unlikely to mind if they get run slightly more often that the writer
  realized, relative to the amount of time the system was really running.

* From the user report above: "I only want to use remind to show a desktop
  notification, it seems excessive to wake up the computer for that. Also,
  I would like to get the reminder first thing in the morning, so the
  OnActiveSec doesn't help with that."

src/core/timer.c

index 1770c593f81f1dcf36884cd2c04f1813bbd00f2d..e0f39584ced407f5543df353a1e8399fbf6b621a 100644 (file)
@@ -379,10 +379,17 @@ static void timer_enter_waiting(Timer *t, bool initial) {
 
                         /* If we know the last time this was
                          * triggered, schedule the job based relative
-                         * to that. If we don't just start from
-                         * now. */
+                         * to that. If we don't, just start from
+                         * the activation time. */
 
-                        b = t->last_trigger.realtime > 0 ? t->last_trigger.realtime : ts.realtime;
+                        if (t->last_trigger.realtime > 0)
+                                b = t->last_trigger.realtime;
+                        else {
+                                if (state_translation_table[t->state] == UNIT_ACTIVE)
+                                        b = UNIT(t)->inactive_exit_timestamp.realtime;
+                                else
+                                        b = ts.realtime;
+                        }
 
                         r = calendar_spec_next_usec(v->calendar_spec, b, &v->next_elapse);
                         if (r < 0)