Re: [pmg-devel] [PATCH pmg-api/docs] make filter timeout configurable

[Stoiko Ivanov <s.ivanov@proxmox.com>]

Thank you very much for the review, testing and the discussion!

Just summing up some points from the talks:

On Fri, 12 Jan 2024 09:35:09 +0100
Dominik Csapak <d.csapak@proxmox.com> wrote:

> gave this a look & spin, and it works as intended
> 
> writing here what we discussed off-list:
> 
> there is still the same race when processing + the actual action
> takes longer than the time out, but that is no trivially fixable
One option to handle this would be to 'dry-run' the (final) actions
for each target-group, assuming that they work, i.e.:
that the mail is considered 'delivered' for accept and quarantine, and
'blocked' for block. with that information pmg-smtp-filter could
answer to the sending postfix, and then start processing the actions.
It would only need to generate a bounce if an accepted mail is not
successfully sent to the postfix on 10025.

But this entails refactoring PMG::SMTP and pmg-smtp-filter -
pmg-smtp-filter would need to return after processing who,what,when
and then SMTP would need to call a second function which deals with
running the actions, writing statistics, ...)
> 
> for "normal" timeouts (e.g. the default of 600s) this should not
> matter much, except for pathological cases where e.g. writing
> to the quarantine takes an absurd amount of time

In case a quarantine insert takes so long (and the processing of a mail
took 599s before) - the issue of getting a mail multiple times is probably
not your largest problem.

> 
> secondly, we probably should adapt the timeouts for virus+custom check scripts
> to the configured one too (or e.g. half) but that can be done afterwards
Thinking about it - I think min($filter_timeout, 5*60) (5 minutes is the
current timeout for virus+custom-checks, might make for a robust change.
splitting the timeout seems odd (just because clamAV needs more than
half the timeout, does not mean that custom checks + avast (if configured
at all) or spamassassin won't finish in a very short time.

as an alternative (which I'd only consider if someone actually runs into
such an issue) - we might introduce timeouts for each of the potentially
long-running things.


> and does not impact the actual issue here
> (except that probably the command runs unnecessarily long)
> 
> IMHO we should still mark the increased timeout for before queue filter
> in the next release notes, since that can be a bit unexpected
Good point - definitely worth mentioning

> 
> so from my side, this series is:
> 
> Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
> Tested-by: Dominik Csapak <d.csapak@proxmox.com>
>