From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 389411FA12 for ; Mon, 25 Jan 2021 06:41:59 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 2/4] miscidx: switch to lazy transactions Date: Sun, 24 Jan 2021 22:41:56 -0800 Message-Id: <20210125064158.15179-3-e@80x24.org> In-Reply-To: <20210125064158.15179-1-e@80x24.org> References: <20210125064158.15179-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: This fixes a sporadic failure on a 1/2 core VM where "git cat-file --batch" hasn't started up by the time $cleanup->() destroys the ALL.git directory in t/lei.t (but not t/lei-oneshot.t). This happens because dwaitpid() runs inside the event loop asynchronously and we were able to return to the client before the cat-file process could even start. I could not reproduce this failure on my usual 4-core workstation via "schedtool -a 0x1" to force the entire test to use a single core. Lazy transactions matches OverIdx and SearchIdx behavior, and I've verified this lets us avoid problems with old Xapian versions (on CentOS 7.x) which failed to set FD_CLOEXEC. --- lib/PublicInbox/ExtSearchIdx.pm | 3 +-- lib/PublicInbox/MiscIdx.pm | 19 ++++++++++++------- lib/PublicInbox/V2Writable.pm | 4 ---- t/miscsearch.t | 1 - 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm index c782a62a..9b7340df 100644 --- a/lib/PublicInbox/ExtSearchIdx.pm +++ b/lib/PublicInbox/ExtSearchIdx.pm @@ -1003,8 +1003,7 @@ sub idx_init { # similar to V2Writable $self->with_umask(\&_idx_init, $self, $opt); $self->{oidx}->begin_lazy; $self->{oidx}->eidx_prep; - $self->git->batch_prepare; - $self->{midx}->begin_txn; + $self->{midx}->create_xdb if @new; } sub _watch_commit { # PublicInbox::DS::add_timer callback diff --git a/lib/PublicInbox/MiscIdx.pm b/lib/PublicInbox/MiscIdx.pm index ab5e029a..f5a374b2 100644 --- a/lib/PublicInbox/MiscIdx.pm +++ b/lib/PublicInbox/MiscIdx.pm @@ -39,25 +39,30 @@ sub new { }, $class; } -sub begin_txn { +sub _begin_txn ($) { my ($self) = @_; - croak 'BUG: already in txn' if $self->{xdb}; # XXX make lazy? my $wdb = $PublicInbox::Search::X{WritableDatabase}; my $xdb = eval { $wdb->new($self->{mi_dir}, $self->{flags}) }; croak "Failed opening $self->{mi_dir}: $@" if $@; - $self->{xdb} = $xdb; $xdb->begin_transaction; + $xdb; } sub commit_txn { my ($self) = @_; - croak 'BUG: not in txn' unless $self->{xdb}; # XXX make lazy? - delete($self->{xdb})->commit_transaction; + my $xdb = delete $self->{xdb} or return; + $xdb->commit_transaction; +} + +sub create_xdb { + my ($self) = @_; + $self->{xdb} //= _begin_txn($self); + commit_txn($self); } sub remove_eidx_key { my ($self, $eidx_key) = @_; - my $xdb = $self->{xdb}; + my $xdb = $self->{xdb} //= _begin_txn($self); my $head = $xdb->postlist_begin('Q'.$eidx_key); my $tail = $xdb->postlist_end('Q'.$eidx_key); my @docids; # only one, unless we had bugs @@ -74,7 +79,7 @@ sub remove_eidx_key { sub index_ibx { my ($self, $ibx) = @_; my $eidx_key = $ibx->eidx_key; - my $xdb = $self->{xdb}; + my $xdb = $self->{xdb} //= _begin_txn($self); # Q = uniQue in Xapian terminology my $head = $xdb->postlist_begin('Q'.$eidx_key); my $tail = $xdb->postlist_end('Q'.$eidx_key); diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm index 0104f87a..7f9342b0 100644 --- a/lib/PublicInbox/V2Writable.pm +++ b/lib/PublicInbox/V2Writable.pm @@ -623,10 +623,6 @@ shard[$i] bad echo:$echo != $i waiting for txn commit $dbh->commit; $dbh->begin_work; } - if ($midx) { - $self->git->batch_prepare; - $midx->begin_txn; - } } $self->{total_bytes} += $self->{transact_bytes}; $self->{transact_bytes} = 0; diff --git a/t/miscsearch.t b/t/miscsearch.t index 0424328d..413579fb 100644 --- a/t/miscsearch.t +++ b/t/miscsearch.t @@ -27,7 +27,6 @@ my $eidx = { xpfx => "$tmp/eidx", -no_fsync => 1 }; # mock ExtSearchIdx }); $v1->init_inbox; my $mi = PublicInbox::MiscIdx->new($eidx); - $mi->begin_txn; $mi->index_ibx($v1); $mi->commit_txn; }