diff mbox series

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

Message ID 20240318153257.27451-3-mg@max.gautier.name (mailing list archive)
State Superseded
Headers show
Series maintenance: use packaged systemd units | expand

Commit Message

Max Gautier March 18, 2024, 3:31 p.m. UTC 
Ensures that:
- git maintenance timers have a fixed time interval between execution.
- the three timers are not executed at the same time.

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.

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(+)

Comments

Patrick Steinhardt March 21, 2024, 12:37 p.m. UTC | #1 
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
> 
>
Max Gautier March 21, 2024, 1:13 p.m. UTC | #2 
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
> > 
> >
diff mbox series

Patch

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)
 
 [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)
 
 [Timer]
 OnCalendar=%i
 Persistent=true
+RandomizedDelaySec=1800
+FixedRandomDelay=true
 
 [Install]
 WantedBy=timers.target