[PATCH 0/5] some random minor fixes

[PATCH 0/5] some random minor fixes

[Eric Wong]

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(-)

[PATCH 1/5] spawn: make @RLIMITS an array

[Eric Wong]

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) {

[PATCH 2/5] spawn: modernize with parent.pm, drop warnings.pm

[Eric Wong]

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/;

[PATCH 3/5] watch: retry signals to kill IDLE and polling processes

[Eric Wong]

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);
 }
 

[PATCH 4/5] tests: add use/require statements for TEST_RUN_MODE=0

[Eric Wong]

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";

[PATCH 5/5] overidx: document why we don't use SQLite WAL

[Eric Wong]

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+)

[PATCH 6/5] spawn: drop unused sys/uio.h include

[Eric Wong]

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>