Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

systemd-timesyncd: save time periodically #10620

Closed
dimitry-ishenko opened this issue Nov 2, 2018 · 10 comments · Fixed by #20142
Closed

systemd-timesyncd: save time periodically #10620

dimitry-ishenko opened this issue Nov 2, 2018 · 10 comments · Fixed by #20142
Labels
RFE 🎁 Request for Enhancement, i.e. a feature request timesync

Comments

@dimitry-ishenko
Copy link
Contributor

Is your feature request related to a problem? Please describe.
I have an offline embedded system without RTC. If the system is shutdown uncleanly, /var/lib/systemd/clock is never updated. I believe even on clean shutdown, if it never gets an NTP fix, it won't update the clock file. (Didn't test the last part, just judging from this code.) I end up with a ton of files in the "future."

Describe the solution you'd like
systemd-timesyncd should accept an optional "update" parameter. If called with this parameter, it should update the clock file and exit immediately. Next, a timer unit can be created that runs, say, once an hour and calls "systemd-timesyncd update." People who want this functionality can enable the timer. The rest of the universe is unaffected.

Describe alternatives you've considered
Currently I have a separate service that runs on startup and shutdown and in cron job.

@yuwata yuwata added RFE 🎁 Request for Enhancement, i.e. a feature request timesync labels Nov 3, 2018
@poettering
Copy link
Member

So I figure it would suffice for you if timesyncd would wake up every 30s or so and sync the clock to disk no matter what?

@dimitry-ishenko
Copy link
Contributor Author

@poettering yes that would be splendid

@dsd
Copy link
Contributor

dsd commented Jul 1, 2021

For Endless OS we are also considering (finally) replacing ntpd & fake-hwclock with systemd-timesyncd.

The always offline with no battery-backed RTC use case is of interest here. Having timesyncd writing the time periodically to disk (even when no NTP synchronization has succeeded) would indeed be a suitable solution here too, allowing fake-hwclock to be dropped.

Also, the simple behaviour of writing the time to disk at shutdown time (even when no NTP synchronization happened) would also suffice for our needs, this is what fake-hwclock does. In fact, this used to be the timesyncd behaviour, and there is a stale comment in the code:

        /* Let's try to make sure that the clock is always
         * monotonically increasing, by saving the clock whenever we
         * have a new NTP time, or when we shut down, and restoring it
         * when we start again. This is particularly helpful on
         * systems lacking a battery backed RTC. We also will adjust
         * the time to at least the build time of systemd. */

The comment is still there, but the described save-on-shutdown behaviour was removed in d636d37.

The reasoning there is curious. It concerns saving and restoring the clock if the time/date was set to a future time. Presumably this change also concerned always-offline systems, since online systems would have a swift correction in the time/date via NTP.

Of the choice between:

  1. Having a constant time/date that is likely within <5 years of the real world time/date, which is restored on each and every boot, or
  2. Having a time/date that is always monotonically increasing,

option (1) was chosen whereas IMO (2) is much more important. It allows such systems to know reliably which files are newer than others, and through having an always-increasing clock it avoids confusion where files that were actually created in the past are seen to the system as having been created in the future.

If the shutdown behaviour were tweaked back again, or if the periodic save-time-to-disk were implemented as hinted above, it would reintroduce any issue that Kay was trying to solve in this commit.

@kaysievers any thoughts?

BTW, this same issue is also one of the things complicating Raspberry Pi OS moving from fake-hwclock to timesyncd. https://www.raspberrypi.org/forums/viewtopic.php?t=200385#p1748104

@poettering
Copy link
Member

i think adding both periodic save to disk + at shutdown for RTC-less systems makes sense.

@dimitry-ishenko
Copy link
Contributor Author

@poettering this seems simple to (re)implement. I can take a stab at it, but may need some guidance. Are you available?

@dsd
Copy link
Contributor

dsd commented Jul 2, 2021

@dimitry-ishenko if you have specific questions, feel free to post them here, I will do my best to provide some useful guidance.

@dimitry-ishenko
Copy link
Contributor Author

dimitry-ishenko commented Jul 5, 2021

OK, so saving the time at the end seems pretty trivial:

/* if we got an authoritative time, store it in the file system */
if (m->sync) {
r = touch(CLOCK_FILE);
if (r < 0)
log_debug_errno(r, "Failed to touch %s, ignoring: %m", CLOCK_FILE);
}

Just remove the condition and call it a day.


In terms of periodic save, the current behavior is to save the time every time a good NTP sample is received:

if (!spike) {
m->sync = true;
r = manager_adjust_clock(m, offset, leap_sec);
if (r < 0)
log_error_errno(r, "Failed to call clock_adjtime(): %m");
}

(touch is called inside manager_adjust_clock).

My idea was to add a timer event (configurable through timesyncd.conf) that periodically calls touch and gets bumped (delayed) whenever a good NTP packet was received.

Thoughts?

@dsd
Copy link
Contributor

dsd commented Jul 6, 2021

Just remove the condition and call it a day.

And update the comment. You may also be able to remove m->sync now too.

My idea was to add a timer event (configurable through timesyncd.conf) that periodically calls touch and gets bumped (delayed) whenever a good NTP packet was received.

Sounds good to me. And if the timer frequency is configured to zero, this behaviour could be disabled.

I suggest making these changes in 2 separate commits (save time on shutdown, save time periodically).

@dimitry-ishenko
Copy link
Contributor Author

dimitry-ishenko commented Jul 6, 2021

Thanks @dsd. The first part is implemented here:

https://github.com/dimitry-ishenko-rpi/systemd/commit/e4be071fbb0cb04fcf3094f25d88412a1e31f2cb

The second part is mostly done:

https://github.com/dimitry-ishenko-rpi/systemd/commit/061a441ce60fe024ade1febb74163c70880765bf

I am trying to see where is the best place to plug in sd_event_add_time_relative. It has to be at some point after:

r = manager_parse_config_file(m);
if (r < 0)
log_warning_errno(r, "Failed to parse configuration file: %m");

as the parameter will be read from the config file.

Any ideas?

@dimitry-ishenko
Copy link
Contributor Author

@dsd @poettering what do you think?

@poettering poettering linked a pull request Aug 9, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFE 🎁 Request for Enhancement, i.e. a feature request timesync
Development

Successfully merging a pull request may close this issue.

4 participants