Differential D19924

cron(8): schedule interval jobs that get loaded during execution
ClosedPublic
Actions

Authored by kevans on Apr 16 2019, 2:57 PM.

Details

Reviewers

bapt
otis
allanjude

Group Reviewers

manpages

Commits

rS346427: cron(8): schedule interval jobs that get loaded during execution

Summary

Jobs using the @<second> syntax currently only get executed if they exist when cron is started. The simplest reproducer of this is:

echo '@20 root echo "Hello!"' >> /etc/cron.d/myjob

myjob will get loaded at the next second==0, but this echo job will ever run. These jobs are normally handled in run_reboot_jobs(), which sets e->lastexit of INTERVAL jobs to the startup time so they run 'n' seconds later.

Fix this by special-casing TargetTime > 0 in the database load. Preexisting jobs will be handled at startup during run_reboot_jobs as normal, but if we've reloaded a database during runtime we'll hit this case and set e->lastexit to the current time when we process it. They will then run every 'n' seconds from that point, and a full restart of cron is no longer required to make these jobs work.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kevans created this revision.Apr 16 2019, 2:57 PM

Herald added a reviewer: manpages. · View Herald TranscriptApr 16 2019, 2:57 PM

Herald added a subscriber: imp. · View Herald Transcript

Harbormaster completed remote builds in B23695: Diff 56251.Apr 16 2019, 2:57 PM

Revision has been tested and working properly.

This revision is now accepted and ready to land.Apr 16 2019, 3:32 PM

bapt accepted this revision.Apr 19 2019, 11:23 AM

bjk added a subscriber: bjk.Apr 19 2019, 6:13 PM

bjk added inline comments.

usr.sbin/cron/crontab/crontab.5
254 ↗	(On Diff #56251)	This is an error in the preexisting text, not your change, but while you're here, please s/scheduled specified amount/scheduled the specified number/

allanjude added a subscriber: allanjude.Apr 19 2019, 6:17 PM

allanjude added inline comments.

usr.sbin/cron/crontab/crontab.5
248 ↗	(On Diff #56251)	"that many seconds after completion of the previous invocation of" is much more grammatically pleasing
253 ↗	(On Diff #56251)	It might be good to be more explicit here. Might it overlap every time you reload cron, or is the only risk of overlap when you modify the file containing the @ entry?
254 ↗	(On Diff #56251)	"The first run is scheduled for the specified number of seconds after cron is started or the crontab entry is reloaded" Does that sound better?