Bigger changes are brewing in other branches... Eric Wong (5): spawn: make @RLIMITS a shared list spawn: modernize with parent.pm, drop warnings.pm watch: retry signals to kill IDLE and polling processes tests: add use/require statements for TEST_RUN_MODE=0 overidx: document why we don't use SQLite WAL lib/PublicInbox/InboxWritable.pm | 1 + lib/PublicInbox/OverIdx.pm | 2 ++ lib/PublicInbox/Qspawn.pm | 4 ++-- lib/PublicInbox/Spawn.pm | 7 +++---- lib/PublicInbox/WatchMaildir.pm | 27 ++++++++++++++++++--------- t/init.t | 1 + t/nntpd.t | 1 + 7 files changed, 28 insertions(+), 15 deletions(-)
Making the RLIMITS list a function doesn't allow constant folding, so just make it an array accessible to other modules. --- lib/PublicInbox/Qspawn.pm | 4 ++-- lib/PublicInbox/Spawn.pm | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/PublicInbox/Qspawn.pm b/lib/PublicInbox/Qspawn.pm index c09e8d2c227..d395a10b3b1 100644 --- a/lib/PublicInbox/Qspawn.pm +++ b/lib/PublicInbox/Qspawn.pm @@ -48,7 +48,7 @@ sub _do_spawn { my ($cmd, $cmd_env, $opt) = @{delete $self->{args}}; my %o = %{$opt || {}}; $self->{limiter} = $limiter; - foreach my $k (PublicInbox::Spawn::RLIMITS()) { + foreach my $k (@PublicInbox::Spawn::RLIMITS) { if (defined(my $rlimit = $limiter->{$k})) { $o{$k} = $rlimit; } @@ -358,7 +358,7 @@ sub new { sub setup_rlimit { my ($self, $name, $config) = @_; - foreach my $rlim (PublicInbox::Spawn::RLIMITS()) { + foreach my $rlim (@PublicInbox::Spawn::RLIMITS) { my $k = lc($rlim); $k =~ tr/_//d; $k = "publicinboxlimiter.$name.$k"; diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm index f90d8f6d3dd..ba6e73675fe 100644 --- a/lib/PublicInbox/Spawn.pm +++ b/lib/PublicInbox/Spawn.pm @@ -18,7 +18,7 @@ use base qw(Exporter); use Symbol qw(gensym); use PublicInbox::ProcessPipe; our @EXPORT_OK = qw/which spawn popen_rd/; -sub RLIMITS () { qw(RLIMIT_CPU RLIMIT_CORE RLIMIT_DATA) } +our @RLIMITS = qw(RLIMIT_CPU RLIMIT_CORE RLIMIT_DATA); my $vfork_spawn = <<'VFORK_SPAWN'; #include <sys/types.h> @@ -209,7 +209,7 @@ sub spawn ($;$$) { } my $rlim = []; - foreach my $l (RLIMITS()) { + foreach my $l (@RLIMITS) { defined(my $v = $opts->{$l}) or next; my $r = eval "require BSD::Resource; BSD::Resource::$l();"; unless (defined $r) {
parent.pm is smaller than base.pm, and we'll also move towards relying on `-w' (or not) to toggle process-wide warnings during development. --- lib/PublicInbox/Spawn.pm | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm index ba6e73675fe..c85b68003ac 100644 --- a/lib/PublicInbox/Spawn.pm +++ b/lib/PublicInbox/Spawn.pm @@ -13,8 +13,7 @@ package PublicInbox::Spawn; use strict; -use warnings; -use base qw(Exporter); +use parent qw(Exporter); use Symbol qw(gensym); use PublicInbox::ProcessPipe; our @EXPORT_OK = qw/which spawn popen_rd/;
To ensure reliable signal delivery in Perl, it seems we need to repeatedly signal processes which aren't using signalfd (or EVFILT_SIGNAL) with our event loop. --- lib/PublicInbox/WatchMaildir.pm | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/lib/PublicInbox/WatchMaildir.pm b/lib/PublicInbox/WatchMaildir.pm index e3df85159e7..23b2e9f110b 100644 --- a/lib/PublicInbox/WatchMaildir.pm +++ b/lib/PublicInbox/WatchMaildir.pm @@ -202,18 +202,27 @@ sub _try_path { } } +sub quit_done ($) { + my ($self) = @_; + return unless $self->{quit}; + + # don't have reliable wakeups, keep signalling + my $done = 1; + for (qw(idle_pids poll_pids)) { + my $pids = $self->{$_} or next; + for (keys %$pids) { + $done = undef if kill('QUIT', $_); + } + } + $done; +} + sub quit { my ($self) = @_; $self->{quit} = 1; %{$self->{opendirs}} = (); _done_for_now($self); - if (my $imap_pid = $self->{-imap_pid}) { - kill('QUIT', $imap_pid); - } - for (qw(idle_pids poll_pids)) { - my $pids = $self->{$_} or next; - kill('QUIT', $_) for (keys %$pids); - } + quit_done($self); if (my $idle_mic = $self->{idle_mic}) { eval { $idle_mic->done }; if ($@) { @@ -921,8 +930,8 @@ sub watch { [$self, $intvl, $urls]); } watch_fs_init($self) if $self->{mdre}; - PublicInbox::DS->SetPostLoopCallback(sub {}); - PublicInbox::DS->EventLoop until $self->{quit}; + PublicInbox::DS->SetPostLoopCallback(sub { !$self->quit_done }); + PublicInbox::DS->EventLoop; _done_for_now($self); }
The default (and fast) TEST_RUN_MODE=2 preloads most modules, but TEST_RUN_MODE=0 is more realistic and can catch some problems which may show up in real-world use. --- lib/PublicInbox/InboxWritable.pm | 1 + t/init.t | 1 + t/nntpd.t | 1 + 3 files changed, 3 insertions(+) diff --git a/lib/PublicInbox/InboxWritable.pm b/lib/PublicInbox/InboxWritable.pm index 9bdf8637e6e..875dcce20b7 100644 --- a/lib/PublicInbox/InboxWritable.pm +++ b/lib/PublicInbox/InboxWritable.pm @@ -44,6 +44,7 @@ sub init_inbox { PublicInbox::Import::init_bare($dir); if (defined($self->{indexlevel}) || defined($skip_artnum)) { require PublicInbox::SearchIdx; + require PublicInbox::Msgmap; my $sidx = PublicInbox::SearchIdx->new($self, 1); # just create $sidx->begin_txn_lazy; $self->with_umask(sub { diff --git a/t/init.t b/t/init.t index f4ebc2f67e6..5c021be729b 100644 --- a/t/init.t +++ b/t/init.t @@ -53,6 +53,7 @@ sub quiet_fail { SKIP: { require_mods(qw(DBD::SQLite Search::Xapian::WritableDatabase), 2); require_git(2.6, 1) or skip "git 2.6+ required", 2; + use_ok 'PublicInbox::Msgmap'; local $ENV{PI_DIR} = "$tmpdir/.public-inbox/"; my $cfgfile = "$ENV{PI_DIR}/config"; my $cmd = [ '-init', '-V2', 'v2list', "$tmpdir/v2list", diff --git a/t/nntpd.t b/t/nntpd.t index d72d6a1ce7e..28008ec15f7 100644 --- a/t/nntpd.t +++ b/t/nntpd.t @@ -396,6 +396,7 @@ sub test_watch { my ($tmpdir, $sock, $group) = @_; use_ok 'PublicInbox::WatchMaildir'; use_ok 'PublicInbox::InboxIdle'; + use_ok 'PublicInbox::Config'; require_git('1.8.5', 1) or skip('git 1.8.5+ needed for --urlmatch', 4); my $old_env = { HOME => $ENV{HOME} }; my $home = "$tmpdir/watch_home";
I was wondering about this myself the other day and had to read up on it. So make a note of it for future readers. --- lib/PublicInbox/OverIdx.pm | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/PublicInbox/OverIdx.pm b/lib/PublicInbox/OverIdx.pm index c7f45a6c910..008a5d1a936 100644 --- a/lib/PublicInbox/OverIdx.pm +++ b/lib/PublicInbox/OverIdx.pm @@ -23,6 +23,8 @@ sub dbh_new { my $dbh = $self->SUPER::dbh_new(1); # TRUNCATE reduces I/O compared to the default (DELETE) + # We do not use WAL since we're optimized for read-only ops, + # (and read-only requires SQLite 3.22.0 (2018-01-22)). $dbh->do('PRAGMA journal_mode = TRUNCATE'); # 80000 pages (80MiB on SQLite <3.12.0, 320MiB on 3.12.0+)
We no longer use writev(2) in pi_fork_exec to emit errors. --- lib/PublicInbox/Spawn.pm | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm index c85b68003ac..db679b770cf 100644 --- a/lib/PublicInbox/Spawn.pm +++ b/lib/PublicInbox/Spawn.pm @@ -21,7 +21,6 @@ our @RLIMITS = qw(RLIMIT_CPU RLIMIT_CORE RLIMIT_DATA); my $vfork_spawn = <<'VFORK_SPAWN'; #include <sys/types.h> -#include <sys/uio.h> #include <sys/time.h> #include <sys/resource.h> #include <unistd.h>