From b6311caae032a5285f785d4862c63c8242cf5538 Mon Sep 17 00:00:00 2001
From: Jared Johnson <jjohnson@efolder.net>
Date: Tue, 27 Jan 2015 11:50:55 -0600
Subject: [PATCH] Pass multiple keys to del() to speed up prune_db()

---
 lib/Qpsmtpd/DB/File/DBM.pm |  8 +++++---
 lib/Qpsmtpd/DB/Redis.pm    |  6 +++---
 plugins/greylisting        |  7 ++++---
 t/qpsmtpd-db-file-dbm.t    | 14 ++++++++++++--
 t/qpsmtpd-db-redis.t       | 24 +++++++++++++++++++++---
 5 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/lib/Qpsmtpd/DB/File/DBM.pm b/lib/Qpsmtpd/DB/File/DBM.pm
index 771f38d..cbbdda1 100644
--- a/lib/Qpsmtpd/DB/File/DBM.pm
+++ b/lib/Qpsmtpd/DB/File/DBM.pm
@@ -159,17 +159,19 @@ sub size {
 }
 
 sub delete {
-    my ( $self, $key ) = @_;
+    my ( $self, @keys ) = @_;
     my $tied = $self->{tied};
     if ( ! $tied ) {
         warn "DBM db not yet set up, delete() failed\n";
         return;
     }
-    if ( ! $key ) {
+    if ( ! @keys ) {
         warn "No key provided, delete() failed\n";
         return;
     }
-    delete $tied->{$key};
+    @keys = grep { exists $tied->{$_} } @keys;
+    delete @$tied{@keys};
+    return scalar @keys;
 }
 
 sub flush {
diff --git a/lib/Qpsmtpd/DB/Redis.pm b/lib/Qpsmtpd/DB/Redis.pm
index 7b96f0d..06d43c6 100644
--- a/lib/Qpsmtpd/DB/Redis.pm
+++ b/lib/Qpsmtpd/DB/Redis.pm
@@ -91,12 +91,12 @@ sub set {
 }
 
 sub delete {
-    my ( $self, $key ) = @_;
-    if ( ! $key ) {
+    my ( $self, @keys ) = @_;
+    if ( ! @keys ) {
         warn "No key provided, delete() failed\n";
         return;
     }
-    return $self->redis->del($key);
+    return $self->redis->del(@keys);
 }
 
 sub get_keys { shift->redis->keys('*') }
diff --git a/plugins/greylisting b/plugins/greylisting
index a30aa00..fb3b05f 100644
--- a/plugins/greylisting
+++ b/plugins/greylisting
@@ -489,7 +489,7 @@ sub prune_db {
     $self->db->lock or return;
     my $count = $self->db->size;
 
-    my $pruned = 0;
+    my @to_delete;
     my $greylist = {};
     my @keys = $self->db->get_keys or return;
     @$greylist{ @keys } = ( $self->db->mget(@keys) );
@@ -497,9 +497,10 @@ sub prune_db {
         my ($ts) = split /:/, delete $greylist->{$key};
         my $age = $self->now - $ts;
         next if $age < $self->{_args}{white_timeout};
-        $pruned++;
-        $self->db->delete($key);
+        push @to_delete, $key;
     }
+    return if ! @to_delete;
+    my $pruned = $self->db->delete(@to_delete);
     $self->db->unlock;
     $self->log(LOGINFO, "pruned $pruned of $count DB entries");
 }
diff --git a/t/qpsmtpd-db-file-dbm.t b/t/qpsmtpd-db-file-dbm.t
index a5adce6..d3b215e 100644
--- a/t/qpsmtpd-db-file-dbm.t
+++ b/t/qpsmtpd-db-file-dbm.t
@@ -56,8 +56,18 @@ sub __delete {
     $db->flush;
     $db->set( oink  => 1 );
     $db->set( quack => 1 );
-    $db->delete('quack');
-    is( join( '|', $db->get_keys ), 'oink', 'delete() removes key' );
+    $db->set( woof  => 1 );
+    $db->set( moo   => 1 );
+    is( $db->delete('quack'), 1,
+        'delete() return value when removing a single key' );
+    is( join( '|', sort $db->get_keys ), 'moo|oink|woof',
+        'delete() removes a single key' );
+    is( $db->delete(qw( moo oink )), 2,
+        'delete() return value when removing a single key' );
+    is( join( '|', sort $db->get_keys ), 'woof',
+        'delete() removes two keys' );
+    is( $db->delete('noop'), 0,
+        'delete() return value when removing a non-existent key' );
     $db->unlock;
 }
 
diff --git a/t/qpsmtpd-db-redis.t b/t/qpsmtpd-db-redis.t
index 4aeff47..9a74222 100644
--- a/t/qpsmtpd-db-redis.t
+++ b/t/qpsmtpd-db-redis.t
@@ -115,8 +115,19 @@ sub __delete {
     $redis->flushdb;
     $redis->set( oink  => 1 );
     $redis->set( quack => 1 );
-    $db->delete('quack');
-    is( join( '|', $redis->keys('*') ), 'oink', 'delete() removes key' );
+    $redis->set( woof  => 1 );
+    $redis->set( moo   => 1 );
+
+    is( $db->delete('quack'), 1,
+        'delete() return value when removing a single key' );
+    is( join( '|', sort $redis->keys('*') ), 'moo|oink|woof',
+        'delete() removes a single key' );
+    is( $db->delete(qw( moo oink )), 2,
+        'delete() return value when removing a single key' );
+    is( join( '|', sort $redis->keys('*') ), 'woof',
+        'delete() removes two keys' );
+    is( $db->delete('noop'), 0,
+        'delete() return value when removing a non-existent key' );
 }
 
 sub __get_keys {
@@ -158,7 +169,14 @@ sub select   { $_[0]->{selected} = $_[1] }
 sub dbsize   { scalar keys %{ $_[0]->fakestore }   }
 sub get      { $_[0]->fakestore->{ $_[1] }         }
 sub set      { $_[0]->fakestore->{ $_[1] } = $_[2] }
-sub del      { delete $_[0]->fakestore->{ $_[1] }  }
+
+sub del {
+    my ($self,@keys) = @_;
+    my $f = $self->fakestore;
+    @keys = grep { exists $f->{$_} } @keys;
+    delete @$f{ @keys };
+    return scalar @keys;
+}
 
 sub mget {
     my ($self,@keys) = @_;