Re: [PATCH v2 0/6] maintenance: use packaged systemd units

> Hi Max
> 
> On 25/03/2024 08:32, Max Gautier wrote:
> > On Sun, Mar 24, 2024 at 02:54:58PM +0000, Phillip Wood wrote:
> > > Hi Max
> > > 
> > > On 22/03/2024 22:11, Max Gautier wrote:
> > > > * Distribute the systemd timers used by the `git maintenance start` with
> > > >     the systemd scheduler as part of git, rather than writing them in
> > > >     $XDG_CONFIG_HOME.
> > > > 
> > > > This allows users to override the units if they wish, and is more
> > > > in-line with the usual practices of distribution for systemd units.
> > > 
> > > Thanks for suggesting this, I think this is a useful change, but the
> > > implementation could be improved.
> > > 
> > > > We also move away from using the random minute, and instead rely on
> > > > systemd features to achieve the same goal (see patch 2). This allows us
> > > > to go back to using unit templating for the timers. This is also a
> > > > prerequisite to have static unit files.
> > > > 
> > > > Note that even if we really need more specific OnCalendar= settings for
> > > > each timer, we should still do it that way, but instead distribute
> > > > override alongside the template, for instance for weekly:
> > > > 
> > > > /usr/lib/systemd-user/git-maintenance@daily.timer.d/override.conf:
> > > > [Timer]
> > > > OnCalendar=<daily specific calendar spec>
> > > 
> > > We should definitely do that. Using systemd's random delay does not prevent
> > > the different maintenance jobs from running concurrently as one job may be
> > > started before a previous job has finished. It is important to only have one
> > > job running at a time because the first thing "git maintenance run" does is
> > > to try and acquire a lock file so if the hourly job is running when the
> > > daily jobs tries to start the daily job will not be run.
> > 
> > Thinking about that, it occurs to me that the current scheme does not
> > prevent concurrent execution either: the timers all use Persistent=true,
> > which means they can fire concurrently on machine boot, if two or more
> > would have been triggered during the time the machine was powered off
> > (or just the user logged out, since it's a user unit).
> 
> Interesting, I wonder if the other schedulers suffer from the same problem.

> 
> > So maybe there should be a more robust mechanism to avoid concurrent
> > execution ? I assume from what you say above the lock is acquired in a
> > non-blocking way. Could going to a blocking one be a solution ?
> 
> It is possible to wait on a lock file but I'd be worried about building up
> an endless queue of processes if the process holding the lock file crashed
> leaving it in place without anyway to automatically remove it.
> 

> I don't think we need to solve that problem as part of this patch series but
> we should take care not to make it worse. Long term we may be better
> scheduling a single job and have "git maintenance run" decide which jobs to
> run based on the last time it run, rather than trying to schedule different
> jobs with the os scheduler.
> 
> > > As the daily job is
> > > a superset of the hourly job and the weekly job is a superset of the daily
> > > job so it does not make sense to run more than one job per hour.
> > 
> > Is that set in stone, or could they perform disjoint set of tasks
> > instead ?
> 
> All of the schedulers are set up to run a single job each hour, I don't see
> why we'd start running disjoint sets of tasks in the different jobs.