* [PATCH] lock: reduce inotify wakeups
@ 2020-06-23 7:30 Eric Wong
2020-06-24 18:45 ` [PATCH v2] " Eric Wong
0 siblings, 1 reply; 2+ messages in thread
From: Eric Wong @ 2020-06-23 7:30 UTC (permalink / raw)
To: meta
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).
---
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 | 3 ++-
lib/PublicInbox/Xapcmd.pm | 1 +
7 files changed, 18 insertions(+), 24 deletions(-)
diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index af35905b..ae508cd8 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 d0bb43c5..97e9d532 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 1b5c578e..110594cc 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 693a3794..c0d4d3b3 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 00e63938..4caa66d3 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 a0f041dd..567f203c 100644
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -670,6 +670,7 @@ sub barrier { checkpoint($_[0], 1) };
# public
sub done {
my ($self) = @_;
+ my $nbytes = $self->{transact_bytes};
my $im = delete $self->{im};
$im->done if $im; # PublicInbox::Import::done
checkpoint($self);
@@ -682,7 +683,7 @@ sub done {
$self->{over}->disconnect;
delete $self->{bnote};
$self->{transact_bytes} = 0;
- $self->lock_release if $shards;
+ $self->lock_release(!!$nbytes) if $shards;
$self->{-inbox}->git->cleanup;
}
diff --git a/lib/PublicInbox/Xapcmd.pm b/lib/PublicInbox/Xapcmd.pm
index 337978bd..78414041 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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [PATCH v2] lock: reduce inotify wakeups
2020-06-23 7:30 [PATCH] lock: reduce inotify wakeups Eric Wong
@ 2020-06-24 18:45 ` Eric Wong
0 siblings, 0 replies; 2+ messages in thread
From: Eric Wong @ 2020-06-24 18:45 UTC (permalink / raw)
To: meta
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-06-24 18:45 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-23 7:30 [PATCH] lock: reduce inotify wakeups Eric Wong
2020-06-24 18:45 ` [PATCH v2] " Eric Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).