From cc0ab8c81031fae2b28e66f52972473875931278 Mon Sep 17 00:00:00 2001 From: Arthur Shau Date: Thu, 14 Mar 2024 12:43:13 -0700 Subject: [PATCH] timer: introduce DeferReactivation setting By default, in instances where timers are running on a realtime schedule, if a service takes longer to run than the interval of a timer, the service will immediately start again when the previous invocation finishes. This is caused by the fact that the next elapse is calculated based on the last trigger time, which, combined with the fact that the interval is shorter than the runtime of the service, causes that elapse to be in the past, which in turn means the timer will trigger as soon as the service finishes running. This behavior can be changed by enabling the new DeferReactivation setting, which will cause the next calendar elapse to be calculated based on when the trigger unit enters inactivity, rather than the last trigger time. Thus, if a timer is on an realtime interval, the trigger will always adhere to that specified interval. E.g. if you have a timer that runs on a minutely interval, the setting guarantees that triggers will happen at *:*:00 times, whereas by default this may skew depending on how long the service runs. Co-authored-by: Matteo Croce --- docs/TRANSIENT-SETTINGS.md | 1 + man/org.freedesktop.systemd1.xml | 10 ++++++++ man/systemd.timer.xml | 19 +++++++++++++++ src/core/dbus-timer.c | 4 ++++ src/core/load-fragment-gperf.gperf.in | 1 + src/core/timer.c | 23 +++++++++++++------ src/core/timer.h | 1 + src/shared/bus-unit-util.c | 3 ++- .../fuzz-unit-file/directives-all.service | 1 + test/fuzz/fuzz-unit-file/tmpfiles-clean.timer | 1 + 10 files changed, 56 insertions(+), 8 deletions(-) diff --git a/docs/TRANSIENT-SETTINGS.md b/docs/TRANSIENT-SETTINGS.md index 15f1cbc47c4..e219131ce61 100644 --- a/docs/TRANSIENT-SETTINGS.md +++ b/docs/TRANSIENT-SETTINGS.md @@ -387,6 +387,7 @@ Most timer unit settings are available to transient units. ✓ AccuracySec= ✓ RandomizedDelaySec= ✓ FixedRandomDelay= +✓ DeferReactivation= Unit= ``` diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index ccc3e25fbca..1e34ddbc857 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -8804,6 +8804,8 @@ node /org/freedesktop/systemd1/unit/systemd_2dtmpfiles_2dclean_2etimer { readonly b WakeSystem = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly b RemainAfterElapse = ...; + @org.freedesktop.DBus.Property.EmitsChangedSignal("const") + readonly b DeferReactivation = ...; }; interface org.freedesktop.DBus.Peer { ... }; interface org.freedesktop.DBus.Introspectable { ... }; @@ -8832,6 +8834,8 @@ node /org/freedesktop/systemd1/unit/systemd_2dtmpfiles_2dclean_2etimer { + + @@ -8874,6 +8878,8 @@ node /org/freedesktop/systemd1/unit/systemd_2dtmpfiles_2dclean_2etimer { + + @@ -12386,5 +12392,9 @@ $ gdbus introspect --system --dest org.freedesktop.systemd1 \ Job Objects ActivationDetails was added in version 252. + + Timer Objects + DeferReactivation was added in version 257. + diff --git a/man/systemd.timer.xml b/man/systemd.timer.xml index ad79552ed66..2ea56d687fa 100644 --- a/man/systemd.timer.xml +++ b/man/systemd.timer.xml @@ -305,6 +305,25 @@ + + DeferReactivation= + + Takes a boolean argument. When enabled, the timer schedules the next elapse based on + the trigger unit entering inactivity, instead of the last trigger time. + This is most apparent in the case where the service unit takes longer to run than the timer interval. + With this setting enabled, the timer will schedule the next elapse based on when the service finishes + running, and so it will have to wait until the next realtime elapse time to trigger. + Otherwise, the default behavior is for the timer unit to immediately trigger again once the service + finishes running. This happens because the timer schedules the next elapse based on the previous trigger + time, and since the interval is shorter than the service runtime, that elapse will be in the past, + causing it to immediately trigger once done. + + This setting has no effect if a realtime timer has not been specified with + OnCalendar=. Defaults to . + + + + OnClockChange= OnTimezoneChange= diff --git a/src/core/dbus-timer.c b/src/core/dbus-timer.c index 4f78a521de2..b9d0c16acd8 100644 --- a/src/core/dbus-timer.c +++ b/src/core/dbus-timer.c @@ -118,6 +118,7 @@ const sd_bus_vtable bus_timer_vtable[] = { SD_BUS_PROPERTY("Persistent", "b", bus_property_get_bool, offsetof(Timer, persistent), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("WakeSystem", "b", bus_property_get_bool, offsetof(Timer, wake_system), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("RemainAfterElapse", "b", bus_property_get_bool, offsetof(Timer, remain_after_elapse), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("DeferReactivation", "b", bus_property_get_bool, offsetof(Timer, defer_reactivation), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_VTABLE_END }; @@ -233,6 +234,9 @@ static int bus_timer_set_transient_property( if (streq(name, "OnClockChange")) return bus_set_transient_bool(u, name, &t->on_clock_change, message, flags, error); + if (streq(name, "DeferReactivation")) + return bus_set_transient_bool(u, name, &t->defer_reactivation, message, flags, error); + if (streq(name, "TimersMonotonic")) { const char *base_name; usec_t usec; diff --git a/src/core/load-fragment-gperf.gperf.in b/src/core/load-fragment-gperf.gperf.in index 221099a39c0..e94b518a9d8 100644 --- a/src/core/load-fragment-gperf.gperf.in +++ b/src/core/load-fragment-gperf.gperf.in @@ -570,6 +570,7 @@ Timer.Persistent, config_parse_bool, Timer.WakeSystem, config_parse_bool, 0, offsetof(Timer, wake_system) Timer.RemainAfterElapse, config_parse_bool, 0, offsetof(Timer, remain_after_elapse) Timer.FixedRandomDelay, config_parse_bool, 0, offsetof(Timer, fixed_random_delay) +Timer.DeferReactivation, config_parse_bool, 0, offsetof(Timer, defer_reactivation) Timer.AccuracySec, config_parse_sec, 0, offsetof(Timer, accuracy_usec) Timer.RandomizedDelaySec, config_parse_sec, 0, offsetof(Timer, random_usec) Timer.Unit, config_parse_trigger_unit, 0, 0 diff --git a/src/core/timer.c b/src/core/timer.c index 7cb58cc2d9d..e44298d5e84 100644 --- a/src/core/timer.c +++ b/src/core/timer.c @@ -245,7 +245,8 @@ static void timer_dump(Unit *u, FILE *f, const char *prefix) { "%sRemainAfterElapse: %s\n" "%sFixedRandomDelay: %s\n" "%sOnClockChange: %s\n" - "%sOnTimeZoneChange: %s\n", + "%sOnTimeZoneChange: %s\n" + "%sDeferReactivation: %s\n", prefix, timer_state_to_string(t->state), prefix, timer_result_to_string(t->result), prefix, trigger ? trigger->id : "n/a", @@ -255,7 +256,8 @@ static void timer_dump(Unit *u, FILE *f, const char *prefix) { prefix, yes_no(t->remain_after_elapse), prefix, yes_no(t->fixed_random_delay), prefix, yes_no(t->on_clock_change), - prefix, yes_no(t->on_timezone_change)); + prefix, yes_no(t->on_timezone_change), + prefix, yes_no(t->defer_reactivation)); LIST_FOREACH(value, v, t->values) if (v->base == TIMER_CALENDAR) { @@ -391,12 +393,19 @@ static void timer_enter_waiting(Timer *t, bool time_change) { if (v->base == TIMER_CALENDAR) { usec_t b, rebased; - /* If we know the last time this was - * triggered, schedule the job based relative - * to that. If we don't, just start from - * the activation time. */ + /* If DeferReactivation= is enabled, schedule the job based on the last time + * the trigger unit entered inactivity. Otherwise, if we know the last time + * this was triggered, schedule the job based relative to that. If we don't, + * just start from the activation time or realtime. */ - if (dual_timestamp_is_set(&t->last_trigger)) + if (t->defer_reactivation && + dual_timestamp_is_set(&trigger->inactive_enter_timestamp)) { + if (dual_timestamp_is_set(&t->last_trigger)) + b = MAX(trigger->inactive_enter_timestamp.realtime, + t->last_trigger.realtime); + else + b = trigger->inactive_enter_timestamp.realtime; + } else if (dual_timestamp_is_set(&t->last_trigger)) b = t->last_trigger.realtime; else if (dual_timestamp_is_set(&UNIT(t)->inactive_exit_timestamp)) b = UNIT(t)->inactive_exit_timestamp.realtime; diff --git a/src/core/timer.h b/src/core/timer.h index 2624001c34b..14a9931dffe 100644 --- a/src/core/timer.h +++ b/src/core/timer.h @@ -61,6 +61,7 @@ struct Timer { bool on_clock_change; bool on_timezone_change; bool fixed_random_delay; + bool defer_reactivation; char *stamp_path; }; diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 9fdf5662052..b1518269208 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -2594,7 +2594,8 @@ static int bus_append_timer_property(sd_bus_message *m, const char *field, const "Persistent", "OnTimezoneChange", "OnClockChange", - "FixedRandomDelay")) + "FixedRandomDelay", + "DeferReactivation")) return bus_append_parse_boolean(m, field, eq); if (STR_IN_SET(field, "AccuracySec", diff --git a/test/fuzz/fuzz-unit-file/directives-all.service b/test/fuzz/fuzz-unit-file/directives-all.service index dbd56ec752f..1cb212bcadd 100644 --- a/test/fuzz/fuzz-unit-file/directives-all.service +++ b/test/fuzz/fuzz-unit-file/directives-all.service @@ -7,6 +7,7 @@ AllowedCPUs= AllowedMemoryNodes= AllowIsolate= Also= +DeferReactivation= AmbientCapabilities= AssertACPower= AssertArchitecture= diff --git a/test/fuzz/fuzz-unit-file/tmpfiles-clean.timer b/test/fuzz/fuzz-unit-file/tmpfiles-clean.timer index 5bf91b9f4cd..5dc269243fa 100644 --- a/test/fuzz/fuzz-unit-file/tmpfiles-clean.timer +++ b/test/fuzz/fuzz-unit-file/tmpfiles-clean.timer @@ -33,6 +33,7 @@ Persistent=true AccuracySec=24h RandomizedDelaySec=234234234 FixedRandomDelay=true +DeferReactivation=true Persistent=no Unit=foo.service -- 2.47.3