[PATCH 1/2] msgmap: fix use of transactions

[Eric Wong <e@80x24.org>]

We want transactions to be the responsibility of the
caller when possible; this fixes the potential for
the msgmap to internally become inconsistent when
using it from inside searchidx.
---
 lib/PublicInbox/Msgmap.pm    | 24 +++++++++---------------
 lib/PublicInbox/SearchIdx.pm | 11 +++--------
 2 files changed, 12 insertions(+), 23 deletions(-)

diff --git a/lib/PublicInbox/Msgmap.pm b/lib/PublicInbox/Msgmap.pm
index 8fe17a9..2583ff4 100644
--- a/lib/PublicInbox/Msgmap.pm
+++ b/lib/PublicInbox/Msgmap.pm
@@ -33,7 +33,9 @@ sub new {
 
 	if ($writable) {
 		create_tables($dbh);
+		$dbh->begin_work;
 		$self->created_at(time) unless $self->created_at;
+		$dbh->commit;
 	}
 	$self;
 }
@@ -51,22 +53,14 @@ sub meta_accessor {
 	defined $value or
 		return $dbh->selectrow_array(meta_select, undef, $key);
 
-	$dbh->begin_work;
-	eval {
-		$prev = $dbh->selectrow_array(meta_select, undef, $key);
+	$prev = $dbh->selectrow_array(meta_select, undef, $key);
 
-		if (defined $prev) {
-			$dbh->do(meta_update, undef, $value, $key);
-		} else {
-			$dbh->do(meta_insert, undef, $key, $value);
-		}
-		$dbh->commit;
-	};
-	my $err = $@;
-	return $prev unless $err;
-
-	$dbh->rollback;
-	die $err;
+	if (defined $prev) {
+		$dbh->do(meta_update, undef, $value, $key);
+	} else {
+		$dbh->do(meta_insert, undef, $key, $value);
+	}
+	$prev;
 }
 
 sub last_commit {
diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm
index c2bf9a2..3f2643c 100644
--- a/lib/PublicInbox/SearchIdx.pm
+++ b/lib/PublicInbox/SearchIdx.pm
@@ -369,18 +369,18 @@ sub _index_sync {
 			# Common case is the indexes are synced,
 			# we only need to run git-log once:
 			$lx = $self->rlog($range, *index_both, *unindex_both);
-			$mm->{dbh}->commit;
 			if (defined $lx) {
 				$db->set_metadata('last_commit', $lx);
 				$mm->last_commit($lx);
 			}
+			$mm->{dbh}->commit;
 		} else {
 			# dumb case, msgmap and xapian are out-of-sync
 			# do not care for performance:
 			my $r = $lm eq '' ? $head : "$lm..$head";
 			$lm = $self->rlog($r, *index_mm, *unindex_mm);
-			$mm->{dbh}->commit;
 			$mm->last_commit($lm) if defined $lm;
+			$mm->{dbh}->commit;
 
 			$lx = $self->rlog($range, *index_mm2, *unindex_mm2);
 			$db->set_metadata('last_commit', $lx) if defined $lx;
@@ -390,12 +390,7 @@ sub _index_sync {
 		$lx = $self->rlog($range, *index_blob, *unindex_blob);
 		$db->set_metadata('last_commit', $lx) if defined $lx;
 	}
-	if ($@) {
-		$db->cancel_transaction;
-		$mm->{dbh}->rollback if $mm;
-	} else {
-		$db->commit_transaction;
-	}
+	$db->commit_transaction;
 }
 
 # this will create a ghost as necessary
-- 
EW