robur-coop / builder

Scheduling build jobs on regular intervals, collecting artifacts
ISC License
13 stars 1 forks source link

Add an interval of "never" to never schedule a job. #34

Closed hannesm closed 11 months ago

hannesm commented 1 year ago

This solves #32 -- at first insertion, the job is scheduled, and then only at Ptime.max. This allows manual scheduling of a job.

as requested by @reynir

reynir commented 11 months ago

Thanks! This works great.

There seems to be a quirk with the timestamp:

info: schedule:
name solo5, opam solo5-tenders next 1999-12-31T23:59:59-00:00, scheduled never
queues: debian-11:

running:

(Ptime.pp_rfc3339 ()) Ptime.max prints 9999-12-31T23:59:59-00:00 so I am a little confused why this is not printed.

In the ASN.1 we use utctime to encode next scheduled build, and the docstring says:

(** [utc_time] is ASN.1 [UTCTime].
    Representable years are 1951–2050. *)
reynir commented 11 months ago

Ok, restarting builder-server results in building all the scheduled: never jobs :/

hannesm commented 11 months ago

ouch. so there's quite some work to be done. also I wasn't aware that Ptime and ASN were using different ranges...

the missing logic piece is upon reading the dumpfile, we need to not schedule the job where period is never...

hannesm commented 11 months ago

@reynir would you mind trying 06e661b? That should avoid scheduling such a period Never job on startup.

hannesm commented 11 months ago

(still we should look into the time representation issue -- i.e. X509 has the following:

  let time =
    let f = function `C1 t -> t | `C2 t -> t
    and g t =
      let (y, _, _) = Ptime.to_date t in
      if y < 2050 then `C1 t else `C2 t in
    map f g (choice2 utc_time generalized_time_no_frac_s)

so maybe we should use generalized_time_no_frac_s?

reynir commented 11 months ago

Yes, sounds good to switch to GeneralizedTime. Some thought should probably be put into backward compatibility.

About the latest commit: I think it will solve the problem, but I suspect the job will be dropped after it's left the queue. For me that would be less desirable when the job involves a custom script. Otherwise I think it would fix the issue.

An idea I had was to map the minimum (or maximum) representable time in UTCTime to Ptime.max.

I will test the commit later - have a nice weekend :-)

reynir commented 11 months ago

I tested the commit, and it does not remove the "never" scheduled jobs. However, I found that it may result in a sort of denial of service: Due to Ptime.max mapping to 1999-12-31T23:59:59-00:00 a restart of builder-server may result in no jobs ever getting scheduled:

$ date; spurv-builder-client info
Mon 25 Sep 2023 17:13:25 CEST
info: schedule:
name solo5, opam solo5-tenders next 1999-12-31T23:59:59-00:00, scheduled never;
name conf-gmp, opam conf-gmp next 2023-09-26T15:05:27-00:00, scheduled daily;
name static-website on debian-11 next 1999-12-31T23:59:59-00:00, scheduled never
queues: debian-11:

running:
hannesm commented 11 months ago

I pushed another commit to store the timestamps in generalized_time format... now that should all be much nicer :) would you mind to test (please remove the never-scheduled old jobs since they'll be with 1999)

reynir commented 11 months ago

Thanks! It seems to work well. I scheduled a job, after removing the old ones, and after it was built I restarted builder and confirmed the timestamp was the same.

$ spurv-builder-client info
info: schedule:
name conf-gmp, opam conf-gmp next 9999-12-31T23:59:59-00:00, scheduled never
queues: debian-11:

running:
name conf-gmp on debian-11,eed3bad3-3ed2-413e-ab5b-9b8d7dbc3437,
2023-09-28T08:07:54-00:00

We could, in another PR, do something about how Ptime.max (or Ptime.truncate ~frac_s:0 Ptime.max) is printed in the next field.