From 39b99c2514230f419fae8c2b52a7d55eaad1cd44 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Fri, 7 Apr 2023 12:40:51 +0000 Subject: umask: rely on the OnDestroy-based call where applicable This lets us get rid of some awkwardness around the old API and single-use subroutines while saving us some LoC. --- lib/PublicInbox/ExtSearchIdx.pm | 14 +++++--------- lib/PublicInbox/SearchIdx.pm | 19 ++++++------------- lib/PublicInbox/V2Writable.pm | 23 +++++++++-------------- lib/PublicInbox/Xapcmd.pm | 35 ++++++++++++++++------------------- 4 files changed, 36 insertions(+), 55 deletions(-) (limited to 'lib/PublicInbox') diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm index 6f3711ba..6856ae66 100644 --- a/lib/PublicInbox/ExtSearchIdx.pm +++ b/lib/PublicInbox/ExtSearchIdx.pm @@ -1179,12 +1179,6 @@ sub update_last_commit { # overrides V2Writable $self->{oidx}->eidx_meta($meta_key, $latest_cmt); } -sub _idx_init { # with_umask callback - my ($self, $opt) = @_; - PublicInbox::V2Writable::_idx_init($self, $opt); # acquires ei.lock - $self->{midx} = PublicInbox::MiscIdx->new($self); -} - sub symlink_packs ($$) { my ($ibx, $pd) = @_; my $ret = 0; @@ -1270,15 +1264,17 @@ sub idx_init { # similar to V2Writable } ($has_new || $prune_nr || $new ne '') and $self->{mg}->write_alternates($mode, $alt, $new); - $git_midx and $self->with_umask(sub { + my $restore = $self->with_umask; + if ($git_midx) { my @cmd = ('multi-pack-index'); push @cmd, '--no-progress' if ($opt->{quiet}//0) > 1; my $lk = $self->lock_for_scope; system('git', "--git-dir=$ALL", @cmd, 'write'); # ignore errors, fairly new command, may not exist - }); + } $self->parallel_init($self->{indexlevel}); - $self->with_umask(\&_idx_init, $self, $opt); + PublicInbox::V2Writable::_idx_init($self, $opt); # acquires ei.lock + $self->{midx} = PublicInbox::MiscIdx->new($self); $self->{oidx}->begin_lazy; $self->{oidx}->eidx_prep; $self->{midx}->create_xdb if $new ne ''; diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm index 69ab30e6..511dd4d6 100644 --- a/lib/PublicInbox/SearchIdx.pm +++ b/lib/PublicInbox/SearchIdx.pm @@ -1110,8 +1110,10 @@ sub DESTROY { $_[0]->{lockfh} = undef; } -sub _begin_txn { +sub begin_txn_lazy { my ($self) = @_; + return if $self->{txn}; + my $restore = $self->with_umask; my $xdb = $self->{xdb} || idx_acquire($self); $self->{oidx}->begin_lazy if $self->{oidx}; $xdb->begin_transaction if $xdb; @@ -1119,11 +1121,6 @@ sub _begin_txn { $xdb; } -sub begin_txn_lazy { - my ($self) = @_; - $self->with_umask(\&_begin_txn, $self) if !$self->{txn}; -} - # store 'indexlevel=medium' in v2 shard=0 and v1 (only one shard) # This metadata is read by InboxWritable->detect_indexlevel: sub set_metadata_once { @@ -1147,8 +1144,10 @@ sub set_metadata_once { } } -sub _commit_txn { +sub commit_txn_lazy { my ($self) = @_; + return unless delete($self->{txn}); + my $restore = $self->with_umask; if (my $eidx = $self->{eidx}) { $eidx->git->async_wait_all; $eidx->{transact_bytes} = 0; @@ -1160,12 +1159,6 @@ sub _commit_txn { $self->{oidx}->commit_lazy if $self->{oidx}; } -sub commit_txn_lazy { - my ($self) = @_; - delete($self->{txn}) and - $self->with_umask(\&_commit_txn, $self); -} - sub eidx_shard_new { my ($class, $eidx, $shard) = @_; my $self = bless { diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm index d3d13941..856442a9 100644 --- a/lib/PublicInbox/V2Writable.pm +++ b/lib/PublicInbox/V2Writable.pm @@ -89,13 +89,6 @@ sub init_inbox { $self->done; } -# returns undef on duplicate or spam -# mimics Import::add and wraps it for v2 -sub add { - my ($self, $eml, $check_cb) = @_; - $self->{ibx}->with_umask(\&_add, $self, $eml, $check_cb); -} - sub idx_shard ($$) { my ($self, $num) = @_; $self->{idx_shards}->[$num % scalar(@{$self->{idx_shards}})]; @@ -113,8 +106,11 @@ sub do_idx ($$$) { $n >= $self->{batch_bytes}; } -sub _add { +# returns undef on duplicate or spam +# mimics Import::add and wraps it for v2 +sub add { my ($self, $mime, $check_cb) = @_; + my $restore = $self->{ibx}->with_umask; # spam check: if ($check_cb) { @@ -391,17 +387,16 @@ sub rewrite_internal ($$;$$$) { # (retval[2]) is not part of the stable API shared with Import->remove sub remove { my ($self, $eml, $cmt_msg) = @_; - my $r = $self->{ibx}->with_umask(\&rewrite_internal, - $self, $eml, $cmt_msg); + my $restore = $self->{ibx}->with_umask; + my $r = rewrite_internal($self, $eml, $cmt_msg); defined($r) && defined($r->[0]) ? @$r: undef; } sub _replace ($$;$$) { my ($self, $old_eml, $new_eml, $sref) = @_; - my $arg = [ $self, $old_eml, undef, $new_eml, $sref ]; - my $rewritten = $self->{ibx}->with_umask(\&rewrite_internal, - $self, $old_eml, undef, $new_eml, $sref) or return; - + my $restore = $self->{ibx}->with_umask; + my $rewritten = rewrite_internal($self, $old_eml, undef, + $new_eml, $sref) or return; my $rewrites = $rewritten->{rewrites}; # ->done is called if there are rewrites since we gc+prune from git $self->idx_init if @$rewrites; diff --git a/lib/PublicInbox/Xapcmd.pm b/lib/PublicInbox/Xapcmd.pm index 10685636..c87baa7b 100644 --- a/lib/PublicInbox/Xapcmd.pm +++ b/lib/PublicInbox/Xapcmd.pm @@ -256,24 +256,6 @@ sub prepare_run { sub check_compact () { runnable_or_die($XAPIAN_COMPACT) } -sub _run { # with_umask callback - my ($ibx, $cb, $opt) = @_; - my $im = $ibx->can('importer') ? $ibx->importer(0) : undef; - ($im // $ibx)->lock_acquire; - my ($tmp, $queue) = prepare_run($ibx, $opt); - - # fine-grained locking if we prepare for reindex - if (!$opt->{-coarse_lock}) { - prepare_reindex($ibx, $opt); - ($im // $ibx)->lock_release; - } - - $ibx->cleanup if $ibx->can('cleanup'); - process_queue($queue, $cb, $opt); - ($im // $ibx)->lock_acquire if !$opt->{-coarse_lock}; - commit_changes($ibx, $im, $tmp, $opt); -} - sub run { my ($ibx, $task, $opt) = @_; # task = 'cpdb' or 'compact' my $cb = \&$task; @@ -296,7 +278,22 @@ sub run { local @SIG{keys %SIG} = values %SIG; setup_signals(); - $ibx->with_umask(\&_run, $ibx, $cb, $opt); + my $restore = $ibx->with_umask; + + my $im = $ibx->can('importer') ? $ibx->importer(0) : undef; + ($im // $ibx)->lock_acquire; + my ($tmp, $queue) = prepare_run($ibx, $opt); + + # fine-grained locking if we prepare for reindex + if (!$opt->{-coarse_lock}) { + prepare_reindex($ibx, $opt); + ($im // $ibx)->lock_release; + } + + $ibx->cleanup if $ibx->can('cleanup'); + process_queue($queue, $cb, $opt); + ($im // $ibx)->lock_acquire if !$opt->{-coarse_lock}; + commit_changes($ibx, $im, $tmp, $opt); } sub cpdb_retryable ($$) { -- cgit v1.2.3-24-ge0c7