[PATCH 0/2] avoid SCM_RIGHTS dep w/o Inline::C||Socket::MsgHdr

[PATCH 0/2] avoid SCM_RIGHTS dep w/o Inline::C||Socket::MsgHdr

[Eric Wong]

Either Inline::C or Socket::MsgHdr is required for SCM_RIGHTS
support (FD passing) on *BSDs where syscall numbers aren't stable.

1/2 is actually an all-around improvement, while 2/2 is a
temporary placeholder while we consider FIFOs or something
else...

Eric Wong (2):
  cindex: make prune work without SCM_RIGHTS support
  t/xap_helper: skip test if missing SCM_RIGHTS support

 lib/PublicInbox/CodeSearchIdx.pm | 60 +++++++++++++++-----------------
 lib/PublicInbox/TestCommon.pm    | 23 ++++++------
 t/xap_helper.t                   |  2 ++
 3 files changed, 44 insertions(+), 41 deletions(-)

[PATCH 1/2] cindex: make prune work without SCM_RIGHTS support

[Eric Wong]

We don't have to create extra pipes to pass around, actually.
Sending records via send/sendmsg isn't noticeably more expensive
than writing to a pipe.
---
 lib/PublicInbox/CodeSearchIdx.pm | 60 +++++++++++++++-----------------
 1 file changed, 29 insertions(+), 31 deletions(-)

diff --git a/lib/PublicInbox/CodeSearchIdx.pm b/lib/PublicInbox/CodeSearchIdx.pm
index 2700744d..f9251be5 100644
--- a/lib/PublicInbox/CodeSearchIdx.pm
+++ b/lib/PublicInbox/CodeSearchIdx.pm
@@ -758,28 +758,31 @@ sub scan_git_dirs ($) {
 	progress($self, "scanning $n code repositories...");
 }
 
-sub prune_do { # via wq_io_do in IDX_SHARDS
+sub prune_init { # via wq_io_do in IDX_SHARDS
 	my ($self) = @_;
-	my $gone = delete $self->{0} // die 'BUG: no {0} gone input';
-	my $prune_op_p = delete $self->{1} // die 'BUG: no {1} op_p';
+	$self->{nr_prune} = 0;
 	$TXN_BYTES = $BATCH_BYTES;
 	$self->begin_txn_lazy;
-	my $xdb = $self->{xdb};
-	my $nr = 0;
-	local $/ = "\0";
-	while (my $p = <$gone>) { # Q$cmt or P$git_dir
-		chomp $p;
-		my @docids = docids_by_postlist($self, $p);
-		for (@docids) {
-			$TXN_BYTES -= $xdb->get_doclength($_) * 42;
-			$xdb->delete_document($_);
-		}
-		++$nr;
-		$TXN_BYTES < 0 and
-			cidx_ckpoint($self, "prune [$self->{shard}] $nr");
+}
+
+sub prune_one { # via wq_io_do in IDX_SHARDS
+	my ($self, $term) = @_;
+	my @docids = docids_by_postlist($self, $term);
+	for (@docids) {
+		$TXN_BYTES -= $self->{xdb}->get_doclength($_) * 42;
+		$self->{xdb}->delete_document($_);
 	}
-	send($prune_op_p, "prune_done $self->{shard}", MSG_EOR);
+	++$self->{nr_prune};
+	$TXN_BYTES < 0 and
+		cidx_ckpoint($self, "prune [$self->{shard}] $self->{nr_prune}");
+}
+
+sub prune_commit { # via wq_io_do in IDX_SHARDS
+	my ($self) = @_;
+	my $prune_op_p = delete $self->{0} // die 'BUG: no {0} op_p';
+	my $nr = delete $self->{nr_prune} // die 'BUG: nr_prune undef';
 	cidx_ckpoint($self, "prune [$self->{shard}] $nr done") if $nr;
+	send($prune_op_p, "prune_done $self->{shard}", MSG_EOR);
 }
 
 sub shards_active { # post_loop_do
@@ -1025,25 +1028,20 @@ EOM
 sub cidx_read_comm { # via PublicInbox::CidxComm::event_step
 	my ($self, $comm_rd) = @_;
 	return if $DO_QUIT;
-	my ($c, $p) = PublicInbox::PktOp->pair;
-	$c->{ops}->{prune_done} = [ $self ];
-	my @gone;
-	for my $n (0..$#IDX_SHARDS) {
-		pipe(my ($r, $w)) or die "pipe: $!";
-		push @gone, $w;
-		$IDX_SHARDS[$n]->wq_io_do('prune_do', [$r, $p->{op_p}]);
-	}
+	$_->wq_do('prune_init') for @IDX_SHARDS;
 	while (defined(my $cmt = <$comm_rd>)) {
-		chomp $cmt;
-		my $n = hex(substr($cmt, 0, 8)) % scalar(@gone);
-		print { $gone[$n] } 'Q', $cmt, "\0" or die "print: $!";
+		chop($cmt) eq "\n" or die "BUG: no LF in comm output ($cmt)";
+		my $n = hex(substr($cmt, 0, 8)) % scalar(@IDX_SHARDS);
+		$IDX_SHARDS[$n]->wq_do('prune_one', 'Q'.$cmt);
 		last if $DO_QUIT;
 	}
 	for my $git_dir (@GIT_DIR_GONE) {
-		my $n = git_dir_hash($git_dir) % scalar(@gone);
-		print { $gone[$n] } 'P', $git_dir, "\0" or die "print: $!";
+		my $n = git_dir_hash($git_dir) % scalar(@IDX_SHARDS);
+		$IDX_SHARDS[$n]->wq_do('prune_one', 'P'.$git_dir);
 	}
-	for (@gone) { close $_ or die "close: $!" };
+	my ($c, $p) = PublicInbox::PktOp->pair;
+	$c->{ops}->{prune_done} = [ $self ];
+	$_->wq_io_do('prune_commit', [ $p->{op_p} ]) for @IDX_SHARDS;
 }
 
 sub init_associate_prefork ($) {

[PATCH 2/2] t/xap_helper: skip test if missing SCM_RIGHTS support

[Eric Wong]

xap_helper currently relies on FDs passed via SCM_RIGHTS for
robustness against $TMPDIR failures and over-eager FS cleanup
tasks.  This depends on stable syscall numbers (Linux) or
Inline::C||Socket::MsgHdr being available, though, as Perl5
itself doesn't support SCM_RIGHTS.

We could probably add FIFO support to xap_helper for portability
to systems where neither Inline::C nor Socket::MsgHdr are available,
but that's for another day.
---
 lib/PublicInbox/TestCommon.pm | 23 +++++++++++++----------
 t/xap_helper.t                |  2 ++
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index ddee58b1..6da993af 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -22,7 +22,7 @@ BEGIN {
 	@EXPORT = qw(tmpdir tcp_server tcp_connect require_git require_mods
 		run_script start_script key2sub xsys xsys_e xqx eml_load tick
 		have_xapian_compact json_utf8 setup_public_inboxes create_inbox
-		create_coderepo
+		create_coderepo no_scm_rights
 		tcp_host_port test_lei lei lei_ok $lei_out $lei_err $lei_opt
 		test_httpd xbail require_cmd is_xdeeply tail_f
 		ignore_inline_c_missing);
@@ -567,6 +567,16 @@ sub ignore_inline_c_missing {
 		grep(!/\bInline\b/, split(/^/m, $_[0])))));
 }
 
+sub no_scm_rights () {
+	state $ok = PublicInbox::Spawn->can('send_cmd4') || do {
+			require PublicInbox::Syscall;
+			PublicInbox::Syscall->can('send_cmd4');
+		} || eval { require Socket::MsgHdr; 1 };
+	return if $ok;
+	'Inline::C unconfigured/missing '.
+	'(mkdir -p ~/.cache/public-inbox/inline-c) OR Socket::MsgHdr missing';
+}
+
 sub test_lei {
 SKIP: {
 	my ($cb) = pop @_;
@@ -591,15 +601,8 @@ SKIP: {
 	$ENV{LANG} = $ENV{LC_ALL} = 'C';
 	my (undef, $fn, $lineno) = caller(0);
 	my $t = "$fn:$lineno";
-	state $lei_daemon = PublicInbox::Spawn->can('send_cmd4') || do {
-			require PublicInbox::Syscall;
-			PublicInbox::Syscall->can('send_cmd4');
-		} || eval { require Socket::MsgHdr; 1 };
-	unless ($lei_daemon) {
-		skip('Inline::C unconfigured/missing '.
-'(mkdir -p ~/.cache/public-inbox/inline-c) OR Socket::MsgHdr missing',
-			1);
-	}
+	state $msg = no_scm_rights();
+	skip $msg, 1 if $msg;
 	$lei_opt = { 1 => \$lei_out, 2 => \$lei_err };
 	my ($daemon_pid, $for_destroy, $daemon_xrd);
 	my $tmpdir = $test_opt->{tmpdir};
diff --git a/t/xap_helper.t b/t/xap_helper.t
index 92da2e6d..3646cf97 100644
--- a/t/xap_helper.t
+++ b/t/xap_helper.t
@@ -4,6 +4,8 @@
 use v5.12;
 use PublicInbox::TestCommon;
 require_mods(qw(DBD::SQLite Search::Xapian));
+my $msg = no_scm_rights;
+plan(skip_all => $msg) if $msg; # TODO: FIFO support?
 use PublicInbox::Spawn qw(spawn);
 use Socket qw(AF_UNIX SOCK_SEQPACKET SOCK_STREAM MSG_EOR);
 require PublicInbox::AutoReap;