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 AA3EE1F5AF; Wed, 24 Jun 2020 18:45:07 +0000 (UTC) Date: Wed, 24 Jun 2020 18:45:07 +0000 From: Eric Wong To: meta@public-inbox.org Subject: [PATCH v2] lock: reduce inotify wakeups Message-ID: <20200624184507.GA2572@dcvr> References: <20200623073010.94732-1-e@yhbt.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200623073010.94732-1-e@yhbt.net> List-Id: We can reduce the amount of platform-specific code by always relying on IN_MODIFY/NOTE_WRITE notifications from lock release. This reduces the number of times our read-only daemons will need to wake up when -watch sees no-op message changes (e.g. replied, seen, recent flag changes). --- v2: - v2writable: track total_bytes. This is needed in case a ->checkpoint happens immediately before ->done, with no ->add calls in between them. lib/PublicInbox/Import.pm | 6 +++--- lib/PublicInbox/InboxIdle.pm | 8 ++++---- lib/PublicInbox/KQNotify.pm | 8 ++------ lib/PublicInbox/Lock.pm | 10 +++------- lib/PublicInbox/SearchIdx.pm | 6 +++--- lib/PublicInbox/V2Writable.pm | 7 +++++-- lib/PublicInbox/Xapcmd.pm | 1 + 7 files changed, 21 insertions(+), 25 deletions(-) diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm index af35905be49..ae508cd8013 100644 --- a/lib/PublicInbox/Import.pm +++ b/lib/PublicInbox/Import.pm @@ -468,9 +468,9 @@ sub done { waitpid($pid, 0) == $pid or die 'fast-import did not finish'; $? == 0 or die "fast-import failed: $?"; - _update_git_info($self, 1) if delete $self->{nchg}; - - $self->lock_release; + my $nchg = delete $self->{nchg}; + _update_git_info($self, 1) if $nchg; + $self->lock_release(!!$nchg); $self->{git}->cleanup; } diff --git a/lib/PublicInbox/InboxIdle.pm b/lib/PublicInbox/InboxIdle.pm index d0bb43c5848..97e9d53250e 100644 --- a/lib/PublicInbox/InboxIdle.pm +++ b/lib/PublicInbox/InboxIdle.pm @@ -8,13 +8,13 @@ use fields qw(pi_config inot pathmap); use Cwd qw(abs_path); use Symbol qw(gensym); use PublicInbox::Syscall qw(EPOLLIN EPOLLET); -my $IN_CLOSE = 0x08 | 0x10; # match Linux inotify +my $IN_MODIFY = 0x02; # match Linux inotify my $ino_cls; if ($^O eq 'linux' && eval { require Linux::Inotify2; 1 }) { - $IN_CLOSE = Linux::Inotify2::IN_CLOSE(); + $IN_MODIFY = Linux::Inotify2::IN_MODIFY(); $ino_cls = 'Linux::Inotify2'; } elsif (eval { require PublicInbox::KQNotify }) { - $IN_CLOSE = PublicInbox::KQNotify::IN_CLOSE(); + $IN_MODIFY = PublicInbox::KQNotify::NOTE_WRITE(); $ino_cls = 'PublicInbox::KQNotify'; } require PublicInbox::In2Tie if $ino_cls; @@ -39,7 +39,7 @@ sub in2_arm ($$) { # PublicInbox::Config::each_inbox callback $cur->[0] = $ibx; my $lock = "$dir/".($ibx->version >= 2 ? 'inbox.lock' : 'ssoma.lock'); - $cur->[1] = $inot->watch($lock, $IN_CLOSE, sub { $ibx->on_unlock }); + $cur->[1] = $inot->watch($lock, $IN_MODIFY, sub { $ibx->on_unlock }); # TODO: detect deleted packs (and possibly other files) } diff --git a/lib/PublicInbox/KQNotify.pm b/lib/PublicInbox/KQNotify.pm index 1b5c578ef3d..110594cc02c 100644 --- a/lib/PublicInbox/KQNotify.pm +++ b/lib/PublicInbox/KQNotify.pm @@ -8,10 +8,6 @@ use strict; use IO::KQueue; use PublicInbox::DSKQXS; # wraps IO::KQueue for fork-safe DESTROY -# only true as far as public-inbox is concerned with .lock files: -sub IN_CLOSE () { NOTE_WRITE } -#sub IN_CLOSE () { 0x200 } # NOTE_CLOSE_WRITE (FreeBSD 11+ only) - sub new { my ($class) = @_; bless { dskq => PublicInbox::DSKQXS->new, watch => {} }, $class; @@ -26,7 +22,7 @@ sub watch { EV_ADD | EV_CLEAR, # flags $mask, # fflags 0, 0); # data, udata - if ($mask == IN_CLOSE) { + if ($mask == NOTE_WRITE) { $self->{watch}->{$ident} = [ $fh, $cb ]; } else { die "TODO Not implemented: $mask"; @@ -52,7 +48,7 @@ sub poll { for my $kev (@kevents) { my $ident = $kev->[KQ_IDENT]; my $mask = $kev->[KQ_FFLAGS]; - if (($mask & IN_CLOSE) == IN_CLOSE) { + if (($mask & NOTE_WRITE) == NOTE_WRITE) { eval { $self->{watch}->{$ident}->[1]->() }; } } diff --git a/lib/PublicInbox/Lock.pm b/lib/PublicInbox/Lock.pm index 693a37949a6..c0d4d3b35c4 100644 --- a/lib/PublicInbox/Lock.pm +++ b/lib/PublicInbox/Lock.pm @@ -21,16 +21,12 @@ sub lock_acquire { } sub lock_release { - my ($self) = @_; + my ($self, $wake) = @_; return unless $self->{lock_path}; my $lockfh = delete $self->{lockfh} or croak 'not locked'; - # NetBSD 8.1 and OpenBSD 6.5 (and maybe other versions/*BSDs) lack - # NOTE_CLOSE_WRITE from FreeBSD 11+, so trigger NOTE_WRITE, instead. - # We also need to change the ctime on Linux systems w/o inotify - if ($^O ne 'linux' || !eval { require Linux::Inotify2; 1 }) { - syswrite($lockfh, '.'); - } + syswrite($lockfh, '.') if $wake; + flock($lockfh, LOCK_UN) or die "unlock failed: $!\n"; close $lockfh or die "close failed: $!\n"; } diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm index 00e63938fc0..4caa66d3751 100644 --- a/lib/PublicInbox/SearchIdx.pm +++ b/lib/PublicInbox/SearchIdx.pm @@ -78,12 +78,12 @@ sub new { sub need_xapian ($) { $_[0]->{indexlevel} =~ $xapianlevels } sub _xdb_release { - my ($self) = @_; + my ($self, $wake) = @_; if (need_xapian($self)) { my $xdb = delete $self->{xdb} or croak 'not acquired'; $xdb->close; } - $self->lock_release if $self->{creat}; + $self->lock_release($wake) if $self->{creat}; undef; } @@ -800,7 +800,7 @@ sub _index_sync { } $self->commit_txn_lazy; $git->cleanup; - $xdb = _xdb_release($self); + $xdb = _xdb_release($self, $nr); # let another process do some work... < $pr->("indexed $nr/$self->{ntodo}\n") if $pr && $nr; if (!$newest) { diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm index a0f041dd3dc..8b31b69a62f 100644 --- a/lib/PublicInbox/V2Writable.pm +++ b/lib/PublicInbox/V2Writable.pm @@ -113,6 +113,7 @@ sub new { im => undef, # PublicInbox::Import parallel => 1, transact_bytes => 0, + total_bytes => 0, current_info => '', xpfx => $xpfx, over => PublicInbox::OverIdx->new("$xpfx/over.sqlite3", 1), @@ -659,6 +660,7 @@ sub checkpoint ($;$) { $dbh->begin_work; } + $self->{total_bytes} += $self->{transact_bytes}; $self->{transact_bytes} = 0; } @@ -681,8 +683,9 @@ sub done { } $self->{over}->disconnect; delete $self->{bnote}; - $self->{transact_bytes} = 0; - $self->lock_release if $shards; + my $nbytes = $self->{total_bytes}; + $self->{total_bytes} = 0; + $self->lock_release(!!$nbytes) if $shards; $self->{-inbox}->git->cleanup; } diff --git a/lib/PublicInbox/Xapcmd.pm b/lib/PublicInbox/Xapcmd.pm index 337978bd1a8..784140416e7 100644 --- a/lib/PublicInbox/Xapcmd.pm +++ b/lib/PublicInbox/Xapcmd.pm @@ -39,6 +39,7 @@ sub commit_changes ($$$$) { my $tmp_over = "$new/over.sqlite3"; $over->connect->sqlite_backup_to_file($tmp_over); $over = undef; + syswrite($im->{lockfh}, '.'); # trigger ->check_inodes } if (!defined($new)) { # culled shard