Re: cron(8): add '@' interval time specifier

2021-07-15 Thread David Higgs
On Thu, Jul 15, 2021 at 11:20 AM Leo Unglaub  wrote:

> Hey,
> this is a very clean solution to a very common problem that i have. A
> lot of tasks that i run from cron have very different completion times.
> Right now i use the -s [1] option in crontab to make sure only one task
> is running at once and so that they don't overlap if they take longer to
> complete than the interval i schedule them. But the problem with the -s
> workflow is that the interval can get messed up if the interval is
> smaller than the execution period. This will basically "skip" a run.
>
> With your patch this can be avioded in a very clean way. I have your
> patch a try and for my local testing usecase it worked as expected. This
> would clean up a lot of things in my personal workflow. Thank you very
> mich for the patch Job, much appreciated!
>
> Thanks and greetings
> Leo
>
> [1] https://man.openbsd.org/crontab.5#s
>
> On 10/07/2021 16:07, Job Snijders wrote:
> > The below patch adds a new kind of time specifier: an interval (in
> > minutes). When used, cron(8) will schedule the next instance of a job
> > after the previous job has completed and a full interval has passed.
> >
> > A crontab(5) configured as following:
> >
> >  $ crontab -l
> >  @3 sleep 100
> >
> > Will result in this schedule:
> >
> >  Jul 10 13:38:17 vurt cron[96937]: (CRON) STARTUP (V5.0)
> >  Jul 10 13:42:01 vurt cron[79385]: (job) CMD (sleep 100)
> >  Jul 10 13:47:01 vurt cron[3165]: (job) CMD (sleep 100)
> >  Jul 10 13:52:01 vurt cron[40539]: (job) CMD (sleep 100)
> >  Jul 10 13:57:01 vurt cron[84504]: (job) CMD (sleep 100)
> >
> > A use case could be running rpki-client more frequently than once an
> > hour:
> >
> >  @15 -n rpki-client && bgpctl reload
> >
> > The above is equivalent to:
> >
> >  * * * * * -sn sleep 900 && rpki-client && bgpctl reload
> >
> > I borrowed the idea from FreeBSD's cron [1]. A difference between the
> > below changeset and the freebsd implementation is that they specify the
> > interval in seconds, while the below specifies in minutes. I was able
> > to leverage the 'singleton' infrastructure. And removed a comment that
> > reads like a TODO nobody is going to do.
> >
> > Thoughts?
>

Can't this type of problem also be solved with a shell script wrapper?

An infinite loop that does work and then sleeps?
Or perhaps a script that upon work completion, schedules itself in the
future with at(1)?
These could be kicked off initially with a @reboot crontab entry.

Or maybe I'm missing something that cron adds to the equation, in which
case, apologies.

--david


Re: cron(8): add '@' interval time specifier

2021-07-15 Thread Leo Unglaub

Hey,
this is a very clean solution to a very common problem that i have. A 
lot of tasks that i run from cron have very different completion times. 
Right now i use the -s [1] option in crontab to make sure only one task 
is running at once and so that they don't overlap if they take longer to 
complete than the interval i schedule them. But the problem with the -s 
workflow is that the interval can get messed up if the interval is 
smaller than the execution period. This will basically "skip" a run.


With your patch this can be avioded in a very clean way. I have your 
patch a try and for my local testing usecase it worked as expected. This
would clean up a lot of things in my personal workflow. Thank you very 
mich for the patch Job, much appreciated!


Thanks and greetings
Leo

[1] https://man.openbsd.org/crontab.5#s

On 10/07/2021 16:07, Job Snijders wrote:

The below patch adds a new kind of time specifier: an interval (in
minutes). When used, cron(8) will schedule the next instance of a job
after the previous job has completed and a full interval has passed.

A crontab(5) configured as following:

 $ crontab -l
 @3 sleep 100

Will result in this schedule:

 Jul 10 13:38:17 vurt cron[96937]: (CRON) STARTUP (V5.0)
 Jul 10 13:42:01 vurt cron[79385]: (job) CMD (sleep 100)
 Jul 10 13:47:01 vurt cron[3165]: (job) CMD (sleep 100)
 Jul 10 13:52:01 vurt cron[40539]: (job) CMD (sleep 100)
 Jul 10 13:57:01 vurt cron[84504]: (job) CMD (sleep 100)

A use case could be running rpki-client more frequently than once an
hour:

 @15 -n rpki-client && bgpctl reload

The above is equivalent to:

 * * * * * -sn sleep 900 && rpki-client && bgpctl reload

I borrowed the idea from FreeBSD's cron [1]. A difference between the
below changeset and the freebsd implementation is that they specify the
interval in seconds, while the below specifies in minutes. I was able
to leverage the 'singleton' infrastructure. And removed a comment that
reads like a TODO nobody is going to do.

Thoughts?




Re: cron(8): add '@' interval time specifier

2021-07-11 Thread Todd C . Miller
On Sat, 10 Jul 2021 15:54:51 -, Job Snijders wrote:

> > > I borrowed the idea from FreeBSD's cron [1]. A difference between
> > > the below changeset and the freebsd implementation is that they
> > > specify the interval in seconds, while the below specifies in
> > > minutes.
> > 
> > Why be incompatible?  Better have a strong justification.
>
> OpenBSD cron would need to be extended to support 'up to 1 second
> precise' scheduling, perhaps some code can be borrowed from 
> https://github.com/freebsd/freebsd-src/commit/7a5c30c5b67555c9f76a053812ba80a
> e258b8874#diff-f8f4ba67b7df93eb4b436203d0c70670560029bd8c5b9a642691a914aa33d4
> 00
> to accomplish this.

We use ppoll(2) when waiting for the next event so it should be
simple to add support for sub-minute resolution.  Our code differs
greatly from FreeBSD's in this way.

> Alternatively, I can remove the below '* SECONDS_PER_MINUTE' multiplier,
> so that from a syntax perspective we are equivalent to FreeBSD, but our
> cron will have 'one minute precision' regarding when the job is
> executed.

I think maintaining compatibility with FreeBSD is the way to go
since that is where this feature originates.  We can add sub-minute
precision separately.

> entry.c:
> + } else if (*cmd != '\0' &&
> + (interval = strtol(cmd, &endptr, 10)) > 0 &&
> + *endptr == '\0') {
> + e->interval = interval * SECONDS_PER_MINUTE;
> + e->flags |= INTERVAL | SINGLE_JOB;
>
> For me 'one minute' precision is good enough, but if others think it is
> worth putting in the effort to get to '1 second' precision scheduling
> I'm happy to try to hack it.

Currently, cron_sleep() takes a target time in minutes but it doesn't
have to be that way.  We could set the target time based the first
job in the queue or now + 1 minute, whichever is smaller.

 - todd



Re: cron(8): add '@' interval time specifier

2021-07-10 Thread Job Snijders
On Sat, Jul 10, 2021 at 09:05:15AM -0600, Theo de Raadt wrote:
> Job Snijders  wrote:
> > A use case could be running rpki-client more frequently than once an
> > hour:

Perhaps I choose a poor example, because of $work *I* run rpki-client
very often, but do not recommend others to follow that schedule. :)

> After cron has this support, I suspect you will propose that root's
> crontab be changed to use the feature, thus further increasing the
> compute done directly on a router.

It is not my goal to change the root's example crontab. I think the
current 1 hour suggestion is appropriate for anno 2021. I agree the CPU
cost is considerable, we should work to reduce it - not to increase it.

> That then begs the question why cron needs this extension for this
> purpose.

I am not propsing it for 'this' purpose, the feature is for folks that
have jobs of unknown run time who wish to put some distance between two
jobs. I saw the feature described in the freebsd manual and just thought
to myself 'This looks doable, perhaps others like it too!' :)

> > @15 -n rpki-client && bgpctl reload
> > 
> > The above is equivalent to:
> > 
> > * * * * * -sn sleep 900 && rpki-client && bgpctl reload
> 
> Somewhat the same effect, but I think there are big differences in
> behaviour.  The second version creates a process hierarchy which sleeps
> for a long time.  I suspect crontab -e causes drastic difference in
> behaviour when the job is already running.

Good point. I hadn't thought of that.

> So I agree with getting away from the sleep pattern.  I have never
> used it myself, and we should discourage others from using it by
> providing something better.
> 
> > I borrowed the idea from FreeBSD's cron [1]. A difference between
> > the below changeset and the freebsd implementation is that they
> > specify the interval in seconds, while the below specifies in
> > minutes.
> 
> Why be incompatible?  Better have a strong justification.

OpenBSD cron would need to be extended to support 'up to 1 second
precise' scheduling, perhaps some code can be borrowed from 
https://github.com/freebsd/freebsd-src/commit/7a5c30c5b67555c9f76a053812ba80ae258b8874#diff-f8f4ba67b7df93eb4b436203d0c70670560029bd8c5b9a642691a914aa33d400
to accomplish this.

Alternatively, I can remove the below '* SECONDS_PER_MINUTE' multiplier,
so that from a syntax perspective we are equivalent to FreeBSD, but our
cron will have 'one minute precision' regarding when the job is
executed.

entry.c:
+   } else if (*cmd != '\0' &&
+   (interval = strtol(cmd, &endptr, 10)) > 0 &&
+   *endptr == '\0') {
+   e->interval = interval * SECONDS_PER_MINUTE;
+   e->flags |= INTERVAL | SINGLE_JOB;

For me 'one minute' precision is good enough, but if others think it is
worth putting in the effort to get to '1 second' precision scheduling
I'm happy to try to hack it.

Suggestions welcome!

Kind regards,

Job



Re: cron(8): add '@' interval time specifier

2021-07-10 Thread Theo de Raadt
Theo de Raadt  wrote:

> It will do 5 minutes of compute, 15 minutes of pause, and do it all
> again.

A recent rpki-client took longer:

# Processing time 989 seconds (199 seconds user, 155 seconds system)

So that would be 6 minutes cpu, over 16 minutes elapsed.

Followed by a 15 minute pause.  Then do it again?

I am worried by where this is heading.



Re: cron(8): add '@' interval time specifier

2021-07-10 Thread Theo de Raadt
Job Snijders  wrote:

> A use case could be running rpki-client more frequently than once an
> hour:

I want to say I think running rpki-client so often is misguided for
these rpki-processing-on-the-router configurations.

rpki-processing-on-the-router seems like more a technology demonstration
than performant practical solution.  A rpki-client run is a lot of x509
compute, roughly 5 minutes of full cpu.  After cron has this support, I
suspect you will propose that root's crontab be changed to use the
feature, thus further increasing the compute done directly on a router.
It will do 5 minutes of compute, 15 minutes of pause, and do it all
again.  This will have impact on traffic flow through the OpenBSD
software routing stack.

Surely the on-router approach is only acceptable in very narrow
circumstances, and those situations don't need to be updating RPKI every
20 minutes, as I doubt the internet at large is doing anything close to
20 minute updates.

The normal RPKI use pattern should be offline processing (on a different
dedicated system), then uploaded to a router.

How often RPKI is run should be sensitively scaled to the resources
available.

Are we actually helping people by proposing they use 5 minutes of total
cpu compute, every 20 minutes?  I have serious doubts.

That then begs the question why cron needs this extension for this purpose.
 
> @15 -n rpki-client && bgpctl reload
> 
> The above is equivalent to:
> 
> * * * * * -sn sleep 900 && rpki-client && bgpctl reload

Somewhat the same effect, but I think there are big differences in
behaviour.  The second version creates a process hierarchy which sleeps
for a long time.  I suspect crontab -e causes drastic difference in
behaviour when the job is already running.

I hate the sleep pattern, because you can't see what is happening from a
system perspective (ie. in ps), and if root decides to run rpki-client
or various bgpctl commands by hand, you can get surprising results.

So I agree with getting away from the sleep pattern.  I have never used
it myself, and we should discourage others from using it by providing
something better.

> I borrowed the idea from FreeBSD's cron [1]. A difference between the
> below changeset and the freebsd implementation is that they specify the
> interval in seconds, while the below specifies in minutes.

Why be incompatible?  Better have a strong justification.




Re: cron(8): add '@' interval time specifier

2021-07-10 Thread Jason McIntyre
On Sat, Jul 10, 2021 at 02:07:53PM +, Job Snijders wrote:
> Hi all,
> 
> The below patch adds a new kind of time specifier: an interval (in
> minutes). When used, cron(8) will schedule the next instance of a job
> after the previous job has completed and a full interval has passed. 
> 
> A crontab(5) configured as following:
> 
> $ crontab -l
> @3 sleep 100
> 
> Will result in this schedule:
> 
> Jul 10 13:38:17 vurt cron[96937]: (CRON) STARTUP (V5.0)
> Jul 10 13:42:01 vurt cron[79385]: (job) CMD (sleep 100)
> Jul 10 13:47:01 vurt cron[3165]: (job) CMD (sleep 100)
> Jul 10 13:52:01 vurt cron[40539]: (job) CMD (sleep 100)
> Jul 10 13:57:01 vurt cron[84504]: (job) CMD (sleep 100)
> 
> A use case could be running rpki-client more frequently than once an
> hour:
> 
> @15 -n rpki-client && bgpctl reload
> 
> The above is equivalent to:
> 
> * * * * * -sn sleep 900 && rpki-client && bgpctl reload
> 
> I borrowed the idea from FreeBSD's cron [1]. A difference between the
> below changeset and the freebsd implementation is that they specify the
> interval in seconds, while the below specifies in minutes. I was able
> to leverage the 'singleton' infrastructure. And removed a comment that
> reads like a TODO nobody is going to do.
> 
> Thoughts?
> 
> Kind regards,
> 
> Job
> 

hi.

just to be clear - the point here is that you don;t know how long a job
takes to run, so you can't specify the interval in the normal way?

the doc changes look fine, but i have some suggestions:

> Index: crontab.5
> ===
> RCS file: /cvs/src/usr.sbin/cron/crontab.5,v
> retrieving revision 1.41
> diff -u -p -r1.41 crontab.5
> --- crontab.5 18 Apr 2020 17:11:40 -  1.41
> +++ crontab.5 10 Jul 2021 13:38:13 -
> @@ -265,7 +265,7 @@ For example,
>  would cause a command to be run at 4:30 am on the 1st and 15th of each
>  month, plus every Friday.
>  .Pp
> -Instead of the first five fields, one of eight special strings may appear:
> +Instead of the first five fields, one of nine special strings may appear:

instead of being specific about numbers, we could just say "one of these
special strings". then we don;t have to keep changing the text.

>  .Bl -column "@midnight" "meaning" -offset indent
>  .It Sy string Ta Sy meaning
>  .It @reboot Ta Run once, at startup.
> @@ -276,6 +276,14 @@ Instead of the first five fields, one of
>  .It @daily Ta Run every midnight (0 0 * * *).
>  .It @midnight Ta The same as @daily.
>  .It @hourly Ta Run every hour, on the hour (0 * * * *).
> +.It Pf @ Ar minutes Ta The

the field "minutes" could be confusing, because the other fields
are similar type (hourly, daily, etc) and literal, whereas this is
an argument name. would it be clearer to use sth like "n", a more
common arg name for an arbitrary number, which would also match
your text "followed by a numeric value"?

that's not a request, it's a question ;)

> +.Sq @
> +symbol followed by a numeric value has a special notion of running a job
> +after an interval specified in minutes has passed, and the previous
> +instance has completed.
> +The first run is scheduled at a full interval after
> +.Xr cron 8
> +started.

i hate how every other entry here is a succint one-liner, and this
addition is a block. i wonder if we could be super trim here, and
be more expansive using an example. i already feel from the text
that i don;t really understand when i might use this, so an example
might help.

for example:

...
@n  Run after previous job + "n" minutes have elapsed.
...
EXAMPLES
...
# the gories
#
#
@15 -n rpki-client && bgpctl reload

jmc

>  .El
>  .Sh ENVIRONMENT
>  .Bl -tag -width "LOGNAMEXXX"
> @@ -346,7 +354,9 @@ MAILTO=paul
>  5 4 * * sun echo "run at 5 after 4 every sunday"
>  
>  # run hourly at a random time within the first 30 minutes of the hour
> -0~30 * * * *   /usr/libexec/spamd-setup
> +0~30 * * * */usr/libexec/spamd-setup
> +
> +@10 sleep 180 && echo "starts every 13 minutes, implies -s"
>  .Ed
>  .Sh SEE ALSO
>  .Xr crontab 1 ,
> @@ -372,6 +382,8 @@ Random intervals are supported using the
>  character.
>  .It
>  Months or days of the week can be specified by name.
> +.It
> +Interval mode.
>  .It
>  Environment variables can be set in a crontab.
>  .It



cron(8): add '@' interval time specifier

2021-07-10 Thread Job Snijders
Hi all,

The below patch adds a new kind of time specifier: an interval (in
minutes). When used, cron(8) will schedule the next instance of a job
after the previous job has completed and a full interval has passed. 

A crontab(5) configured as following:

$ crontab -l
@3 sleep 100

Will result in this schedule:

Jul 10 13:38:17 vurt cron[96937]: (CRON) STARTUP (V5.0)
Jul 10 13:42:01 vurt cron[79385]: (job) CMD (sleep 100)
Jul 10 13:47:01 vurt cron[3165]: (job) CMD (sleep 100)
Jul 10 13:52:01 vurt cron[40539]: (job) CMD (sleep 100)
Jul 10 13:57:01 vurt cron[84504]: (job) CMD (sleep 100)

A use case could be running rpki-client more frequently than once an
hour:

@15 -n rpki-client && bgpctl reload

The above is equivalent to:

* * * * * -sn sleep 900 && rpki-client && bgpctl reload

I borrowed the idea from FreeBSD's cron [1]. A difference between the
below changeset and the freebsd implementation is that they specify the
interval in seconds, while the below specifies in minutes. I was able
to leverage the 'singleton' infrastructure. And removed a comment that
reads like a TODO nobody is going to do.

Thoughts?

Kind regards,

Job

[1]: 
https://github.com/freebsd/freebsd-src/commit/a08d12d3f2d4f4dabfc01953be696fdc0750da9c#

Index: cron.c
===
RCS file: /cvs/src/usr.sbin/cron/cron.c,v
retrieving revision 1.79
diff -u -p -r1.79 cron.c
--- cron.c  16 Apr 2020 17:51:56 -  1.79
+++ cron.c  10 Jul 2021 13:38:13 -
@@ -273,6 +273,8 @@ run_reboot_jobs(cron_db *db)
SLIST_FOREACH(e, &u->crontab, entries) {
if (e->flags & WHEN_REBOOT)
job_add(e, u);
+   if (e->flags & INTERVAL)
+   e->lastexit = StartTime;
}
}
(void) job_runqueue();
@@ -303,6 +305,12 @@ find_jobs(time_t vtime, cron_db *db, int
 */
TAILQ_FOREACH(u, &db->users, entries) {
SLIST_FOREACH(e, &u->crontab, entries) {
+   if (e->flags & INTERVAL) {
+   if (e->lastexit > 0 &&
+   virtualSecond >= e->lastexit + e->interval)
+   job_add(e, u);
+   continue;
+   }
if (bit_test(e->minute, minute) &&
bit_test(e->hour, hour) &&
bit_test(e->month, month) &&
Index: crontab.5
===
RCS file: /cvs/src/usr.sbin/cron/crontab.5,v
retrieving revision 1.41
diff -u -p -r1.41 crontab.5
--- crontab.5   18 Apr 2020 17:11:40 -  1.41
+++ crontab.5   10 Jul 2021 13:38:13 -
@@ -265,7 +265,7 @@ For example,
 would cause a command to be run at 4:30 am on the 1st and 15th of each
 month, plus every Friday.
 .Pp
-Instead of the first five fields, one of eight special strings may appear:
+Instead of the first five fields, one of nine special strings may appear:
 .Bl -column "@midnight" "meaning" -offset indent
 .It Sy string Ta Sy meaning
 .It @reboot Ta Run once, at startup.
@@ -276,6 +276,14 @@ Instead of the first five fields, one of
 .It @daily Ta Run every midnight (0 0 * * *).
 .It @midnight Ta The same as @daily.
 .It @hourly Ta Run every hour, on the hour (0 * * * *).
+.It Pf @ Ar minutes Ta The
+.Sq @
+symbol followed by a numeric value has a special notion of running a job
+after an interval specified in minutes has passed, and the previous
+instance has completed.
+The first run is scheduled at a full interval after
+.Xr cron 8
+started.
 .El
 .Sh ENVIRONMENT
 .Bl -tag -width "LOGNAMEXXX"
@@ -346,7 +354,9 @@ MAILTO=paul
 5 4 * * sun echo "run at 5 after 4 every sunday"
 
 # run hourly at a random time within the first 30 minutes of the hour
-0~30 * * * *   /usr/libexec/spamd-setup
+0~30 * * * */usr/libexec/spamd-setup
+
+@10 sleep 180 && echo "starts every 13 minutes, implies -s"
 .Ed
 .Sh SEE ALSO
 .Xr crontab 1 ,
@@ -372,6 +382,8 @@ Random intervals are supported using the
 character.
 .It
 Months or days of the week can be specified by name.
+.It
+Interval mode.
 .It
 Environment variables can be set in a crontab.
 .It
Index: do_command.c
===
RCS file: /cvs/src/usr.sbin/cron/do_command.c,v
retrieving revision 1.61
diff -u -p -r1.61 do_command.c
--- do_command.c16 Apr 2020 17:51:56 -  1.61
+++ do_command.c10 Jul 2021 13:38:13 -
@@ -54,7 +54,8 @@ do_command(entry *e, user *u)
 
/* fork to become asynchronous -- parent process is done immediately,
 * and continues to run the normal cron code, which means return to
-* tick().  the child and grandchild don't leave this function, alive.
+* find_jobs().
+* The child and gr