Re: [RFC PATCH 2/5] maintenance: add fixed random delay to systemd timers

[Max Gautier <mg@max.gautier.name>]

On Thu, Mar 21, 2024 at 01:37:35PM +0100, Patrick Steinhardt wrote:
> On Mon, Mar 18, 2024 at 04:31:16PM +0100, Max Gautier wrote:
> > Ensures that:
> > - git maintenance timers have a fixed time interval between execution.
> > - the three timers are not executed at the same time.
> 
> Commit messages are typically structured so that you first explain what
> the problem is that you're trying to solve, and then you explain how you
> solve it. Your commit message is missing the first part.
> 
> > This is intended to implement an alternative to the two followings
> > commits:
> > c97ec0378b (maintenance: fix systemd schedule overlaps, 2023-08-10)
> > daa787010c (maintenance: use random minute in systemd scheduler, 2023-08-10)
> > 
> > Instead of manually adding a specific minute (which is reset on each
> > invocation of `git maintenance start`), we use systemd timers
> > RandomizedDelaySec and FixedRandomDelay functionalities.
> 
> I think it would help to not list commits, but put the commit references
> in a paragraph. Something in the spirit of "In commit c97ec0378b we
> already tried to address this issue in such and such a way, but that is
> quite limiting due to that and that. Similarly, in commit daa787010c..".
> 

Ok I see how that would work.
Stating the limitations of the implemenation added by those commits
would be the problem we're trying to solve, then the proposed solution.

> > From man systemd.timer:
> > >FixedRandomDelay=
> > >  Takes a boolean argument. When enabled, the randomized offset
> > >  specified by RandomizedDelaySec= is reused for all firings of the
> > >  same timer. For a given timer unit, **the offset depends on the
> > >  machine ID, user identifier and timer name**, which means that it is
> > >  stable between restarts of the manager. This effectively creates a
> > >  fixed offset for an individual timer, reducing the jitter in
> > >  firings of this timer, while still avoiding firing at the same time
> > >  as other similarly configured timers.
> > 
> > -> which is exactly the use case for git-maintenance timers.
> > 
> > Signed-off-by: Max Gautier <mg@max.gautier.name>
> > ---
> >  systemd/user/git-maintenance@.service | 1 +
> >  systemd/user/git-maintenance@.timer   | 3 +++
> >  2 files changed, 4 insertions(+)
> > 
> > diff --git a/systemd/user/git-maintenance@.service b/systemd/user/git-maintenance@.service
> > index 87ac0c86e6..f949e1a217 100644
> > --- a/systemd/user/git-maintenance@.service
> > +++ b/systemd/user/git-maintenance@.service
> > @@ -1,5 +1,6 @@
> >  [Unit]
> >  Description=Optimize Git repositories data
> > +Documentation=man:git-maintenance(1)
> 
> This change feels unrelated and should likely go into the first commit.
> 
> >  
> >  [Service]
> >  Type=oneshot
> > diff --git a/systemd/user/git-maintenance@.timer b/systemd/user/git-maintenance@.timer
> > index 40fbc77a62..667c5998ba 100644
> > --- a/systemd/user/git-maintenance@.timer
> > +++ b/systemd/user/git-maintenance@.timer
> > @@ -1,9 +1,12 @@
> >  [Unit]
> >  Description=Optimize Git repositories data
> > +Documentation=man:git-maintenance(1)
> 
> Same.
> 
> Patrick

Ack, will do.

> 
> >  [Timer]
> >  OnCalendar=%i
> >  Persistent=true
> > +RandomizedDelaySec=1800
> > +FixedRandomDelay=true
> >  
> >  [Install]
> >  WantedBy=timers.target
> > -- 
> > 2.44.0
> > 
> > 



-- 
Max Gautier