* [PATCH 1/9] lei q: start ->mset while query_prepare runs
2021-01-19 9:34 [PATCH 0/9] lei bugfixes and error handling Eric Wong
@ 2021-01-19 9:34 ` Eric Wong
2021-01-19 9:34 ` [PATCH 2/9] lei q: fix SIGPIPE handling from lei2mail workers Eric Wong
` (15 subsequent siblings)
16 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-01-19 9:34 UTC (permalink / raw)
To: meta
We don't need the result of query_prepare (for augmenting or
mass unlinking) until we're ready to deduplicate and write
results to the filesystem. This ought to let us hide some of
the cost of Xapian searches on multi-device/core systems for
extremely expensive searches.
---
lib/PublicInbox/LEI.pm | 2 +-
lib/PublicInbox/LeiToMail.pm | 3 +-
lib/PublicInbox/LeiXSearch.pm | 54 ++++++++++++++++++++---------------
lib/PublicInbox/Spawn.pm | 2 +-
4 files changed, 35 insertions(+), 26 deletions(-)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 6b6ee0f5..4b1dc673 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -293,7 +293,7 @@ sub atfork_child_wq {
my ($sock, $l2m_wq_s1);
(@$self{qw(0 1 2)}, $sock, $l2m_wq_s1) = delete(@$wq{0..4});
$self->{sock} = $sock if -S $sock;
- $self->{l2m}->{-wq_s1} = $l2m_wq_s1 if $l2m_wq_s1;
+ $self->{l2m}->{-wq_s1} = $l2m_wq_s1 if $l2m_wq_s1 && -S $l2m_wq_s1;
%PATH2CFG = ();
$quit = \&CORE::exit;
@TO_CLOSE_ATFORK_CHILD = ();
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index dcf6d8a3..a1dce550 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -440,6 +440,7 @@ sub lock_free {
sub write_mail { # via ->wq_do
my ($self, $git_dir, $oid, $lei, $kw) = @_;
+ my $not_done = delete $self->{4}; # write end of {each_smsg_done}
my $wcb = $self->{wcb} //= do { # first message
my %sig = $lei->atfork_child_wq($self);
@SIG{keys %sig} = values %sig; # not local
@@ -447,7 +448,7 @@ sub write_mail { # via ->wq_do
$self->write_cb($lei);
};
my $git = $self->{"$$\0$git_dir"} //= PublicInbox::Git->new($git_dir);
- $git->cat_async($oid, \&git_to_mail, [ $wcb, $kw ]);
+ $git->cat_async($oid, \&git_to_mail, [ $wcb, $kw, $not_done ]);
}
sub ipc_atfork_prepare {
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index dc5cf3b6..73fd17f4 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -94,8 +94,17 @@ sub _mset_more ($$) {
$size && (($mo->{offset} += $size) < ($mo->{limit} // 10000));
}
+# $startq will EOF when query_prepare is done augmenting and allow
+# query_mset and query_thread_mset to proceed.
+sub wait_startq ($) {
+ my ($startq) = @_;
+ $_[0] = undef;
+ read($startq, my $query_prepare_done, 1);
+}
+
sub query_thread_mset { # for --thread
my ($self, $lei, $ibxish) = @_;
+ my $startq = delete $self->{5};
my %sig = $lei->atfork_child_wq($self);
local @SIG{keys %sig} = values %sig;
@@ -119,6 +128,7 @@ sub query_thread_mset { # for --thread
while ($over->expand_thread($ctx)) {
for my $n (@{$ctx->{xids}}) {
my $smsg = $over->get_art($n) or next;
+ wait_startq($startq) if $startq;
next if $dedupe->is_smsg_dup($smsg);
my $mitem = delete $n2item{$smsg->{num}};
$each_smsg->($smsg, $mitem);
@@ -132,6 +142,7 @@ sub query_thread_mset { # for --thread
sub query_mset { # non-parallel for non-"--thread" users
my ($self, $lei, $srcs) = @_;
+ my $startq = delete $self->{5};
my %sig = $lei->atfork_child_wq($self);
local @SIG{keys %sig} = values %sig;
my $mo = { %{$lei->{mset_opt}} };
@@ -144,6 +155,7 @@ sub query_mset { # non-parallel for non-"--thread" users
$mset = $self->mset($mo->{qstr}, $mo);
for my $it ($mset->items) {
my $smsg = smsg_for($self, $it) or next;
+ wait_startq($startq) if $startq;
next if $dedupe->is_smsg_dup($smsg);
$each_smsg->($smsg, $it);
}
@@ -207,47 +219,42 @@ sub start_query { # always runs in main (lei-daemon) process
@$io = ();
}
-sub query_prepare { # wq_do
+sub query_prepare { # for wq_do,
my ($self, $lei) = @_;
my %sig = $lei->atfork_child_wq($self);
local @SIG{keys %sig} = values %sig;
- if (my $l2m = $lei->{l2m}) {
- eval { $l2m->do_augment($lei) };
- return $lei->fail($@) if $@;
- }
- # trigger PublicInbox::OpPipe->event_step
- my $qry_status_wr = $lei->{0} or
- return $lei->fail('BUG: qry_status_wr missing');
- $qry_status_wr->autoflush(1);
- print $qry_status_wr '.' or # this should never fail...
- return $lei->fail("BUG? print qry_status_wr: $!");
+ eval { $lei->{l2m}->do_augment($lei) };
+ $lei->fail($@) if $@;
}
sub do_query {
my ($self, $lei_orig, $srcs) = @_;
my ($lei, @io) = $lei_orig->atfork_parent_wq($self);
$io[0] = undef;
- pipe(my $qry_status_rd, $io[0]) or die "pipe $!";
+ pipe(my $done, $io[0]) or die "pipe $!";
$lei_orig->event_step_init; # wait for shutdowns
- my $op_map = { '' => [ \&query_done, $self, $lei_orig ] };
+ my $done_op = { '' => [ \&query_done, $self, $lei_orig ] };
my $in_loop = exists $lei_orig->{sock};
- my $opp = PublicInbox::OpPipe->new($qry_status_rd, $op_map, $in_loop);
+ $done = PublicInbox::OpPipe->new($done, $done_op, $in_loop);
my $l2m = $lei->{l2m};
if ($l2m) {
$l2m->pre_augment($lei_orig); # may redirect $lei->{1} for mbox
$io[1] = $lei_orig->{1};
- $op_map->{'.'} = [ \&start_query, $self, \@io, $lei, $srcs ];
- $self->wq_do('query_prepare', \@io, $lei);
- $opp->event_step if !$in_loop;
- } else {
- start_query($self, \@io, $lei, $srcs);
+ my @l2m_io = (undef, @io[1..$#io]);
+ pipe(my $startq, $l2m_io[0]) or die "pipe: $!";
+ $self->wq_do('query_prepare', \@l2m_io, $lei);
+ $io[4] //= *STDERR{GLOB};
+ die "BUG: unexpected \$io[5]: $io[5]" if $io[5];
+ fcntl($startq, 1031, 4096) if $^O eq 'linux'; # F_SETPIPE_SZ
+ $io[5] = $startq;
}
+ start_query($self, \@io, $lei, $srcs);
unless ($in_loop) {
my @pids = $self->wq_close;
# for the $lei->atfork_child_wq PIPE handler:
- $op_map->{'!'} = [ \&CORE::kill, 'TERM', @pids ];
- $opp->event_step;
+ $done_op->{'!'} = [ \&CORE::kill, 'TERM', @pids ];
+ $done->event_step;
my $ipc_worker_reap = $self->can('ipc_worker_reap');
if (my $l2m_pids = delete $self->{l2m_pids}) {
dwaitpid($_, $ipc_worker_reap, $l2m) for @$l2m_pids;
@@ -258,8 +265,9 @@ sub do_query {
sub ipc_atfork_prepare {
my ($self) = @_;
- # (qry_status_wr, stdout|mbox, stderr, 3: sock, 4: $l2m->{-wq_s1})
- $self->wq_set_recv_modes(qw[+<&= >&= >&= +<&= +<&=]);
+ # (0: qry_status_wr, 1: stdout|mbox, 2: stderr,
+ # 3: sock, 4: $l2m->{-wq_s1}, 5: $startq)
+ $self->wq_set_recv_modes(qw[+<&= >&= >&= +<&= +<&= <&=]);
$self->SUPER::ipc_atfork_prepare; # PublicInbox::IPC
}
diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm
index b03f2d59..376d2190 100644
--- a/lib/PublicInbox/Spawn.pm
+++ b/lib/PublicInbox/Spawn.pm
@@ -209,7 +209,7 @@ my $fdpass = <<'FDPASS';
#include <sys/socket.h>
#if defined(CMSG_SPACE) && defined(CMSG_LEN)
-#define SEND_FD_CAPA 5
+#define SEND_FD_CAPA 6
#define SEND_FD_SPACE (SEND_FD_CAPA * sizeof(int))
union my_cmsg {
struct cmsghdr hdr;
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 2/9] lei q: fix SIGPIPE handling from lei2mail workers
2021-01-19 9:34 [PATCH 0/9] lei bugfixes and error handling Eric Wong
2021-01-19 9:34 ` [PATCH 1/9] lei q: start ->mset while query_prepare runs Eric Wong
@ 2021-01-19 9:34 ` Eric Wong
2021-01-19 9:34 ` [PATCH 3/9] lei q: do not spawn MUA early Eric Wong
` (14 subsequent siblings)
16 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-01-19 9:34 UTC (permalink / raw)
To: meta
We need to properly propagate SIGPIPE to the top-level
lei-daemon process and avoid relying on auto-close,
since auto-close triggers Perl warnings when explicit
close() does not.
---
lib/PublicInbox/LEI.pm | 15 +++++++++------
lib/PublicInbox/LeiToMail.pm | 7 ++++++-
lib/PublicInbox/LeiXSearch.pm | 23 +++++++++++++++++++----
xt/lei-sigpipe.t | 29 ++++++++++++++++-------------
4 files changed, 50 insertions(+), 24 deletions(-)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 4b1dc673..802d2cd9 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -301,10 +301,13 @@ sub atfork_child_wq {
PIPE => sub {
$self->x_it(13); # SIGPIPE = 13
# we need to close explicitly to avoid Perl warning on SIGPIPE
- close(delete $self->{1});
- # regular files and /dev/null (-c) won't trigger SIGPIPE
- close(delete $self->{2}) unless (-f $self->{2} || -c _);
- syswrite($self->{0}, '!') unless $self->{sock}; # for eof_wait
+ for my $i (1, 2) {
+ next unless $self->{$i} && (-p $self->{$i} || -S _);
+ close(delete $self->{$i});
+ }
+ # trigger the LeiXSearch $done OpPipe:
+ syswrite($self->{0}, '!') if $self->{0} && -p $self->{0};
+ $SIG{PIPE} = 'DEFAULT';
die bless(\"$_[0]", 'PublicInbox::SIGPIPE'),
});
}
@@ -322,7 +325,7 @@ sub atfork_parent_wq {
my @io = delete @$ret{0..2};
$io[3] = delete($ret->{sock}) // *STDERR{GLOB};
my $l2m = $ret->{l2m};
- if ($l2m && $l2m != $wq) {
+ if ($l2m && $l2m != $wq) { # $wq == lxs
$io[4] = $l2m->{-wq_s1} if $l2m->{-wq_s1};
if (my @pids = $l2m->wq_close) {
$wq->{l2m_pids} = \@pids;
@@ -672,7 +675,7 @@ sub start_mua {
@cmd = map { $_ eq '%f' ? ($replaced = $mfolder) : $_ } @cmd;
push @cmd, $mfolder unless defined($replaced);
$sock //= $self->{sock};
- if ($PublicInbox::DS::in_loop) { # lei(1) client process runs it
+ if ($sock) { # lei(1) client process runs it
send($sock, exec_buf(\@cmd, {}), MSG_EOR);
} else { # oneshot
$self->{"mua.pid.$self.$$"} = spawn(\@cmd);
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index a1dce550..8e58ad11 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -247,11 +247,16 @@ sub _mbox_write_cb ($$) {
$dedupe->prepare_dedupe;
sub { # for git_to_mail
my ($buf, $oid, $kw) = @_;
+ return unless $out;
my $eml = PublicInbox::Eml->new($buf);
if (!$dedupe->is_dup($eml, $oid)) {
$buf = $eml2mbox->($eml, $kw);
my $lk = $ovv->lock_for_scope;
- $write->($out, $buf);
+ eval { $write->($out, $buf) };
+ if ($@) {
+ die $@ if ref($@) ne 'PublicInbox::SIGPIPE';
+ undef $out
+ }
}
}
}
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index 73fd17f4..45a073a0 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -219,7 +219,7 @@ sub start_query { # always runs in main (lei-daemon) process
@$io = ();
}
-sub query_prepare { # for wq_do,
+sub query_prepare { # called by wq_do
my ($self, $lei) = @_;
my %sig = $lei->atfork_child_wq($self);
local @SIG{keys %sig} = values %sig;
@@ -227,6 +227,18 @@ sub query_prepare { # for wq_do,
$lei->fail($@) if $@;
}
+sub sigpipe_handler {
+ my ($self, $lei_orig, $pids) = @_;
+ if ($pids) { # one-shot (no event loop)
+ kill 'TERM', @$pids;
+ kill 'PIPE', $$;
+ } else {
+ $self->wq_kill;
+ $self->wq_close;
+ }
+ close(delete $lei_orig->{1}) if $lei_orig->{1};
+}
+
sub do_query {
my ($self, $lei_orig, $srcs) = @_;
my ($lei, @io) = $lei_orig->atfork_parent_wq($self);
@@ -234,7 +246,10 @@ sub do_query {
pipe(my $done, $io[0]) or die "pipe $!";
$lei_orig->event_step_init; # wait for shutdowns
- my $done_op = { '' => [ \&query_done, $self, $lei_orig ] };
+ my $done_op = {
+ '' => [ \&query_done, $self, $lei_orig ],
+ '!' => [ \&sigpipe_handler, $self, $lei_orig ]
+ };
my $in_loop = exists $lei_orig->{sock};
$done = PublicInbox::OpPipe->new($done, $done_op, $in_loop);
my $l2m = $lei->{l2m};
@@ -244,7 +259,7 @@ sub do_query {
my @l2m_io = (undef, @io[1..$#io]);
pipe(my $startq, $l2m_io[0]) or die "pipe: $!";
$self->wq_do('query_prepare', \@l2m_io, $lei);
- $io[4] //= *STDERR{GLOB};
+ $io[4] = *STDERR{GLOB}; # don't send l2m->{-wq_s1}
die "BUG: unexpected \$io[5]: $io[5]" if $io[5];
fcntl($startq, 1031, 4096) if $^O eq 'linux'; # F_SETPIPE_SZ
$io[5] = $startq;
@@ -253,7 +268,7 @@ sub do_query {
unless ($in_loop) {
my @pids = $self->wq_close;
# for the $lei->atfork_child_wq PIPE handler:
- $done_op->{'!'} = [ \&CORE::kill, 'TERM', @pids ];
+ $done_op->{'!'}->[3] = \@pids;
$done->event_step;
my $ipc_worker_reap = $self->can('ipc_worker_reap');
if (my $l2m_pids = delete $self->{l2m_pids}) {
diff --git a/xt/lei-sigpipe.t b/xt/lei-sigpipe.t
index 4d35bbb3..448bd7db 100644
--- a/xt/lei-sigpipe.t
+++ b/xt/lei-sigpipe.t
@@ -11,19 +11,22 @@ require_mods(qw(json DBD::SQLite Search::Xapian));
my $do_test = sub {
my $env = shift // {};
- pipe(my ($r, $w)) or BAIL_OUT $!;
- open my $err, '+>', undef or BAIL_OUT $!;
- my $opt = { run_mode => 0, 1 => $w, 2 => $err };
- my $tp = start_script([qw(lei q -t), 'bytes:1..'], $env, $opt);
- close $w;
- sysread($r, my $buf, 1);
- close $r; # trigger SIGPIPE
- $tp->join;
- ok(WIFSIGNALED($?), 'signaled');
- is(WTERMSIG($?), SIGPIPE, 'got SIGPIPE');
- seek($err, 0, 0);
- my @err = grep(!m{mkdir /dev/null\b}, <$err>);
- is_deeply(\@err, [], 'no errors');
+ for my $out ([], [qw(-f mboxcl2)]) {
+ pipe(my ($r, $w)) or BAIL_OUT $!;
+ open my $err, '+>', undef or BAIL_OUT $!;
+ my $opt = { run_mode => 0, 1 => $w, 2 => $err };
+ my $cmd = [qw(lei q -t), @$out, 'bytes:1..'];
+ my $tp = start_script($cmd, $env, $opt);
+ close $w;
+ sysread($r, my $buf, 1);
+ close $r; # trigger SIGPIPE
+ $tp->join;
+ ok(WIFSIGNALED($?), "signaled @$out");
+ is(WTERMSIG($?), SIGPIPE, "got SIGPIPE @$out");
+ seek($err, 0, 0);
+ my @err = grep(!m{mkdir /dev/null\b}, <$err>);
+ is_deeply(\@err, [], "no errors @$out");
+ }
};
$do_test->();
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 3/9] lei q: do not spawn MUA early
2021-01-19 9:34 [PATCH 0/9] lei bugfixes and error handling Eric Wong
2021-01-19 9:34 ` [PATCH 1/9] lei q: start ->mset while query_prepare runs Eric Wong
2021-01-19 9:34 ` [PATCH 2/9] lei q: fix SIGPIPE handling from lei2mail workers Eric Wong
@ 2021-01-19 9:34 ` Eric Wong
2021-01-19 9:34 ` [PATCH 4/9] lei: write daemon errors to the sock directory Eric Wong
` (13 subsequent siblings)
16 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-01-19 9:34 UTC (permalink / raw)
To: meta
I'm not sure why, but mutt sometimes won't detect small
quickly. We'll display a progress bar meter when writing
results, instead.
---
lib/PublicInbox/LeiToMail.pm | 4 ----
lib/PublicInbox/LeiXSearch.pm | 3 +--
2 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index 8e58ad11..99388b5b 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -439,10 +439,6 @@ sub post_augment { # fast (spawn compressor or mkdir), runs in main daemon
$self->$m($lei);
}
-sub lock_free {
- $_[0]->{base_type} =~ /\A(?:maildir|mh|imap|jmap)\z/ ? 1 : 0;
-}
-
sub write_mail { # via ->wq_do
my ($self, $git_dir, $oid, $lei, $kw) = @_;
my $not_done = delete $self->{4}; # write end of {each_smsg_done}
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index 45a073a0..120857b8 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -191,7 +191,7 @@ sub query_done { # EOF callback
dwaitpid($_, $ipc_worker_reap, $l2m) for @$pids;
}
$lei->{ovv}->ovv_end($lei);
- $lei->start_mua if $l2m && !$l2m->lock_free;
+ $lei->start_mua if $l2m;
$lei->dclose;
}
@@ -201,7 +201,6 @@ sub start_query { # always runs in main (lei-daemon) process
$lei->{1} = $io->[1];
$l2m->post_augment($lei);
$io->[1] = delete $lei->{1};
- $lei->start_mua($io->[3]) if $l2m->lock_free;
}
my $remotes = $self->{remotes} // [];
if ($lei->{opt}->{thread}) {
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 4/9] lei: write daemon errors to the sock directory
2021-01-19 9:34 [PATCH 0/9] lei bugfixes and error handling Eric Wong
` (2 preceding siblings ...)
2021-01-19 9:34 ` [PATCH 3/9] lei q: do not spawn MUA early Eric Wong
@ 2021-01-19 9:34 ` Eric Wong
2021-01-19 9:34 ` [PATCH 5/9] lei q: fix augment of compressed mailboxes Eric Wong
` (12 subsequent siblings)
16 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-01-19 9:34 UTC (permalink / raw)
To: meta
Most everything should be captured by the __WARN__ handlers and
routed to syslog, but it appears Perl may write to stderr in
some emergency cases, as can libc or other libraries. Just
point it to a small file that's cleared on reboot.
---
lib/PublicInbox/LEI.pm | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 802d2cd9..e4f8bedb 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -830,7 +830,9 @@ sub lazy_start {
require PublicInbox::Listener;
require PublicInbox::EOFpipe;
(-p STDOUT) or die "E: stdout must be a pipe\n";
- open(STDIN, '+<', '/dev/null') or die "redirect stdin failed: $!";
+ my ($err) = ($path =~ m!\A(.+?/)[^/]+\z!);
+ $err .= 'errors.log';
+ open(STDIN, '+>>', $err) or die "open($err): $!";
POSIX::setsid() > 0 or die "setsid: $!";
my $pid = fork // die "fork: $!";
return if $pid;
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 5/9] lei q: fix augment of compressed mailboxes
2021-01-19 9:34 [PATCH 0/9] lei bugfixes and error handling Eric Wong
` (3 preceding siblings ...)
2021-01-19 9:34 ` [PATCH 4/9] lei: write daemon errors to the sock directory Eric Wong
@ 2021-01-19 9:34 ` Eric Wong
2021-01-19 9:34 ` [PATCH 6/9] lei_overview: do not write if $lei->{1} is gone Eric Wong
` (11 subsequent siblings)
16 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-01-19 9:34 UTC (permalink / raw)
To: meta
We need to delay writing out the mailbox until the compressor
process is up and running, so have startq wait a bit. This
means we must create the pipe early and hand it off to the
workers before augmenting, despite spawning the
gzip/pigz/xz/bzip2 process after augment is complete.
---
lib/PublicInbox/LEI.pm | 1 +
lib/PublicInbox/LeiToMail.pm | 19 +++++++++-------
lib/PublicInbox/LeiXSearch.pm | 40 +++++++++++++++++++++------------
t/lei.t | 42 ++++++++++++++++++++++-------------
t/lei_to_mail.t | 4 ++--
5 files changed, 66 insertions(+), 40 deletions(-)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index e4f8bedb..f3edfe82 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -758,6 +758,7 @@ sub accept_dispatch { # Listener {post_accept} callback
sub dclose {
my ($self) = @_;
delete $self->{lxs}; # stops LeiXSearch queries
+ close(delete $self->{1}) if $self->{1}; # may reap_compress
$self->close if $self->{sock}; # PublicInbox::DS::close
}
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index 99388b5b..a6e517ea 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -200,18 +200,19 @@ sub zsfx2cmd ($$$) {
}
sub _post_augment_mbox { # open a compressor process
- my ($self, $lei) = @_;
+ my ($self, $lei, $zpipe) = @_;
my $zsfx = $self->{zsfx} or return;
my $cmd = zsfx2cmd($zsfx, undef, $lei);
- pipe(my ($r, $w)) or die "pipe: $!";
+ my ($r, $w) = splice(@$zpipe, 0, 2);
my $rdr = { 0 => $r, 1 => $lei->{1}, 2 => $lei->{2} };
my $pid = spawn($cmd, $lei->{env}, $rdr);
- $lei->{"pid.$pid"} = $cmd;
my $pp = gensym;
- tie *$pp, 'PublicInbox::ProcessPipe', $pid, $w, \&reap_compress, $lei;
+ my $dup = bless { "pid.$pid" => $cmd }, ref($lei);
+ $dup->{$_} = $lei->{$_} for qw(2 sock);
+ tie *$pp, 'PublicInbox::ProcessPipe', $pid, $w, \&reap_compress, $dup;
$lei->{1} = $pp;
die 'BUG: unexpected {ovv}->{lock_path}' if $lei->{ovv}->{lock_path};
- $lei->{ovv}->ovv_out_lk_init if ($lei->{opt}->{jobs} // 2) > 1;
+ $lei->{ovv}->ovv_out_lk_init;
}
sub decompress_src ($$$) {
@@ -395,7 +396,9 @@ sub _pre_augment_mbox {
die "seek($dst): $!\n";
}
state $zsfx_allow = join('|', keys %zsfx2cmd);
- ($self->{zsfx}) = ($dst =~ /\.($zsfx_allow)\z/);
+ ($self->{zsfx}) = ($dst =~ /\.($zsfx_allow)\z/) or return;
+ pipe(my ($r, $w)) or die "pipe: $!";
+ [ $r, $w ];
}
sub _do_augment_mbox {
@@ -433,10 +436,10 @@ sub do_augment { # slow, runs in wq worker
}
sub post_augment { # fast (spawn compressor or mkdir), runs in main daemon
- my ($self, $lei) = @_;
+ my ($self, $lei, @args) = @_;
# _post_augment_maildir, _post_augment_mbox
my $m = "_post_augment_$self->{base_type}";
- $self->$m($lei);
+ $self->$m($lei, @args);
}
sub write_mail { # via ->wq_do
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index 120857b8..002791c2 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -191,17 +191,22 @@ sub query_done { # EOF callback
dwaitpid($_, $ipc_worker_reap, $l2m) for @$pids;
}
$lei->{ovv}->ovv_end($lei);
- $lei->start_mua if $l2m;
+ if ($l2m) { # calls LeiToMail reap_compress
+ close(delete($lei->{1})) if $lei->{1};
+ $lei->start_mua;
+ }
$lei->dclose;
}
+sub do_post_augment {
+ my ($lei, $zpipe, $au_done) = @_;
+ my $l2m = $lei->{l2m} or die 'BUG: no {l2m}';
+ $l2m->post_augment($lei, $zpipe);
+ close $au_done; # triggers wait_startq
+}
+
sub start_query { # always runs in main (lei-daemon) process
my ($self, $io, $lei, $srcs) = @_;
- if (my $l2m = $lei->{l2m}) {
- $lei->{1} = $io->[1];
- $l2m->post_augment($lei);
- $io->[1] = delete $lei->{1};
- }
my $remotes = $self->{remotes} // [];
if ($lei->{opt}->{thread}) {
for my $ibxish (@$srcs) {
@@ -221,9 +226,11 @@ sub start_query { # always runs in main (lei-daemon) process
sub query_prepare { # called by wq_do
my ($self, $lei) = @_;
my %sig = $lei->atfork_child_wq($self);
+ -p $lei->{0} or die "BUG: \$done pipe expected";
local @SIG{keys %sig} = values %sig;
eval { $lei->{l2m}->do_augment($lei) };
$lei->fail($@) if $@;
+ syswrite($lei->{0}, '.') == 1 or die "do_post_augment trigger: $!";
}
sub sigpipe_handler {
@@ -253,26 +260,31 @@ sub do_query {
$done = PublicInbox::OpPipe->new($done, $done_op, $in_loop);
my $l2m = $lei->{l2m};
if ($l2m) {
- $l2m->pre_augment($lei_orig); # may redirect $lei->{1} for mbox
+ # may redirect $lei->{1} for mbox
+ my $zpipe = $l2m->pre_augment($lei_orig);
$io[1] = $lei_orig->{1};
- my @l2m_io = (undef, @io[1..$#io]);
- pipe(my $startq, $l2m_io[0]) or die "pipe: $!";
- $self->wq_do('query_prepare', \@l2m_io, $lei);
+ pipe(my ($startq, $au_done)) or die "pipe: $!";
+ $done_op->{'.'} = [ \&do_post_augment, $lei_orig,
+ $zpipe, $au_done ];
$io[4] = *STDERR{GLOB}; # don't send l2m->{-wq_s1}
+ $self->wq_do('query_prepare', \@io, $lei);
die "BUG: unexpected \$io[5]: $io[5]" if $io[5];
fcntl($startq, 1031, 4096) if $^O eq 'linux'; # F_SETPIPE_SZ
$io[5] = $startq;
+ $io[1] = $zpipe->[1] if $zpipe;
}
start_query($self, \@io, $lei, $srcs);
unless ($in_loop) {
my @pids = $self->wq_close;
# for the $lei->atfork_child_wq PIPE handler:
$done_op->{'!'}->[3] = \@pids;
- $done->event_step;
+ # $done->event_step;
+ # my $ipc_worker_reap = $self->can('ipc_worker_reap');
+ # if (my $l2m_pids = delete $self->{l2m_pids}) {
+ # dwaitpid($_, $ipc_worker_reap, $l2m) for @$l2m_pids;
+ # }
+ while ($done->{sock}) { $done->event_step }
my $ipc_worker_reap = $self->can('ipc_worker_reap');
- if (my $l2m_pids = delete $self->{l2m_pids}) {
- dwaitpid($_, $ipc_worker_reap, $l2m) for @$l2m_pids;
- }
dwaitpid($_, $ipc_worker_reap, $self) for @pids;
}
}
diff --git a/t/lei.t b/t/lei.t
index c4692217..8eede13e 100644
--- a/t/lei.t
+++ b/t/lei.t
@@ -189,25 +189,35 @@ my $test_external = sub {
# No double-quoting should be imposed on users on the CLI
$lei->('q', 's:use boolean prefix');
like($out, qr/search: use boolean prefix/, 'phrase search got result');
+ require IO::Uncompress::Gunzip;
+ for my $sfx ('', '.gz') {
+ my $f = "$home/mbox$sfx";
+ $lei->('q', '-o', "mboxcl2:$f", 's:use boolean prefix');
+ my $cat = $sfx eq '' ? sub {
+ open my $mb, '<', $f or fail "no mbox: $!";
+ <$mb>
+ } : sub {
+ my $z = IO::Uncompress::Gunzip->new($f, MultiStream=>1);
+ <$z>;
+ };
+ my @s = grep(/^Subject:/, $cat->());
+ is(scalar(@s), 1, "1 result in mbox$sfx");
+ $lei->('q', '-a', '-o', "mboxcl2:$f", 's:see attachment');
+ is($err, '', 'no errors from augment');
+ @s = grep(/^Subject:/, my @wtf = $cat->());
+ is(scalar(@s), 2, "2 results in mbox$sfx");
- $lei->('q', '-o', "mboxcl2:$home/mbox", 's:use boolean prefix');
- open my $mb, '<', "$home/mbox" or fail "no mbox: $!";
- my @s = grep(/^Subject:/, <$mb>);
- is(scalar(@s), 1, '1 result in mbox');
- $lei->('q', '-a', '-o', "mboxcl2:$home/mbox", 's:see attachment');
- is($err, '', 'no errors from augment');
- seek($mb, 0, SEEK_SET) or BAIL_OUT "seek: $!";
- @s = grep(/^Subject:/, <$mb>);
- is(scalar(@s), 2, '2 results in mbox');
+ $lei->('q', '-a', '-o', "mboxcl2:$f", 's:nonexistent');
+ is($err, '', "no errors on no results ($sfx)");
- $lei->('q', '-a', '-o', "mboxcl2:$home/mbox", 's:nonexistent');
- is($err, '', 'no errors on no results');
- seek($mb, 0, SEEK_SET) or BAIL_OUT "seek: $!";
- my @s2 = grep(/^Subject:/, <$mb>);
- is_deeply(\@s2, \@s, 'same 2 old results w/ --augment and bad search');
+ my @s2 = grep(/^Subject:/, $cat->());
+ is_deeply(\@s2, \@s,
+ "same 2 old results w/ --augment and bad search $sfx");
- $lei->('q', '-o', "mboxcl2:$home/mbox", 's:nonexistent');
- is(-s "$home/mbox", 0, 'clobber w/o --augment');
+ $lei->('q', '-o', "mboxcl2:$f", 's:nonexistent');
+ my @res = $cat->();
+ is_deeply(\@res, [], "clobber w/o --augment $sfx");
+ }
};
my $test_lei_common = sub {
diff --git a/t/lei_to_mail.t b/t/lei_to_mail.t
index e5ac8eac..6673d9a6 100644
--- a/t/lei_to_mail.t
+++ b/t/lei_to_mail.t
@@ -94,9 +94,9 @@ my $wcb_get = sub {
my $dup = Storable::thaw(Storable::freeze($l2m));
is_deeply($dup, $l2m, "$fmt round-trips through storable");
}
- $l2m->pre_augment($lei);
+ my $zpipe = $l2m->pre_augment($lei);
$l2m->do_augment($lei);
- $l2m->post_augment($lei);
+ $l2m->post_augment($lei, $zpipe);
my $cb = $l2m->write_cb($lei);
delete $lei->{1};
$cb;
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 6/9] lei_overview: do not write if $lei->{1} is gone
2021-01-19 9:34 [PATCH 0/9] lei bugfixes and error handling Eric Wong
` (4 preceding siblings ...)
2021-01-19 9:34 ` [PATCH 5/9] lei q: fix augment of compressed mailboxes Eric Wong
@ 2021-01-19 9:34 ` Eric Wong
2021-01-19 9:34 ` [PATCH 7/9] t/lei: fix double-running of socket test with oneshot Eric Wong
` (10 subsequent siblings)
16 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-01-19 9:34 UTC (permalink / raw)
To: meta
We'll invalidate the {1} (stdout) field on SIGPIPE,
so don't trigger a Perl warning by writing to it.
---
lib/PublicInbox/LeiOverview.pm | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/PublicInbox/LeiOverview.pm b/lib/PublicInbox/LeiOverview.pm
index 538d6bd5..8781259a 100644
--- a/lib/PublicInbox/LeiOverview.pm
+++ b/lib/PublicInbox/LeiOverview.pm
@@ -99,12 +99,13 @@ sub ovv_begin {
# called once by parent (via PublicInbox::EOFpipe)
sub ovv_end {
my ($self, $lei) = @_;
+ my $out = $lei->{1} or return;
if ($self->{fmt} eq 'json') {
# JSON doesn't allow trailing commas, and preventing
# trailing commas is a PITA when parallelizing outputs
- print { $lei->{1} } "null]\n";
+ print $out "null]\n";
} elsif ($self->{fmt} eq 'concatjson') {
- print { $lei->{1} } "\n";
+ print $out "\n";
}
}
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 7/9] t/lei: fix double-running of socket test with oneshot
2021-01-19 9:34 [PATCH 0/9] lei bugfixes and error handling Eric Wong
` (5 preceding siblings ...)
2021-01-19 9:34 ` [PATCH 6/9] lei_overview: do not write if $lei->{1} is gone Eric Wong
@ 2021-01-19 9:34 ` Eric Wong
2021-01-19 9:34 ` [PATCH 8/9] lei: test some likely errors due to misuse Eric Wong
` (9 subsequent siblings)
16 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-01-19 9:34 UTC (permalink / raw)
To: meta
We split out t/lei-oneshot.t and t/lei.t so it's easier
to isolate run-mode specific bugs and behavior and there's
no reason to rerun the socket daemon tests.
---
t/lei.t | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/t/lei.t b/t/lei.t
index 8eede13e..c804ff59 100644
--- a/t/lei.t
+++ b/t/lei.t
@@ -234,18 +234,14 @@ if ($ENV{TEST_LEI_ONESHOT}) {
local $ENV{XDG_RUNTIME_DIR} = $xrd;
$err_filter = qr!\Q$xrd!;
$test_lei_common->();
-}
-
+} else {
SKIP: { # real socket
- require_mods(qw(Cwd), my $nr = 105);
- my $nfd = eval { require Socket::MsgHdr; 5 } // do {
+ eval { require Socket::MsgHdr; 1 } // do {
require PublicInbox::Spawn;
- PublicInbox::Spawn->can('send_cmd4') ? 5 : undef;
- } //
- skip 'Socket::MsgHdr or Inline::C missing or unconfigured', $nr;
-
+ PublicInbox::Spawn->can('send_cmd4');
+ } // skip 'Socket::MsgHdr or Inline::C missing or unconfigured', 115;
local $ENV{XDG_RUNTIME_DIR} = "$home/xdg_run";
- my $sock = "$ENV{XDG_RUNTIME_DIR}/lei/$nfd.seq.sock";
+ my $sock = "$ENV{XDG_RUNTIME_DIR}/lei/5.seq.sock";
ok($lei->('daemon-pid'), 'daemon-pid');
is($err, '', 'no error from daemon-pid');
@@ -297,6 +293,7 @@ SKIP: { # real socket
}
ok(!kill(0, $new_pid), 'daemon exits after unlink');
# success over socket, can't test without
-};
+}; # SKIP
+} # else
done_testing;
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 8/9] lei: test some likely errors due to misuse
2021-01-19 9:34 [PATCH 0/9] lei bugfixes and error handling Eric Wong
` (6 preceding siblings ...)
2021-01-19 9:34 ` [PATCH 7/9] t/lei: fix double-running of socket test with oneshot Eric Wong
@ 2021-01-19 9:34 ` Eric Wong
2021-01-19 9:34 ` [PATCH 9/9] lei_overview: start implementing format detection Eric Wong
` (8 subsequent siblings)
16 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-01-19 9:34 UTC (permalink / raw)
To: meta
Because user errors happen...
---
lib/PublicInbox/LeiOverview.pm | 3 ++-
lib/PublicInbox/LeiToMail.pm | 6 +++++-
lib/PublicInbox/LeiXSearch.pm | 9 ++++++++-
t/lei.t | 14 ++++++++++++++
4 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/lib/PublicInbox/LeiOverview.pm b/lib/PublicInbox/LeiOverview.pm
index 8781259a..a7021b03 100644
--- a/lib/PublicInbox/LeiOverview.pm
+++ b/lib/PublicInbox/LeiOverview.pm
@@ -82,7 +82,8 @@ sub new {
if (!$json) {
# default to the cheapest sort since MUA usually resorts
$lei->{opt}->{'sort'} //= 'docid' if $dst ne '/dev/stdout';
- $lei->{l2m} = PublicInbox::LeiToMail->new($lei);
+ $lei->{l2m} = eval { PublicInbox::LeiToMail->new($lei) };
+ return $lei->fail($@) if $@;
}
$lei->{dedupe} //= PublicInbox::LeiDedupe->new($lei);
$self;
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index a6e517ea..49b5c8ab 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -339,8 +339,12 @@ sub new {
my $self = bless {}, $cls;
if ($fmt eq 'maildir') {
$self->{base_type} = 'maildir';
+ -e $dst && !-d _ and die
+ "$dst exists and is not a directory\n";
$lei->{ovv}->{dst} = $dst .= '/' if substr($dst, -1) ne '/';
} elsif (substr($fmt, 0, 4) eq 'mbox') {
+ -e $dst && !-f _ && !-p _ and die
+ "$dst exists and is not a regular file\n";
$self->can("eml2$fmt") or die "bad mbox --format=$fmt\n";
$self->{base_type} = 'mbox';
} else {
@@ -374,7 +378,7 @@ sub _post_augment_maildir {
my $d = $dst.$x;
next if -d $d;
require File::Path;
- File::Path::mkpath($d) or die "mkpath($d): $!";
+ File::Path::mkpath($d);
-d $d or die "$d is not a directory";
}
}
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index 002791c2..fa37543f 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -201,7 +201,14 @@ sub query_done { # EOF callback
sub do_post_augment {
my ($lei, $zpipe, $au_done) = @_;
my $l2m = $lei->{l2m} or die 'BUG: no {l2m}';
- $l2m->post_augment($lei, $zpipe);
+ eval { $l2m->post_augment($lei, $zpipe) };
+ if (my $err = $@) {
+ if (my $lxs = delete $lei->{lxs}) {
+ $lxs->wq_kill;
+ $lxs->wq_close;
+ }
+ $lei->fail("$err");
+ }
close $au_done; # triggers wait_startq
}
diff --git a/t/lei.t b/t/lei.t
index c804ff59..8bb4e439 100644
--- a/t/lei.t
+++ b/t/lei.t
@@ -181,6 +181,20 @@ my $test_external = sub {
$lei->('ls-external');
like($out, qr/boost=0\n/s, 'ls-external has output');
+ ok(!$lei->(qw(q s:prefix -o /dev/null -f maildir)), 'bad maildir');
+ like($err, qr!/dev/null exists and is not a directory!,
+ 'error shown');
+ is($? >> 8, 1, 'errored out with exit 1');
+
+ ok(!$lei->(qw(q s:prefix -f mboxcl2 -o), $home), 'bad mbox');
+ like($err, qr!\Q$home\E exists and is not a regular file!,
+ 'error shown');
+ is($? >> 8, 1, 'errored out with exit 1');
+
+ ok(!$lei->(qw(q s:prefix -o /dev/stdout -f Mbox2)), 'bad format');
+ like($err, qr/bad mbox --format=mbox2/, 'error shown');
+ is($? >> 8, 1, 'errored out with exit 1');
+
# note, on a Bourne shell users should be able to use either:
# s:"use boolean prefix"
# "s:use boolean prefix"
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 9/9] lei_overview: start implementing format detection
2021-01-19 9:34 [PATCH 0/9] lei bugfixes and error handling Eric Wong
` (7 preceding siblings ...)
2021-01-19 9:34 ` [PATCH 8/9] lei: test some likely errors due to misuse Eric Wong
@ 2021-01-19 9:34 ` Eric Wong
2021-01-20 5:04 ` [PATCH 0/7] lei: fixes piled higher and deeper Eric Wong
` (7 subsequent siblings)
16 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-01-19 9:34 UTC (permalink / raw)
To: meta
We'll need it for IMAP support, at least. Proper mbox family
detection will be expensive, so deal with it later.
---
lib/PublicInbox/LeiOverview.pm | 17 ++++++++++++++++-
t/lei.t | 2 ++
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/lib/PublicInbox/LeiOverview.pm b/lib/PublicInbox/LeiOverview.pm
index a7021b03..dcc3088b 100644
--- a/lib/PublicInbox/LeiOverview.pm
+++ b/lib/PublicInbox/LeiOverview.pm
@@ -38,6 +38,21 @@ sub ovv_out_lk_cancel ($) {
unlink(delete($self->{lock_path}));
}
+sub detect_fmt ($$) {
+ my ($lei, $dst) = @_;
+ if ($dst =~ m!\A([:/]+://)!) {
+ $lei->fail("$1 support not implemented, yet\n");
+ } elsif (!-e $dst) {
+ 'maildir'; # the default
+ } elsif (-f _ || -p _) {
+ $lei->fail("unable to determine mbox family of $dst\n");
+ } elsif (-d _) { # TODO: MH?
+ 'maildir';
+ } else {
+ $lei->fail("unable to determine format of $dst\n");
+ }
+}
+
sub new {
my ($class, $lei) = @_;
my $opt = $lei->{opt};
@@ -54,7 +69,7 @@ sub new {
}
$fmt //= 'json' if $dst eq '/dev/stdout';
- $fmt //= 'maildir';
+ $fmt //= detect_fmt($lei, $dst) or return;
if (index($dst, '://') < 0) { # not a URL, so assume path
$dst = File::Spec->canonpath($dst);
diff --git a/t/lei.t b/t/lei.t
index 8bb4e439..64cb5f0e 100644
--- a/t/lei.t
+++ b/t/lei.t
@@ -232,6 +232,8 @@ my $test_external = sub {
my @res = $cat->();
is_deeply(\@res, [], "clobber w/o --augment $sfx");
}
+ ok(!$lei->('q', '-o', "$home/mbox", 's:nope'),
+ 'fails if mbox format unspecified');
};
my $test_lei_common = sub {
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 0/7] lei: fixes piled higher and deeper
2021-01-19 9:34 [PATCH 0/9] lei bugfixes and error handling Eric Wong
` (8 preceding siblings ...)
2021-01-19 9:34 ` [PATCH 9/9] lei_overview: start implementing format detection Eric Wong
@ 2021-01-20 5:04 ` Eric Wong
2021-01-20 5:16 ` misnumbered, should be [PATCH 10/9]..[PATCH 16/9] :x Eric Wong
2021-01-20 5:04 ` [PATCH 1/7] lei: allow more mbox inode types Eric Wong
` (6 subsequent siblings)
16 siblings, 1 reply; 19+ messages in thread
From: Eric Wong @ 2021-01-20 5:04 UTC (permalink / raw)
To: meta
1/7 was necessary on my FreeBSD 11.x VM
2/7 fixes TEST_RUN_MODE=0
3/7 fixes a long-standing (well, several weeks) annoyance
4/7 depended on 3/7, sorta
5/7 should've been done ages ago
6/7 oops :x
7/7 belts and suspenders
Eric Wong (7):
lei: allow more mbox inode types
lei: exit code in oneshot mode
overidx: eidx_prep: fix leftover dbh reference
lei q: cleanup store initialization
lei: dump and clear errors.log in daemon mode
lei_xsearch: keep l2m->{-wq_s1} while preparing query
lei_to_mail: call PublicInbox::IPC::DESTROY
lib/PublicInbox/LEI.pm | 32 +++++++++++++++++++++++++++-----
lib/PublicInbox/LeiOverview.pm | 6 ++----
lib/PublicInbox/LeiQuery.pm | 18 ++++++++----------
lib/PublicInbox/LeiToMail.pm | 5 +++--
lib/PublicInbox/LeiXSearch.pm | 4 ++--
lib/PublicInbox/OverIdx.pm | 8 +++-----
t/lei.t | 8 +++++++-
7 files changed, 52 insertions(+), 29 deletions(-)
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 1/7] lei: allow more mbox inode types
2021-01-19 9:34 [PATCH 0/9] lei bugfixes and error handling Eric Wong
` (9 preceding siblings ...)
2021-01-20 5:04 ` [PATCH 0/7] lei: fixes piled higher and deeper Eric Wong
@ 2021-01-20 5:04 ` Eric Wong
2021-01-20 5:04 ` [PATCH 2/7] lei: exit code in oneshot mode Eric Wong
` (5 subsequent siblings)
16 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-01-20 5:04 UTC (permalink / raw)
To: meta
We may attempt to write an mbox to any terminal, block, or
character device, not just regular files and FIFOs/pipes.
The only thing that is known to not work is a directory.
Sockets may be possible with some OSes (e.g. Plan 9) or
filesystems. This fixes t/lei.t on FreeBSD 11.x
---
lib/PublicInbox/LeiOverview.pm | 6 ++----
lib/PublicInbox/LeiToMail.pm | 4 ++--
t/lei.t | 2 +-
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/lib/PublicInbox/LeiOverview.pm b/lib/PublicInbox/LeiOverview.pm
index dcc3088b..cab2b055 100644
--- a/lib/PublicInbox/LeiOverview.pm
+++ b/lib/PublicInbox/LeiOverview.pm
@@ -42,12 +42,10 @@ sub detect_fmt ($$) {
my ($lei, $dst) = @_;
if ($dst =~ m!\A([:/]+://)!) {
$lei->fail("$1 support not implemented, yet\n");
- } elsif (!-e $dst) {
- 'maildir'; # the default
+ } elsif (!-e $dst || -d _) {
+ 'maildir'; # the default TODO: MH?
} elsif (-f _ || -p _) {
$lei->fail("unable to determine mbox family of $dst\n");
- } elsif (-d _) { # TODO: MH?
- 'maildir';
} else {
$lei->fail("unable to determine format of $dst\n");
}
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index 49b5c8ab..9d9b5748 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -343,8 +343,8 @@ sub new {
"$dst exists and is not a directory\n";
$lei->{ovv}->{dst} = $dst .= '/' if substr($dst, -1) ne '/';
} elsif (substr($fmt, 0, 4) eq 'mbox') {
- -e $dst && !-f _ && !-p _ and die
- "$dst exists and is not a regular file\n";
+ (-d $dst || (-e _ && !-w _)) and die
+ "$dst exists and is not a writable file\n";
$self->can("eml2$fmt") or die "bad mbox --format=$fmt\n";
$self->{base_type} = 'mbox';
} else {
diff --git a/t/lei.t b/t/lei.t
index 64cb5f0e..d49dc01a 100644
--- a/t/lei.t
+++ b/t/lei.t
@@ -187,7 +187,7 @@ my $test_external = sub {
is($? >> 8, 1, 'errored out with exit 1');
ok(!$lei->(qw(q s:prefix -f mboxcl2 -o), $home), 'bad mbox');
- like($err, qr!\Q$home\E exists and is not a regular file!,
+ like($err, qr!\Q$home\E exists and is not a writable file!,
'error shown');
is($? >> 8, 1, 'errored out with exit 1');
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 2/7] lei: exit code in oneshot mode
2021-01-19 9:34 [PATCH 0/9] lei bugfixes and error handling Eric Wong
` (10 preceding siblings ...)
2021-01-20 5:04 ` [PATCH 1/7] lei: allow more mbox inode types Eric Wong
@ 2021-01-20 5:04 ` Eric Wong
2021-01-20 5:04 ` [PATCH 3/7] overidx: eidx_prep: fix leftover dbh reference Eric Wong
` (4 subsequent siblings)
16 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-01-20 5:04 UTC (permalink / raw)
To: meta
waitpid() in DESTROY ends up setting $? for the exit status,
thus we must reap IPC children before calling CORE::exit.
This fixes t/lei-oneshot.t with TEST_RUN_MODE=0
---
lib/PublicInbox/LEI.pm | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index f3edfe82..97ae2c41 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -249,6 +249,11 @@ sub x_it ($$) {
if (my $sock = $self->{sock}) {
send($sock, "x_it $code", MSG_EOR);
} elsif (!($code & 127)) { # oneshot, ignore signals
+ # don't want to end up using $? from child processes
+ for my $f (qw(lxs l2m)) {
+ my $wq = delete $self->{$f} or next;
+ $wq->DESTROY;
+ }
$quit->($code >> 8);
}
}
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 3/7] overidx: eidx_prep: fix leftover dbh reference
2021-01-19 9:34 [PATCH 0/9] lei bugfixes and error handling Eric Wong
` (11 preceding siblings ...)
2021-01-20 5:04 ` [PATCH 2/7] lei: exit code in oneshot mode Eric Wong
@ 2021-01-20 5:04 ` Eric Wong
2021-01-20 5:04 ` [PATCH 4/7] lei q: cleanup store initialization Eric Wong
` (3 subsequent siblings)
16 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-01-20 5:04 UTC (permalink / raw)
To: meta
Leaving $dbh in another field was causing over.sqlite3 to
remain open after ->dbh_close. Fix up some minor style
issues while we're at it.
---
lib/PublicInbox/OverIdx.pm | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/lib/PublicInbox/OverIdx.pm b/lib/PublicInbox/OverIdx.pm
index 0a4eb39e..e606dcf5 100644
--- a/lib/PublicInbox/OverIdx.pm
+++ b/lib/PublicInbox/OverIdx.pm
@@ -537,7 +537,7 @@ sub eidx_prep ($) {
my ($self) = @_;
$self->{-eidx_prep} //= do {
my $dbh = $self->dbh;
- $dbh->do(<<"");
+ $dbh->do(<<'');
INSERT OR IGNORE INTO counter (key) VALUES ('eidx_docid')
$dbh->do(<<'');
@@ -574,11 +574,9 @@ CREATE TABLE IF NOT EXISTS eidx_meta (
# Currently used for "-extindex --reindex" for Xapian
# data, but may be used in more places down the line.
$dbh->do(<<'');
-CREATE TABLE IF NOT EXISTS eidxq (
- docid INTEGER PRIMARY KEY NOT NULL
-)
+CREATE TABLE IF NOT EXISTS eidxq (docid INTEGER PRIMARY KEY NOT NULL)
- $dbh;
+ 1;
};
}
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 4/7] lei q: cleanup store initialization
2021-01-19 9:34 [PATCH 0/9] lei bugfixes and error handling Eric Wong
` (12 preceding siblings ...)
2021-01-20 5:04 ` [PATCH 3/7] overidx: eidx_prep: fix leftover dbh reference Eric Wong
@ 2021-01-20 5:04 ` Eric Wong
2021-01-20 5:04 ` [PATCH 5/7] lei: dump and clear errors.log in daemon mode Eric Wong
` (2 subsequent siblings)
16 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-01-20 5:04 UTC (permalink / raw)
To: meta
Since we no longer leak an FD for over.sqlite3, we can
initialize and actually enable it by default as originally
intended.
---
lib/PublicInbox/LeiQuery.pm | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/lib/PublicInbox/LeiQuery.pm b/lib/PublicInbox/LeiQuery.pm
index d6e801e3..941bc299 100644
--- a/lib/PublicInbox/LeiQuery.pm
+++ b/lib/PublicInbox/LeiQuery.pm
@@ -23,14 +23,15 @@ sub _vivify_external { # _externals_each callback
# the main "lei q SEARCH_TERMS" method
sub lei_q {
my ($self, @argv) = @_;
- my $opt = $self->{opt};
-
- # --local is enabled by default
- # src: LeiXSearch || LeiSearch || Inbox
- my @srcs;
require PublicInbox::LeiXSearch;
require PublicInbox::LeiOverview;
- PublicInbox::Config->json;
+ PublicInbox::Config->json; # preload before forking
+ my $opt = $self->{opt};
+ my @srcs; # any number of LeiXSearch || LeiSearch || Inbox
+ if ($opt->{'local'} //= 1) { # --local is enabled by default
+ my $sto = $self->_lei_store(1);
+ push @srcs, $sto->search;
+ }
my $lxs = PublicInbox::LeiXSearch->new;
# --external is enabled by default, but allow --no-external
@@ -39,7 +40,6 @@ sub lei_q {
}
my $j = $opt->{jobs} // (scalar(@srcs) > 3 ? 3 : scalar(@srcs));
$j = 1 if !$opt->{thread};
- $j++ if $opt->{'local'}; # for sto->search below
$self->atfork_prepare_wq($lxs);
$lxs->wq_workers_start('lei_xsearch', $j, $self->oldset);
$self->{lxs} = $lxs;
@@ -50,10 +50,8 @@ sub lei_q {
$self->atfork_prepare_wq($l2m);
$l2m->wq_workers_start('lei2mail', $j, $self->oldset);
}
-
# no forking workers after this
- my $sto = $self->_lei_store(1);
- unshift(@srcs, $sto->search) if $opt->{'local'};
+
my %mset_opt = map { $_ => $opt->{$_} } qw(thread limit offset);
$mset_opt{asc} = $opt->{'reverse'} ? 1 : 0;
$mset_opt{qstr} = join(' ', map {;
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 5/7] lei: dump and clear errors.log in daemon mode
2021-01-19 9:34 [PATCH 0/9] lei bugfixes and error handling Eric Wong
` (13 preceding siblings ...)
2021-01-20 5:04 ` [PATCH 4/7] lei q: cleanup store initialization Eric Wong
@ 2021-01-20 5:04 ` Eric Wong
2021-01-20 5:04 ` [PATCH 6/7] lei_xsearch: keep l2m->{-wq_s1} while preparing query Eric Wong
2021-01-20 5:04 ` [PATCH 7/7] lei_to_mail: call PublicInbox::IPC::DESTROY Eric Wong
16 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-01-20 5:04 UTC (permalink / raw)
To: meta
Inspired by "dmesg -c", this should help users report bugs
and avoids eating up $XDG_RUNTIME_DIR.
Once lei is ready for release, hopefully the need for this
should be few an far between, but shit happens.
---
lib/PublicInbox/LEI.pm | 27 ++++++++++++++++++++++-----
t/lei.t | 6 ++++++
2 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 97ae2c41..6be6d10b 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -15,6 +15,7 @@ use Socket qw(AF_UNIX SOCK_SEQPACKET MSG_EOR pack_sockaddr_un);
use Errno qw(EAGAIN EINTR ECONNREFUSED ENOENT ECONNRESET);
use POSIX ();
use IO::Handle ();
+use Fcntl qw(SEEK_SET);
use Sys::Syslog qw(syslog openlog);
use PublicInbox::Config;
use PublicInbox::Syscall qw(SFD_NONBLOCK EPOLLIN EPOLLET);
@@ -26,7 +27,7 @@ use Text::Wrap qw(wrap);
use File::Path qw(mkpath);
use File::Spec;
our $quit = \&CORE::exit;
-our $current_lei;
+our ($current_lei, $errors_log);
my ($recv_cmd, $send_cmd);
my $GLP = Getopt::Long::Parser->new;
$GLP->configure(qw(gnu_getopt no_ignore_case auto_abbrev));
@@ -246,6 +247,7 @@ sub x_it ($$) {
my ($self, $code) = @_;
# make sure client sees stdout before exit
$self->{1}->autoflush(1) if $self->{1};
+ dump_and_clear_log();
if (my $sock = $self->{sock}) {
send($sock, "x_it $code", MSG_EOR);
} elsif (!($code & 127)) { # oneshot, ignore signals
@@ -264,7 +266,7 @@ sub out ($;@) { print { shift->{1} } @_ }
sub err ($;@) {
my $self = shift;
- my $err = $self->{2} // *STDERR{IO};
+ my $err = $self->{2} // ($self->{pgr} // [])->[2] // *STDERR{IO};
print $err @_, (substr($_[-1], -1, 1) eq "\n" ? () : "\n");
}
@@ -300,6 +302,7 @@ sub atfork_child_wq {
$self->{sock} = $sock if -S $sock;
$self->{l2m}->{-wq_s1} = $l2m_wq_s1 if $l2m_wq_s1 && -S $l2m_wq_s1;
%PATH2CFG = ();
+ undef $errors_log;
$quit = \&CORE::exit;
@TO_CLOSE_ATFORK_CHILD = ();
(__WARN__ => sub { err($self, @_) },
@@ -483,6 +486,7 @@ sub optparse ($$$) {
sub dispatch {
my ($self, $cmd, @argv) = @_;
local $current_lei = $self; # for __WARN__
+ dump_and_clear_log("from previous run\n");
return _help($self, 'no command given') unless defined($cmd);
my $func = "lei_$cmd";
$func =~ tr/-/_/;
@@ -772,6 +776,7 @@ sub event_step {
my ($self) = @_;
local %ENV = %{$self->{env}};
my $sock = $self->{sock};
+ local $current_lei = $self;
eval {
while (my @fds = $recv_cmd->($sock, my $buf, 4096)) {
if (scalar(@fds) == 1 && !defined($fds[0])) {
@@ -805,6 +810,15 @@ sub noop {}
our $oldset; sub oldset { $oldset }
+sub dump_and_clear_log {
+ if (defined($errors_log) && -s STDIN && seek(STDIN, 0, SEEK_SET)) {
+ my @pfx = @_;
+ unshift(@pfx, "$errors_log ") if @pfx;
+ warn @pfx, do { local $/; <STDIN> };
+ truncate(STDIN, 0) or warn "ftruncate ($errors_log): $!";
+ }
+}
+
# lei(1) calls this when it can't connect
sub lazy_start {
my ($path, $errno, $narg) = @_;
@@ -836,9 +850,12 @@ sub lazy_start {
require PublicInbox::Listener;
require PublicInbox::EOFpipe;
(-p STDOUT) or die "E: stdout must be a pipe\n";
- my ($err) = ($path =~ m!\A(.+?/)[^/]+\z!);
- $err .= 'errors.log';
- open(STDIN, '+>>', $err) or die "open($err): $!";
+ local $errors_log;
+ ($errors_log) = ($path =~ m!\A(.+?/)[^/]+\z!);
+ $errors_log .= 'errors.log';
+ open(STDIN, '+>>', $errors_log) or die "open($errors_log): $!";
+ STDIN->autoflush(1);
+ dump_and_clear_log("from previous daemon process:\n");
POSIX::setsid() > 0 or die "setsid: $!";
my $pid = fork // die "fork: $!";
return if $pid;
diff --git a/t/lei.t b/t/lei.t
index d49dc01a..ef820fe3 100644
--- a/t/lei.t
+++ b/t/lei.t
@@ -258,6 +258,7 @@ SKIP: { # real socket
} // skip 'Socket::MsgHdr or Inline::C missing or unconfigured', 115;
local $ENV{XDG_RUNTIME_DIR} = "$home/xdg_run";
my $sock = "$ENV{XDG_RUNTIME_DIR}/lei/5.seq.sock";
+ my $err_log = "$ENV{XDG_RUNTIME_DIR}/lei/errors.log";
ok($lei->('daemon-pid'), 'daemon-pid');
is($err, '', 'no error from daemon-pid');
@@ -267,10 +268,15 @@ SKIP: { # real socket
ok(-S $sock, 'sock created');
$test_lei_common->();
+ is(-s $err_log, 0, 'nothing in errors.log');
+ open my $efh, '>>', $err_log or BAIL_OUT $!;
+ print $efh "phail\n" or BAIL_OUT $!;
+ close $efh or BAIL_OUT $!;
ok($lei->('daemon-pid'), 'daemon-pid');
chomp(my $pid_again = $out);
is($pid, $pid_again, 'daemon-pid idempotent');
+ like($err, qr/phail/, 'got mock "phail" error previous run');
ok($lei->(qw(daemon-kill)), 'daemon-kill');
is($out, '', 'no output from daemon-kill');
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 6/7] lei_xsearch: keep l2m->{-wq_s1} while preparing query
2021-01-19 9:34 [PATCH 0/9] lei bugfixes and error handling Eric Wong
` (14 preceding siblings ...)
2021-01-20 5:04 ` [PATCH 5/7] lei: dump and clear errors.log in daemon mode Eric Wong
@ 2021-01-20 5:04 ` Eric Wong
2021-01-20 5:04 ` [PATCH 7/7] lei_to_mail: call PublicInbox::IPC::DESTROY Eric Wong
16 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-01-20 5:04 UTC (permalink / raw)
To: meta
This caused a performance regression which made parallel
lei2mail processes fail prematurely and fall back to
writing blobs in the lei_xsearch worker.
---
lib/PublicInbox/LeiXSearch.pm | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index fa37543f..a6d827de 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -273,9 +273,9 @@ sub do_query {
pipe(my ($startq, $au_done)) or die "pipe: $!";
$done_op->{'.'} = [ \&do_post_augment, $lei_orig,
$zpipe, $au_done ];
- $io[4] = *STDERR{GLOB}; # don't send l2m->{-wq_s1}
- $self->wq_do('query_prepare', \@io, $lei);
+ local $io[4] = *STDERR{GLOB}; # don't send l2m->{-wq_s1}
die "BUG: unexpected \$io[5]: $io[5]" if $io[5];
+ $self->wq_do('query_prepare', \@io, $lei);
fcntl($startq, 1031, 4096) if $^O eq 'linux'; # F_SETPIPE_SZ
$io[5] = $startq;
$io[1] = $zpipe->[1] if $zpipe;
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 7/7] lei_to_mail: call PublicInbox::IPC::DESTROY
2021-01-19 9:34 [PATCH 0/9] lei bugfixes and error handling Eric Wong
` (15 preceding siblings ...)
2021-01-20 5:04 ` [PATCH 6/7] lei_xsearch: keep l2m->{-wq_s1} while preparing query Eric Wong
@ 2021-01-20 5:04 ` Eric Wong
16 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-01-20 5:04 UTC (permalink / raw)
To: meta
It doesn't seem to matter at the moment, but it should
save us from some surprises down the line.
---
lib/PublicInbox/LeiToMail.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index 9d9b5748..1be0b09c 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -471,6 +471,7 @@ sub DESTROY {
for my $pid_git (grep(/\A$$\0/, keys %$self)) {
$self->{$pid_git}->async_wait_all;
}
+ $self->SUPER::DESTROY; # PublicInbox::IPC
}
1;
^ permalink raw reply related [flat|nested] 19+ messages in thread