all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fuhrmann, Markus" <markus.fuhrmann@naheimst.de>
To: Stoiko Ivanov <s.ivanov@proxmox.com>,
	Thomas Lamprecht <t.lamprecht@proxmox.com>
Cc: "pmg-devel@lists.proxmox.com" <pmg-devel@lists.proxmox.com>
Subject: Re: [pmg-devel] [PATCH] fix #3164: api: quarantine: allow to return spam from all users
Date: Mon, 22 Mar 2021 14:12:57 +0000	[thread overview]
Message-ID: <AM0PR0402MB342562FFB641D0AC3135F46DEC659@AM0PR0402MB3425.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20210322150615.2894ba90@rosa.proxmox.com>

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

Yes! Thanks for your support and the implementation of this long awaited feature!

Regards,
Markus








Unternehmensgruppe Nassauische Heimstätte | Wohnstadt: Geschäftsführung: Dr. Thomas Hain (Leitender Geschäftsführer), Monika Fontaine-Kretschmer, Dr. Constantin Westphal: Nassauische Heimstätte GmbH | Schaumainkai 47 | 60596 Frankfurt am Main | Registergericht Frankfurt am Main | HRB 6712 | Aufsichtsratsvorsitz: Staatsminister Tarek Al-Wazir, NH ProjektStadt GmbH Schaumainkai 47 | 60596 Frankfurt am Main | Registergericht Frankfurt am Main | HRB 97395 | Aufsichtsratsvorsitz: Staatsminister Tarek Al-Wazir, Bauland-Offensive Hessen GmbH | Schaumainkai 47 | 60596 Frankfurt am Main | Registergericht Frankfurt am Main | HRB 109334 | Aufsichtsratsvorsitz: Staatsminister Tarek Al-Wazir, Wohnstadt GmbH | Wolfsschlucht 18 | 34117 Kassel | Registergericht Kassel | HRB 2157, MET GmbH | Wolfsschlucht 18 | 34117 Kassel | Registergericht Kassel | HRB 5898

Diese E-Mail ist vertraulich. Wenn Sie nicht der rechtmäßige Empfänger sind, dürfen Sie den Inhalt weder kopieren, verbreiten noch benutzen. Sollten Sie diese E-Mail versehentlich erhalten haben, informieren Sie mich bitte via E-Mail-Antwort und löschen Sie sie anschließend. Danke!



-----Ursprüngliche Nachricht-----
Von: pmg-devel <pmg-devel-bounces@lists.proxmox.com> Im Auftrag von Stoiko Ivanov
Gesendet: Montag, 22. März 2021 15:06
An: Thomas Lamprecht <t.lamprecht@proxmox.com>
Cc: pmg-devel@lists.proxmox.com
Betreff: Re: [pmg-devel] [PATCH] fix #3164: api: quarantine: allow to return spam from all users

Huge thanks for tackling this feature quite a few users have asked and have been waiting for!


On Mon, 22 Mar 2021 10:00:44 +0100
Thomas Lamprecht <t.lamprecht@proxmox.com> wrote:

> The pmail was only checked for the spam quarantine call, and there
> mainly to ensure that the quarantine user only can check their own
> mails. Make the pmail parameter also optional for this quarantine
> related endpoint as long as one has a role other than quser.
> This allows to query all spam quarantine entries from all pmails at
> once, providing the backend side to address #3164.
>
> The main argument against this was performance, but postgres can
> handle even hundreds of thousands of rows rather fine, it's a high
> performant database after all and this is quite the simple query (no
> joins, functions on columns or nested queries).
agreed @postgres being a performant dbms - small nit - the quarantine query has a join (it's quering cmsreceivers and cmailstore in one query)

On my machine a 30k mail test-set even showed some room for improvement with the postgres parameters (sorting resorting to a temp-file for < 10Megs sounds like a bad tradeoff) - however this is a different topic and as you said - users with huge deployments using quarantine, hopefully know how to tweak a few postgres parameters.


quickly gave your patches a spin - it works quite well on my test-setup (currently at 48k mails selecting 'all' takes 5s in the GUI (and yields 10.4MB of results)

The changes to pmg-api look good!
The ones to pmg-gui and proxmox-widet-toolkit as well (but I'm not the best judge of java-script code)

with that said:
Tested-By: Stoiko Ivanov <s.ivanov@proxmox.com>
Reviewed-By: Stoiko Ivanov <s.ivanov@proxmox.com>


>
> Some data, 45k records on a read limited disk, gathered with EXPLAIN
> ANALYZE commands:
>
> All caches dropped and fresh start: 440ms Running for a bit with
> caches warm:  55ms
>
> A simple extrapolation would mean that for half a million rows we
> would spent about 5s in the DB, which is not too bad considering our
> hard limit of 30s per requests, and the overhead of perl/https seems
> to put the limit on my not so beefy VM at at least ~1.5 million rows
> from a *cold* cache, which seems plenty (default 7 days keep window
> and an avg. of 10 spam mails per day means >21k qusers). And with warm
> caches and a beefier machine one can probably gain one or even two
> order of magnitudes here.
>
> And at the end, no mail admin is forced to use this and if they run a
> setup with tens of millions of spam in their spam-keep time window,
> well, they really should not be surprised that querying all has a
> certain cost.
>
> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> ---
>  src/PMG/API2/Quarantine.pm | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/PMG/API2/Quarantine.pm b/src/PMG/API2/Quarantine.pm
> index 56f248d..666dffa 100644
> --- a/src/PMG/API2/Quarantine.pm
> +++ b/src/PMG/API2/Quarantine.pm
> @@ -597,14 +597,14 @@ my $quarantine_api = sub {
>
>      my $rpcenv = PMG::RESTEnvironment->get();
>      my $authuser = $rpcenv->get_user();
> +    my $role = $rpcenv->get_role();
>
>      my $start = $param->{starttime} // (time - 86400);
>      my $end = $param->{endtime} // ($start + 86400);
>
>      my $select;
>      my $pmail;
> -    if ($check_pmail) {
> -	my $role = $rpcenv->get_role();
> +    if ($check_pmail || $role eq 'quser') {
>  	$pmail = $verify_optional_pmail->($authuser, $role, $param->{pmail});
>  	$select = "SELECT * " .
>  		  "FROM CMailStore, CMSReceivers WHERE " .
> @@ -700,7 +700,7 @@ __PACKAGE__->register_method ({
>      },
>      code => sub {
>  	my ($param) = @_;
> -	return $quarantine_api->($param, 'S', 1);
> +	return $quarantine_api->($param, 'S', defined($param->{pmail}));
>      }});
>
>  __PACKAGE__->register_method ({



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


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 7407 bytes --]

  reply	other threads:[~2021-03-22 14:18 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-22  9:00 Thomas Lamprecht
2021-03-22  9:00 ` [pmg-devel] [PATCH] override deselection in CheckboxModel to improve performance Thomas Lamprecht
2021-03-22  9:00 ` [pmg-devel] [PATCH] fix #3164: allow one to display all quarantined spam mails Thomas Lamprecht
2021-03-22 15:46   ` Dominik Csapak
2021-03-22 14:06 ` [pmg-devel] [PATCH] fix #3164: api: quarantine: allow to return spam from all users Stoiko Ivanov
2021-03-22 14:12   ` Fuhrmann, Markus [this message]
2021-03-22 16:38   ` Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AM0PR0402MB342562FFB641D0AC3135F46DEC659@AM0PR0402MB3425.eurprd04.prod.outlook.com \
    --to=markus.fuhrmann@naheimst.de \
    --cc=pmg-devel@lists.proxmox.com \
    --cc=s.ivanov@proxmox.com \
    --cc=t.lamprecht@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal