[pmg-devel] [PATCH pmg-api v2 1/1] fix #3450: api: queue: add POST endpoint for batch deletion/delivery
Thomas Lamprecht
t.lamprecht at proxmox.com
Tue Sep 23 15:22:59 CEST 2025
Am 23.09.25 um 11:33 schrieb Hannes Laimer:
> Delivery is done using `postqueue -i <id>`, this is slower than using
> `postsuper -r -`. But `postsuper -r -` would only re-queue the mails,
> they'd also recieve new IDs. Flush should rather be an immediate delivery
> attempt.
>
> Signed-off-by: Hannes Laimer <h.laimer at proxmox.com>
> ---
> for 2000 mails flush took ~8s, since 2000 is the max we can do at once
> through the UI I think this is ok. Alternatively we could also disable
> the Flush button if more than 200 or so a selected, but I don't think
> there is much value in that.
Such info is fine to have recorded in the commit message
>
> v2:
> - single POST endpoitn that takes type(delete/deliver) and list of id's
Fine for delivery, but why not add a new optional ids parameter to the
existing DELETE endpoint?
I mean, I have no strong objection against this, but I'd at least like
to have a reason recorded in the commit message for why that route was
not chosen.
>
> src/PMG/API2/Postfix.pm | 43 +++++++++++++++++++++++++++++++++++++++++
> src/PMG/Postfix.pm | 33 +++++++++++++++++++++++++++++++
> 2 files changed, 76 insertions(+)
>
> diff --git a/src/PMG/API2/Postfix.pm b/src/PMG/API2/Postfix.pm
> index ba0689c..90a74e9 100644
> --- a/src/PMG/API2/Postfix.pm
> +++ b/src/PMG/API2/Postfix.pm
> @@ -335,6 +335,49 @@ __PACKAGE__->register_method({
> },
> });
>
> +__PACKAGE__->register_method({
> + name => 'queue_action',
> + path => 'queue/{queue}',
> + method => 'POST',
> + description => "Perform an action on the given queue IDs (delete/deliver).",
> + proxyto => 'node',
> + permissions => { check => ['admin'] },
> + protected => 1,
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + node => get_standard_option('pve-node'),
> + queue => $queue_name_option,
> + action => {
> + description => 'Operation to perform on the given IDs.',
> + type => 'string',
> + enum => ['delete', 'deliver'],
> + },
> + id => {
"ids" would slightly better signal that this can be more than one.
> + description => 'Queue ID(s), separated by semicolons (;).',
> + type => 'string',
> + pattern => '[A-Za-z0-9]{8,20}(;[A-Za-z0-9]{8,20})*',
hmm, don't we have a format for this or at least an existing regex
we can reuse?
> + },
> + },
> + },
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + my @ids = defined($param->{id}) && length($param->{id})
> + ? split(/\s*;\s*/, $param->{id})
> + : ();
I'd prefer using PVE::Tools' split_list method, which handles most edge
cases already.
> +
> + if ($param->{action} eq 'delete') {
> + PMG::Postfix::delete_queue_ids($param->{queue}, \@ids);
> + } elsif ($param->{action} eq 'deliver') {
> + PMG::Postfix::flush_queue_ids(\@ids);
> + }
> +
> + return undef;
> + },
> +});
> +
> __PACKAGE__->register_method({
> name => 'delete_queue',
> path => 'queue/{queue}',
> diff --git a/src/PMG/Postfix.pm b/src/PMG/Postfix.pm
> index 966130f..dff8ec6 100644
> --- a/src/PMG/Postfix.pm
> +++ b/src/PMG/Postfix.pm
> @@ -221,6 +221,39 @@ sub delete_queue {
> PVE::Tools::run_command($cmd);
> }
>
> +# delete multiple mails by queue IDs
> +sub delete_queue_ids {
> + my ($queue, $ids) = @_;
> +
> + return if !$ids || ref($ids) ne 'ARRAY' || !@$ids;
> +
> + my %seen;
> + my @queue_ids;
> + foreach my $qid (@$ids) {
please prefer for over foreach for new code.
> + next if !$qid;
> + next if $seen{$qid}++;
> + push @queue_ids, $qid;
> + }
> +
> + return if !@queue_ids;
> +
> + my $input = join("\n", @queue_ids) . "\n";
I mean, if you already iterate above, why not assemble the input string
directly there? While sometimes it can be nicer for code readability to
use a intermediate array This is short enough to still be easy enough to
read and grasp.
And btw. as of now you have the $id 4 times in memory:
1. in $ids
2. in %seen (well deduplicated, but still)
3. in @queue_ids
4. in $input
If you want to cope with duplicates then it would be probably even better
to just do
my $unique_qids = { map { $_ => 1 } $ids->@* };
my $input = join("\n", keys $unique_qids->%*) . "\n";
That avoids at least one copy.
> + my $cmd = ['/usr/sbin/postsuper', '-d', '-'];
> + push @$cmd, $queue if defined($queue);
> + PVE::Tools::run_command($cmd, input => $input);
> +}
> +
> +# flush for multiple queue IDs
> +sub flush_queue_ids {
> + my ($ids) = @_;
> +
> + return if !$ids || ref($ids) ne 'ARRAY' || !@$ids;
> +
> + foreach my $qid (@$ids) {
s/foreach/for/
> + PVE::Tools::run_command(['/usr/sbin/postqueue', '-i', $qid]);
> + }
> +}
> +
> sub discard_verify_cache {
> unlink "/var/lib/postfix/verify_cache.db";
>
More information about the pmg-devel
mailing list