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

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 3002 bytes --]

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..".

> 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

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]