msgmap: fix use of transactions

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.

2 files changed, 12 insertions, 23 deletions

diff --git a/lib/PublicInbox/Msgmap.pm b/lib/PublicInbox/Msgmap.pm
index 8fe17a95..2583ff47 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 c2bf9a2d..3f2643c6 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