PR#1: Run cache update in a transient service using systemd-run - rpms/man-db

rpms / man-db

#1 Run cache update in a transient service using systemd-run

Closed 6 years ago by nforro. Opened 6 years ago by nforro.

private-nforro-bz1318058 into master

move cache update to a separate service file

Nikola Forró • 6 years ago

3341218

run cache update in a transient service using systemd-run

Nikola Forró • 6 years ago

1de1fcc

man-db-cache-update.service

file added

+13

		`@@ -0,0 +1,13 @@`
		`+ [Unit]`
		`+ DefaultDependencies=no`
		`+ After=local-fs.target`
		`+ Before=reboot.target shutdown.target halt.target`
		`+ JobTimeoutSec=15min`
		`+`
		`+ [Service]`
		`+ Type=oneshot`
		`+ TimeoutStartSec=infinity`
		`+ Environment=MAN_NO_LOCALE_WARNING=1`
		`+ ExecStart=/usr/bin/mandb`
		`+ Nice=19`
		`+ IOWeight=20`

man-db.spec

file modified

+20 -3

		`@@ -4,7 +4,7 @@`
		`Summary: Tools for searching and reading man pages`
		`Name: man-db`
		`Version: 2.7.6.1`
		`- Release: 5%{?dist}`
		`+ Release: 7%{?dist}`
		`# GPLv2+ .. man-db`
		`# GPLv3+ .. gnulib`
		`License: GPLv2+ and GPLv3+`
		`@@ -14,6 +14,7 @@`
		`Source0: http://download.savannah.gnu.org/releases/%{name}/%{name}-%{version}.tar.xz`
		`Source1: man-db.crondaily`
		`Source2: man-db.sysconfig`
		`+ Source3: man-db-cache-update.service`
		`Patch0: man-db-2.7.6.1-change-owner-of-man-cache.patch`

		`# http://lists.nongnu.org/archive/html/man-db-devel/2017-01/msg00013.html`
		`@@ -26,6 +27,9 @@`
		`Provides: bundled(gnulib) = %{gnulib_ver}`

		`Requires: coreutils, grep, groff-base, gzip, less`
		`+ # for file trigger scriptlets`
		`+ Requires: systemd`
		`+ BuildRequires: systemd`
		`BuildRequires: gdbm-devel, gettext, groff, less, libpipeline-devel, zlib-devel`
		`BuildRequires: po4a, perl-interpreter, perl-version`

		`@@ -89,6 +93,9 @@`
		`# config for tmpfiles.d`
		`install -D -p -m 0644 init/systemd/man-db.conf $RPM_BUILD_ROOT/usr/lib/tmpfiles.d/.`

		`+ # man-db-cache-update.service`
		`+ install -D -p -m 0644 %{SOURCE3} $RPM_BUILD_ROOT%{_unitdir}/man-db-cache-update.service`
		`+`
		`%find_lang %{name}`
		`%find_lang %{name}-gnulib`

		`@@ -107,11 +114,12 @@`

		`# update cache`
		`%transfiletriggerin -- %{_mandir}`
		`- MAN_NO_LOCALE_WARNING=1 /usr/bin/mandb -q`
		`+ /usr/bin/systemd-run /usr/bin/systemctl start man-db-cache-update`

		`# update cache`
		`%transfiletriggerpostun -- %{_mandir}`
		`- MAN_NO_LOCALE_WARNING=1 /usr/bin/mandb -q`
		`+ /usr/bin/systemd-run /usr/bin/systemctl start man-db-cache-update`
		`+`

		`%files -f %{name}.lang -f %{name}-gnulib.lang`
		`%{!?_licensedir:%global license %%doc}`
		`@@ -119,6 +127,7 @@`
		`%doc README man-db-manual.txt man-db-manual.ps ChangeLog NEWS`
		`%config(noreplace) %{_sysconfdir}/man_db.conf`
		`%config(noreplace) /usr/lib/tmpfiles.d/man-db.conf`
		`+ %{_unitdir}/man-db-cache-update.service`
		`%{_sbindir}/accessdb`
		`%{_bindir}/man`
		`%{_bindir}/whatis`
		`@@ -163,6 +172,14 @@`
		`%config(noreplace) %{_sysconfdir}/sysconfig/man-db`

		`%changelog`
		`+ * Thu Nov 09 2017 Nikola Forró <nforro@redhat.com> - 2.7.6.1-7`
		`+ - move cache update to a separate service file that can prevent`
		`+ system shutdown/reboot`
		`+`
		`+ * Wed Nov 08 2017 Nikola Forró <nforro@redhat.com> - 2.7.6.1-6`
		`+ - run cache update in a transient service using systemd-run`
		`+ resolves #1318058`
		`+`
		`* Thu Aug 03 2017 Fedora Release Engineering <releng@fedoraproject.org> - 2.7.6.1-5`
		`- Rebuilt for https://fedoraproject.org/wiki/Fedora_27_Binutils_Mass_Rebuild`

no initial comment

zbyszek commented on line 30 of man-db.spec 6 years ago

I'm not sure if Restart=on-failure is a good idea. If it fails for some reason (e.g. a malformed man page, or permission issues), it would go in a loop. Seems safer to just allow it to fail. Man pages are upgraded regularly, so it'll get another chance to run then.

zbyszek commented 6 years ago

Another thing: systemd-run jobs are not protected from reboots. This is particularly painful for offline updates where a reboot will happen after the rpm transaction is finished. You need to wrap the mandb run in systemd-inhibit to prevent that.

nforro commented 6 years ago

@zbyszek: Thanks for the tip. Can systemd-inhibit be combined with systemd-run?

zbyszek commented 6 years ago

Hmm, sorry. systemd-inhibit only works partially here: it'll block reboot requests from users, but not a reboot command from root. A real unit will be required with shutdown ordering, I don't think systemd-run is flexible enough.

zbyszek commented 6 years ago

This needs a bit of a hack. A similar issue came up recently with coredump processing, where the job is started in a Type=oneshot unit, and if the machine is reboot, the reboot is not delayed to allowe the process to finish. We'll need to figure out some solution for this type of thing.

A short term solution:

# /usr/lib/systemd/system/man-db-cache-update.service
[Unit]
DefaultDependencies=no
After=local-fs.target
Before=shutdown.target
JobRunningTimeoutSec=30 min

[Service]
Type=notify
ExecStartPre=/bin/echo "DOING..."                   # remove after testing
ExecStart=/bin/sh -c 'mandb; systemd-notify READY=1'
ExecStartPost=/bin/echo "...DONE!"                  # remove after testing
Environment=MAN_NO_LOCALE_WARNING=1
Nice=19
IOWeight=20
NotifyAccess=all

This delays shutdown appropriately.

All that said, I think a better long-term solution would be to teach mandb to only update the index for pages that were actually added/changed/removed. Then the update will be much quicker and this will be less of a problem. %transfiletriggers allow this to be done (you get a list of files that caused the trigger).

nforro commented 6 years ago

@zbyszek: Thanks. I guess even if mandb would update cache only for specific man pages, it still could take a long time (in rare cases, of course). So maybe the best long term solution is to implement both.

zbyszek commented 6 years ago

Agreed.