[pmg-devel] [PATCH pmg-api master v1] systemd: fix report services failing if triggered to early by timers

[pmg-devel] [PATCH pmg-api master v1] systemd: fix report services failing if triggered to early by timers

[Max R. Carrara]

Currently, the `pmgreport.service` and `pmgspamreport.service` units
might fail if their corresponding timers activate them too early.

To elaborate, both timers have `Persistent=true` in addition to their
`OnCalendar` option. `Persistent=true` means that the timer's service
unit will be triggered immediately when the timer is activated, but
only if it would have been triggered while the timer was inactive [0].

Since the timers are activated relatively early, they might trigger
their service units before postfix.service and postgresql.service have
come up, causing `pmgreport.service`, or `pmgspamreport.service`, or
both of them to fail.

Fix this by letting both service units wait until postfix and postgres
are up, which are necessary for the units to run successfully. Do this
by adding the `After` and `Wants` options for `postfix.service` and
`postgresql.service` to both service units.

Additional context:

While this is somewhat hard to encounter / debug under normal
circumstances, it is possible to make this race condition much more
apparent by adding an arbitrarily long delay to `postgresql.service`
and `postfix.service` by adding an override for each:

 # systemctl edit postgresql.service

Then add the following:

[Service]
ExecStartPre=-sleep 15

Do the same for `postfix.service`.

Afterwards, change both timers to activate a few seconds after every
boot by adding an override for each:

 # systemctl edit pmgreport.timer

Then add the following:

[Timer]
OnCalendar=
OnBootSec=5

Do the same for `pmgspamreport.timer`.

A reboot should now suffice to make the issue reproducible.
Conversely, the issue should not appear if this commit is applied.
(Also, don't forget to remove the overrides again after debugging.)

[0]: `man 5 systemd.timer`

Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
 debian/pmgreport.service     | 4 ++++
 debian/pmgspamreport.service | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/debian/pmgreport.service b/debian/pmgreport.service
index 6b05213..89e25c7 100644
--- a/debian/pmgreport.service
+++ b/debian/pmgreport.service
@@ -1,6 +1,10 @@
 [Unit]
 Description=Send Daily System Report Mail
 ConditionPathExists=/usr/bin/pmgreport
+After=postfix.service
+After=postgresql.service
+Wants=postfix.service
+Wants=postgresql.service
 
 [Service]
 Type=oneshot
diff --git a/debian/pmgspamreport.service b/debian/pmgspamreport.service
index a20214f..2b4f163 100644
--- a/debian/pmgspamreport.service
+++ b/debian/pmgspamreport.service
@@ -1,6 +1,10 @@
 [Unit]
 Description=Send Daily Spam Report Mails
 ConditionPathExists=/usr/bin/pmgqm
+After=postfix.service
+After=postgresql.service
+Wants=postfix.service
+Wants=postgresql.service
 
 [Service]
 Type=oneshot
-- 
2.47.3



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

Re: [pmg-devel] [PATCH pmg-api master v1] systemd: fix report services failing if triggered to early by timers

[Stoiko Ivanov]

Thanks for the patch - and thanks for the attention to detail you took to
the logs on a fresh system - much appreciated!

As said off-list - quickly reading through `systemd.timer(5)`
I'm not sure the `Persistent=true` makes sense for those two particular
timers:
```
...This is useful to catch up on missed runs of the service when the
system was powered down....
```

Most PMG installations are probably not powered down over prolonged
timespans, and even if they are it's probably not helping anyone to get a
spamreport or adminreport covering the time before the system was stopped.
(e.g. `pmgqm` cleans mails which are beyond the quarantine life-time
setting and this runs after the reports are sent out - so it could
probably happen that you get a report and when you view your quarantine
all mails are gone already)

If you find the time - testing if you observe any ill side-effects from
dropping `Persistent=true` (false is the default according to the man-page)
and sending this as a follow-up would be great!

(if you get to that - maybe also add the actual log-messages you saw in the
journal to the commit-message/notes below) as this can help in getting an
overview what's happening)


On Tue, 23 Sep 2025 17:19:24 +0200
"Max R. Carrara" <m.carrara@proxmox.com> wrote:

> Currently, the `pmgreport.service` and `pmgspamreport.service` units
> might fail if their corresponding timers activate them too early.
> 
> To elaborate, both timers have `Persistent=true` in addition to their
> `OnCalendar` option. `Persistent=true` means that the timer's service
> unit will be triggered immediately when the timer is activated, but
> only if it would have been triggered while the timer was inactive [0].
> 
> Since the timers are activated relatively early, they might trigger
> their service units before postfix.service and postgresql.service have
> come up, causing `pmgreport.service`, or `pmgspamreport.service`, or
> both of them to fail.
> 
> Fix this by letting both service units wait until postfix and postgres
> are up, which are necessary for the units to run successfully. Do this
> by adding the `After` and `Wants` options for `postfix.service` and
> `postgresql.service` to both service units.
> 
> Additional context:
> 
> While this is somewhat hard to encounter / debug under normal
> circumstances, it is possible to make this race condition much more
> apparent by adding an arbitrarily long delay to `postgresql.service`
> and `postfix.service` by adding an override for each:
> 
>  # systemctl edit postgresql.service
> 
> Then add the following:
> 
> [Service]
> ExecStartPre=-sleep 15
> 
> Do the same for `postfix.service`.
> 
> Afterwards, change both timers to activate a few seconds after every
> boot by adding an override for each:
> 
>  # systemctl edit pmgreport.timer
> 
> Then add the following:
> 
> [Timer]
> OnCalendar=
> OnBootSec=5
> 
> Do the same for `pmgspamreport.timer`.
> 
> A reboot should now suffice to make the issue reproducible.
> Conversely, the issue should not appear if this commit is applied.
> (Also, don't forget to remove the overrides again after debugging.)
> 
> [0]: `man 5 systemd.timer`
> 
> Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
> ---
>  debian/pmgreport.service     | 4 ++++
>  debian/pmgspamreport.service | 4 ++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/debian/pmgreport.service b/debian/pmgreport.service
> index 6b05213..89e25c7 100644
> --- a/debian/pmgreport.service
> +++ b/debian/pmgreport.service
> @@ -1,6 +1,10 @@
>  [Unit]
>  Description=Send Daily System Report Mail
>  ConditionPathExists=/usr/bin/pmgreport
> +After=postfix.service
> +After=postgresql.service
> +Wants=postfix.service
> +Wants=postgresql.service
>  
>  [Service]
>  Type=oneshot
> diff --git a/debian/pmgspamreport.service b/debian/pmgspamreport.service
> index a20214f..2b4f163 100644
> --- a/debian/pmgspamreport.service
> +++ b/debian/pmgspamreport.service
> @@ -1,6 +1,10 @@
>  [Unit]
>  Description=Send Daily Spam Report Mails
>  ConditionPathExists=/usr/bin/pmgqm
> +After=postfix.service
> +After=postgresql.service
> +Wants=postfix.service
> +Wants=postgresql.service
>  
>  [Service]
>  Type=oneshot



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

Re: [pmg-devel] [PATCH pmg-api master v1] systemd: fix report services failing if triggered to early by timers

[Max R. Carrara]

On Tue Sep 23, 2025 at 5:34 PM CEST, Stoiko Ivanov wrote:
> Thanks for the patch - and thanks for the attention to detail you took to
> the logs on a fresh system - much appreciated!
>
> As said off-list - quickly reading through `systemd.timer(5)`
> I'm not sure the `Persistent=true` makes sense for those two particular
> timers:
> ```
> ...This is useful to catch up on missed runs of the service when the
> system was powered down....
> ```
>
> Most PMG installations are probably not powered down over prolonged
> timespans, and even if they are it's probably not helping anyone to get a
> spamreport or adminreport covering the time before the system was stopped.
> (e.g. `pmgqm` cleans mails which are beyond the quarantine life-time
> setting and this runs after the reports are sent out - so it could
> probably happen that you get a report and when you view your quarantine
> all mails are gone already)
>
> If you find the time - testing if you observe any ill side-effects from
> dropping `Persistent=true` (false is the default according to the man-page)
> and sending this as a follow-up would be great!
>
> (if you get to that - maybe also add the actual log-messages you saw in the
> journal to the commit-message/notes below) as this can help in getting an
> overview what's happening)

Just sent in a v2 addressing all of this! (:

The tl;dr is: `Persistent=true` ensures that reports are being sent even
in the case of a particularly poorly timed downtime / reboot at midnight.

While yes, this isn't ideal if there's a prolonged downtime, most PMG
installations are probably not powered down over prolonged timespans, as
you mentioned. So IMO it's more important to ensure that no reports are
missing instead of sending out potentially useless reports.

Also added some of the logs I've shown you off-list already.

>
>
> On Tue, 23 Sep 2025 17:19:24 +0200
> "Max R. Carrara" <m.carrara@proxmox.com> wrote:
>
> > Currently, the `pmgreport.service` and `pmgspamreport.service` units
> > might fail if their corresponding timers activate them too early.
> > 
> > To elaborate, both timers have `Persistent=true` in addition to their
> > `OnCalendar` option. `Persistent=true` means that the timer's service
> > unit will be triggered immediately when the timer is activated, but
> > only if it would have been triggered while the timer was inactive [0].
> > 
> > Since the timers are activated relatively early, they might trigger
> > their service units before postfix.service and postgresql.service have
> > come up, causing `pmgreport.service`, or `pmgspamreport.service`, or
> > both of them to fail.
> > 
> > Fix this by letting both service units wait until postfix and postgres
> > are up, which are necessary for the units to run successfully. Do this
> > by adding the `After` and `Wants` options for `postfix.service` and
> > `postgresql.service` to both service units.
> >
> > [...]


_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] superseded: [PATCH pmg-api master v1] systemd: fix report services failing if triggered to early by timers

[Max R. Carrara]

v2: https://lore.proxmox.com/pmg-devel/20250923164723.532488-1-m.carrara@proxmox.com/

On Tue Sep 23, 2025 at 5:19 PM CEST, Max R. Carrara wrote:
> Currently, the `pmgreport.service` and `pmgspamreport.service` units
> might fail if their corresponding timers activate them too early.
>
> To elaborate, both timers have `Persistent=true` in addition to their
> `OnCalendar` option. `Persistent=true` means that the timer's service
> unit will be triggered immediately when the timer is activated, but
> only if it would have been triggered while the timer was inactive [0].
>
> Since the timers are activated relatively early, they might trigger
> their service units before postfix.service and postgresql.service have
> come up, causing `pmgreport.service`, or `pmgspamreport.service`, or
> both of them to fail.
>
> Fix this by letting both service units wait until postfix and postgres
> are up, which are necessary for the units to run successfully. Do this
> by adding the `After` and `Wants` options for `postfix.service` and
> `postgresql.service` to both service units.
>
> [...]


_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel