On 8/8/2023 5:53 AM, Phillip Wood wrote:> Hi Stolee
>
> On 07/08/2023 19:51, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <derrickstolee@xxxxxxxxxx>
>> + switch (schedule) {
>> + case SCHEDULE_HOURLY:
>> + schedule_pattern = xstrfmt("*-*-* *:%02d:00", minute);
>> + break;
>> +
>> + case SCHEDULE_DAILY:
>> + schedule_pattern = xstrfmt("*-*-* 0:%02d:00", minute);
>> + break;
>> +
>> + case SCHEDULE_WEEKLY:
>> + schedule_pattern = xstrfmt("Mon 0:%02d:00", minute);
>> + break;
>
> This is not a new issue with this patch but we run the hourly job even
> when we want to run the daily job or the weekly job and we run the daily
> job when we want to run the weekly job.
This is an excellent point! Thanks for bringing it up.
> So only one of these jobs will succeed. The cron entries are careful to
> only run one job at a time, I think it would be worth doing the same
> thing here. I think the using the following format strings would fix this.
>
> Hourly: "Tue..Sun *-*-* 1..23:00:%02d"
> Daily: "Tue..Sun *-*-* 00:00:%02d"
> Weekly: "Mon *-*-* 00:00:%02d"
I would modify this with dropping the "Tue..Sun" from the hourly schedule,
as we still want 23 runs on Mondays.
> It looks like the launchctl schedule has the same issue.
I will take a look at this and consider some additional patches to correct
these issues across both schedulers. Thank you for the attention to detail!
> One thing I've been wondering about which is related to maintenance but
> totally off-topic for this patch is that I think when auto maintenance
> is enabled we stop automatically running "gc" so how do the reflogs get
> expired?
The default maintenance schedule does not include a 'gc' run as it does
not intend to remove any data. Reflog expiration could be considered as a
separate maintenance task that might be useful in the default maintenance
schedule.
Thanks,
-Stolee
