[PATCH] pop3: remove untouched rows on QUIT/disconnect

[PATCH] pop3: remove untouched rows on QUIT/disconnect

[Eric Wong]

Some POP3 clients may connect and never retrieve messages nor
trigger deletes.  In that case, save some storage by removing
unused rows from the `deletes' and `users' tables.
---
 lib/PublicInbox/POP3.pm  | 37 ++++++++++++++++++++++++++------
 lib/PublicInbox/POP3D.pm |  6 ++++++
 t/pop3d.t                | 46 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 7 deletions(-)

diff --git a/lib/PublicInbox/POP3.pm b/lib/PublicInbox/POP3.pm
index 53fb2e05..c993e558 100644
--- a/lib/PublicInbox/POP3.pm
+++ b/lib/PublicInbox/POP3.pm
@@ -301,6 +301,27 @@ sub close {
 	$self->SUPER::close;
 }
 
+# must be called inside a state_dbh transaction with flock held
+sub __cleanup_state {
+	my ($self, $txn_id) = @_;
+	my $user_id = $self->{user_id} // die 'BUG: no {user_id}';
+	$self->{pop3d}->{-state_dbh}->prepare_cached(<<'')->execute($txn_id);
+DELETE FROM deletes WHERE txn_id = ? AND uid_dele = -1
+
+	my $sth = $self->{pop3d}->{-state_dbh}->prepare_cached(<<'');
+SELECT COUNT(*) FROM deletes WHERE user_id = ?
+
+	$sth->execute($user_id);
+	my $nr = $sth->fetchrow_array;
+	if ($nr == 0) {
+		$sth = $self->{pop3d}->{-state_dbh}->prepare_cached(<<'');
+DELETE FROM users WHERE user_id = ?
+
+		$sth->execute($user_id);
+	}
+	$nr;
+}
+
 sub cmd_quit {
 	my ($self) = @_;
 	if (defined(my $txn_id = $self->{txn_id})) {
@@ -308,23 +329,25 @@ sub cmd_quit {
 		if (my $exp = delete $self->{expire}) {
 			mark_dele($self, $_) for unpack('S*', $exp);
 		}
+		my $keep = 1;
 		my $dbh = $self->{pop3d}->{-state_dbh};
 		my $lk = $self->{pop3d}->lock_for_scope;
-		my $sth;
 		$dbh->begin_work;
 
-		if (defined $self->{txn_max_uid}) {
-			$sth = $dbh->prepare_cached(<<'');
+		if (defined(my $max = $self->{txn_max_uid})) {
+			$dbh->prepare_cached(<<'')->execute($max, $txn_id, $max)
 UPDATE deletes SET uid_dele = ? WHERE txn_id = ? AND uid_dele < ?
 
-			$sth->execute($self->{txn_max_uid}, $txn_id,
-					$self->{txn_max_uid});
+		} else {
+			$keep = $self->__cleanup_state($txn_id);
 		}
-		$sth = $dbh->prepare_cached(<<'');
+		$dbh->prepare_cached(<<'')->execute(time, $user_id) if $keep;
 UPDATE users SET last_seen = ? WHERE user_id = ?
 
-		$sth->execute(time, $user_id);
 		$dbh->commit;
+		# we MUST do txn_id F_UNLCK here inside ->lock_for_scope:
+		$self->{did_quit} = 1;
+		$self->{pop3d}->unlock_mailbox($self);
 	}
 	$self->write(\"+OK public-inbox POP3 server signing off\r\n");
 	$self->close;
diff --git a/lib/PublicInbox/POP3D.pm b/lib/PublicInbox/POP3D.pm
index 764f9ffe..7432a964 100644
--- a/lib/PublicInbox/POP3D.pm
+++ b/lib/PublicInbox/POP3D.pm
@@ -245,6 +245,12 @@ SELECT txn_id,uid_dele FROM deletes WHERE user_id = ? AND mailbox_id = ?
 sub unlock_mailbox {
 	my ($self, $pop3) = @_;
 	my $txn_id = delete($pop3->{txn_id}) // return;
+	if (!$pop3->{did_quit}) { # deal with QUIT-less disconnects
+		my $lk = $self->lock_for_scope;
+		$self->{-state_dbh}->begin_work;
+		$pop3->__cleanup_state($txn_id);
+		$self->{-state_dbh}->commit;
+	}
 	delete $self->{txn_locks}->{$txn_id}; # same worker
 
 	# other workers
diff --git a/t/pop3d.t b/t/pop3d.t
index e5d53767..7248c03f 100644
--- a/t/pop3d.t
+++ b/t/pop3d.t
@@ -60,6 +60,14 @@ my $oldc = Net::POP3->new(@old_args);
 my $locked_mb = ('e'x32)."\@$group";
 ok($oldc->apop("$locked_mb.0", 'anonymous'), 'APOP to old');
 
+my $dbh = DBI->connect("dbi:SQLite:dbname=$tmpdir/p3state/db.sqlite3",'','', {
+	AutoCommit => 1,
+	RaiseError => 1,
+	PrintError => 0,
+	sqlite_use_immediate_transaction => 1,
+	sqlite_see_if_its_a_number => 1,
+});
+
 { # locking within the same process
 	my $x = Net::POP3->new(@old_args);
 	ok(!$x->apop("$locked_mb.0", 'anonymous'), 'APOP lock failure');
@@ -146,11 +154,26 @@ for my $args (
 		ok(!$np3->apop($mailbox, 'anonymous'), "APOP $mailbox reject");
 		ok($np3->quit, "QUIT after APOP fail $mailbox");
 	}
+
+	# we do connect+QUIT bumps to try ensuring non-QUIT disconnects
+	# get processed below:
 	for my $mailbox ($group, "$group.0") {
 		my $u = ('f'x32)."\@$mailbox";
+		undef $np3;
+		ok(Net::POP3->new(@np3_args)->quit, 'connect+QUIT bump');
 		$np3 = Net::POP3->new(@np3_args);
+		my $n0 = $dbh->selectrow_array('SELECT COUNT(*) FROM deletes');
+		my $u0 = $dbh->selectrow_array('SELECT COUNT(*) FROM users');
 		ok($np3->user($u), "UUID\@$mailbox accept");
 		ok($np3->pass('anonymous'), 'pass works');
+		my $n1 = $dbh->selectrow_array('SELECT COUNT(*) FROM deletes');
+		is($n1 - $n0, 1, 'deletes bumped while connected');
+		ok($np3->quit, 'client QUIT');
+
+		$n1 = $dbh->selectrow_array('SELECT COUNT(*) FROM deletes');
+		is($n1, $n0, 'deletes row gone on no-op after QUIT');
+		my $u1 = $dbh->selectrow_array('SELECT COUNT(*) FROM users');
+		is($u1, $u0, 'users row gone on no-op after QUIT');
 
 		$np3 = Net::POP3->new(@np3_args);
 		ok($np3->user($u), "UUID\@$mailbox accept");
@@ -163,9 +186,32 @@ for my $args (
 		ok($_ > 0, 'bytes in LIST result') for values %$list;
 		like($_, qr/\A[a-z0-9]{40,}\z/,
 			'blob IDs in UIDL result') for values %$uidl;
+		ok($np3->quit, 'QUIT after LIST+UIDL');
+		$n1 = $dbh->selectrow_array('SELECT COUNT(*) FROM deletes');
+		is($n1, $n0, 'deletes row gone on no-op after LIST+UIDL');
+		$n0 = $n1;
+
+		$np3 = Net::POP3->new(@np3_args);
+		ok($np3->user($u), "UUID\@$mailbox accept");
+		ok($np3->pass('anonymous'), 'pass works');
+		undef $np3; # QUIT-less disconnect
+		ok(Net::POP3->new(@np3_args)->quit, 'connect+QUIT bump');
+
+		$u1 = $dbh->selectrow_array('SELECT COUNT(*) FROM users');
+		is($u1, $u0, 'users row gone on QUIT-less disconnect');
+		$n1 = $dbh->selectrow_array('SELECT COUNT(*) FROM deletes');
+		is($n1, $n0, 'deletes row gone on QUIT-less disconnect');
+		$n0 = $n1;
 
 		$np3 = Net::POP3->new(@np3_args);
 		ok(!$np3->apop($u, 'anonumuss'), 'APOP wrong pass reject');
+		$n1 = $dbh->selectrow_array('SELECT COUNT(*) FROM deletes');
+		is($n1, $n0, 'deletes row not bumped w/ wrong pass');
+		undef $np3; # QUIT-less disconnect
+		ok(Net::POP3->new(@np3_args)->quit, 'connect+QUIT bump');
+
+		$n1 = $dbh->selectrow_array('SELECT COUNT(*) FROM deletes');
+		is($n1, $n0, 'deletes row not bumped w/ wrong pass');
 
 		$np3 = Net::POP3->new(@np3_args);
 		ok($np3->apop($u, 'anonymous'), "APOP UUID\@$mailbox");