* [PATCH 0/2] daemon shutdown improvements
@ 2023-09-25 10:17 Eric Wong
2023-09-25 10:17 ` [PATCH 1/2] ds: force event_loop wakeup on final child death Eric Wong
2023-09-25 10:17 ` [PATCH 2/2] tests: add quit_waiter_pipe and wait_for_eof Eric Wong
0 siblings, 2 replies; 3+ messages in thread
From: Eric Wong @ 2023-09-25 10:17 UTC (permalink / raw)
To: meta
More stuff extracted from another branch...
Eric Wong (2):
ds: force event_loop wakeup on final child death
tests: add quit_waiter_pipe and wait_for_eof
lib/PublicInbox/DS.pm | 9 ++++++---
lib/PublicInbox/LEI.pm | 1 -
lib/PublicInbox/TestCommon.pm | 36 +++++++++++++++++++++++++----------
lib/PublicInbox/XapHelper.pm | 1 -
t/httpd-unix.t | 35 ++++++++++------------------------
5 files changed, 42 insertions(+), 40 deletions(-)
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/2] ds: force event_loop wakeup on final child death
2023-09-25 10:17 [PATCH 0/2] daemon shutdown improvements Eric Wong
@ 2023-09-25 10:17 ` Eric Wong
2023-09-25 10:17 ` [PATCH 2/2] tests: add quit_waiter_pipe and wait_for_eof Eric Wong
1 sibling, 0 replies; 3+ messages in thread
From: Eric Wong @ 2023-09-25 10:17 UTC (permalink / raw)
To: meta
Reaping children needs to keep the event_loop spinning another
round when the @post_loop_do callback may be used to check
on process exit during shutdown.
This allows us to get rid of the hacky SetLoopTimeout calls in
lei-daemon and XapHelper.pm during process shutdown if we're
trying to wait for all PIDs to exit before leaving the event
loop.
---
lib/PublicInbox/DS.pm | 9 ++++++---
lib/PublicInbox/LEI.pm | 1 -
lib/PublicInbox/XapHelper.pm | 1 -
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 919a4b67..49550b2b 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -216,12 +216,15 @@ sub await_cb ($;@) {
sub reap_pids {
$reap_armed = undef;
while (1) {
- my $pid = waitpid(-1, WNOHANG) // last;
- last if $pid <= 0;
+ my $pid = waitpid(-1, WNOHANG) or return;
if (defined(my $cb_args = delete $AWAIT_PIDS->{$pid})) {
await_cb($pid, @$cb_args) if $cb_args;
- } else {
+ } elsif ($pid == -1 && $! == ECHILD) {
+ return requeue(\&dflush); # force @post_loop_do to run
+ } elsif ($pid > 0) {
warn "W: reaped unknown PID=$pid: \$?=$?\n";
+ } else { # does this happen?
+ return warn("W: waitpid(-1, WNOHANG) => $pid ($!)");
}
}
}
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index be77fa90..432ae61e 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -1316,7 +1316,6 @@ sub lazy_start {
# closing eof_p triggers \&noop wakeup
$listener = $eof_p = $pil = $path = undef;
$lis->close; # DS::close
- PublicInbox::DS->SetLoopTimeout(1000);
};
};
my $sig = {
diff --git a/lib/PublicInbox/XapHelper.pm b/lib/PublicInbox/XapHelper.pm
index e8eeb2dc..8c2b86d6 100644
--- a/lib/PublicInbox/XapHelper.pm
+++ b/lib/PublicInbox/XapHelper.pm
@@ -209,7 +209,6 @@ sub reap_worker { # awaitpid CB
delete $WORKERS{$nr};
if (($? >> 8) == 66) { # EX_NOINPUT
$alive = undef;
- PublicInbox::DS->SetLoopTimeout(1);
} elsif ($?) {
warn "worker[$nr] died \$?=$?\n";
}
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/2] tests: add quit_waiter_pipe and wait_for_eof
2023-09-25 10:17 [PATCH 0/2] daemon shutdown improvements Eric Wong
2023-09-25 10:17 ` [PATCH 1/2] ds: force event_loop wakeup on final child death Eric Wong
@ 2023-09-25 10:17 ` Eric Wong
1 sibling, 0 replies; 3+ messages in thread
From: Eric Wong @ 2023-09-25 10:17 UTC (permalink / raw)
To: meta
These generalize the idiom from t/httpd-unix.t and allows them
to be used for all the lei daemon shutdown in tests. This
speeds up the serialized run of `prove -lvw t/lei.t t/lei-daemon.t'
by roughly 8-9% or so since we're no longer sleeping and killing
the daemon in a loop.
Parallelized `make check' and `make check-run' don't show much
difference, yet.
Note that we do eliminate some kill(2) checks since those still
require retries an delays. We assume the kernel auto-closing
FDs on process exit is a strong-enough guarantee that the
process will soon be reaped by PID:1.
These will be useful for the FUSE daemons, as well.
We'll also start introducing more uses of autodie to simplify
our code.
---
lib/PublicInbox/TestCommon.pm | 36 +++++++++++++++++++++++++----------
t/httpd-unix.t | 35 ++++++++++------------------------
2 files changed, 36 insertions(+), 35 deletions(-)
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 1fdb5ebd..7b628769 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -24,6 +24,7 @@ BEGIN {
run_script start_script key2sub xsys xsys_e xqx eml_load tick
have_xapian_compact json_utf8 setup_public_inboxes create_inbox
create_coderepo no_scm_rights
+ quit_waiter_pipe wait_for_eof
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 no_pollerfd no_coredump cfg_new);
@@ -632,12 +633,28 @@ sub need_scm_rights () {
'(mkdir -p ~/.cache/public-inbox/inline-c) OR Socket::MsgHdr missing';
}
+# returns a pipe with FD_CLOEXEC disabled on the write-end
+sub quit_waiter_pipe () {
+ use autodie qw(fcntl pipe);
+ pipe(my $r, my $w);
+ fcntl($w, F_SETFD, fcntl($w, F_GETFD, 0) & ~FD_CLOEXEC);
+ ($r, $w);
+}
+
+sub wait_for_eof ($$;$) {
+ my ($io, $msg, $sec) = @_;
+ vec(my $rset = '', fileno($io), 1) = 1;
+ ok(select($rset, undef, undef, $sec // 9), "$msg (select)");
+ is(my $line = <$io>, undef, "$msg EOF");
+}
+
sub test_lei {
SKIP: {
my ($cb) = pop @_;
my $test_opt = shift // {};
local $lei_cwdfh;
- opendir $lei_cwdfh, '.' or xbail "opendir .: $!";
+ use autodie qw(mkdir open opendir);
+ opendir $lei_cwdfh, '.';
require_git(2.6, 1);
my $mods = $test_opt->{mods} // [ 'lei' ];
require_mods(@$mods, 2);
@@ -661,22 +678,25 @@ SKIP: {
my $tmpdir = $test_opt->{tmpdir};
File::Path::mkpath($tmpdir) if defined $tmpdir;
($tmpdir, $for_destroy) = tmpdir unless $tmpdir;
+ my ($dead_r, $dead_w);
state $persist_xrd = $ENV{TEST_LEI_DAEMON_PERSIST_DIR};
SKIP: {
$ENV{TEST_LEI_ONESHOT} and
xbail 'TEST_LEI_ONESHOT no longer supported';
my $home = "$tmpdir/lei-daemon";
- mkdir($home, 0700) or BAIL_OUT "mkdir: $!";
+ mkdir($home, 0700);
local $ENV{HOME} = $home;
my $persist;
if ($persist_xrd && !$test_opt->{daemon_only}) {
$persist = $daemon_xrd = $persist_xrd;
} else {
$daemon_xrd = "$home/xdg_run";
- mkdir($daemon_xrd, 0700) or BAIL_OUT "mkdir: $!";
+ mkdir($daemon_xrd, 0700);
+ ($dead_r, $dead_w) = quit_waiter_pipe;
}
local $ENV{XDG_RUNTIME_DIR} = $daemon_xrd;
- $cb->();
+ $cb->(); # likely shares $dead_w with lei-daemon
+ undef $dead_w; # so select() wakes up when daemon dies
if ($persist) { # remove before ~/.local gets removed
File::Path::rmtree([glob("$home/*")]);
File::Path::rmtree("$home/.config");
@@ -693,14 +713,10 @@ SKIP: {
}
}; # SKIP for lei_daemon
if ($daemon_pid) {
- for (0..10) {
- kill(0, $daemon_pid) or last;
- tick;
- }
- ok(!kill(0, $daemon_pid), "$t daemon stopped");
+ wait_for_eof($dead_r, 'daemon quit pipe');
no_coredump $tmpdir;
my $f = "$daemon_xrd/lei/errors.log";
- open my $fh, '<', $f or BAIL_OUT "$f: $!";
+ open my $fh, '<', $f;
my @l = <$fh>;
is_xdeeply(\@l, [],
"$t daemon XDG_RUNTIME_DIR/lei/errors.log empty");
diff --git a/t/httpd-unix.t b/t/httpd-unix.t
index 95f589ad..0b620bd6 100644
--- a/t/httpd-unix.t
+++ b/t/httpd-unix.t
@@ -7,7 +7,7 @@ use PublicInbox::TestCommon;
use Errno qw(EADDRINUSE);
use Cwd qw(abs_path);
use Carp qw(croak);
-use Fcntl qw(FD_CLOEXEC F_SETFD);
+use autodie qw(close);
require_mods(qw(Plack::Util Plack::Builder HTTP::Date HTTP::Status));
use IO::Socket::UNIX;
use POSIX qw(mkfifo);
@@ -125,11 +125,10 @@ SKIP: {
};
for my $w (qw(-W0 -W1)) {
- pipe(my ($p0, $p1)) or xbail "pipe: $!";
- fcntl($p1, F_SETFD, 0) or xbail "fcntl: $!"; # clear FD_CLOEXEC
+ my ($p0, $p1) = quit_waiter_pipe;
# wait for daemonization
$spawn_httpd->("-l$unix", '-D', '-P', $pid_file, $w);
- close $p1 or xbail "close: $!";
+ close $p1;
$td->join;
is($?, 0, "daemonized $w process");
check_sock($unix);
@@ -137,13 +136,9 @@ SKIP: {
my $pid = $read_pid->($pid_file);
no_pollerfd($pid) if $w eq '-W1';
is(kill('TERM', $pid), 1, "signaled daemonized $w process");
- vec(my $rvec = '', fileno($p0), 1) = 1;
delete $td->{-extra}; # drop tail(1) process
- is(select($rvec, undef, undef, 1), 1, 'timeout for pipe HUP');
- is(my $undef = <$p0>, undef, 'process closed pipe writer at exit');
+ wait_for_eof($p0, "httpd $w quit pipe");
ok(!-e $pid_file, "$w pid file unlinked at exit");
- delay_until(sub { !kill(0, $pid) },
- "daemonized $w really not running");
}
my $httpd = abs_path('blib/script/public-inbox-httpd');
@@ -152,11 +147,9 @@ SKIP: {
my @args = ("-l$unix", '-D', '-P', $pid_file, -1, $out, -2, $err);
if ('USR2 upgrades with workers') {
- pipe(my ($p0, $p1)) or xbail "pipe: $!";
- fcntl($p1, F_SETFD, 0) or xbail "fcntl: $!"; # clear FD_CLOEXEC
-
+ my ($p0, $p1) = quit_waiter_pipe;
$td = start_script([$httpd, @args, $psgi], undef, $opt);
- close($p1) or xbail "close: $!";
+ close $p1;
$td->join;
is($?, 0, "daemonized process again");
check_sock($unix);
@@ -211,19 +204,14 @@ SKIP: {
check_sock($unix);
kill('QUIT', $new_pid) or die "QUIT failed: $!";
- vec(my $rvec = '', fileno($p0), 1) = 1;
- is(select($rvec, undef, undef, 1), 1, 'timeout for pipe HUP');
- is(my $u = <$p0>, undef, 'process closed pipe writer at exit');
-
+ wait_for_eof($p0, 'new process');
ok(!-f $pid_file, 'PID file is gone');
- delay_until(sub { !kill(0, $new_pid) }, 'new PID really died');
}
if ('try USR2 without workers (-W0)') {
- pipe(my ($p0, $p1)) or xbail "pipe: $!";
- fcntl($p1, F_SETFD, 0) or xbail "fcntl: $!"; # clear FD_CLOEXEC
+ my ($p0, $p1) = quit_waiter_pipe;
$td = start_script([$httpd, @args, '-W0', $psgi], undef, $opt);
- close $p1 or xbail "close: $!";
+ close $p1;
$td->join;
is($?, 0, 'daemonized w/o workers');
$register_exit_fifo->($unix, $f1);
@@ -238,11 +226,8 @@ SKIP: {
$pid = $read_pid->($pid_file);
kill('QUIT', $pid) or xbail "USR2 failed: $!";
- vec(my $rvec = '', fileno($p0), 1) = 1;
- is(select($rvec, undef, undef, 1), 1, 'timeout for pipe HUP');
- is(my $u = <$p0>, undef, 'process closed pipe writer at exit');
+ wait_for_eof($p0, '-W0 USR2 test pipe');
ok(!-f $pid_file, 'PID file is gone');
- delay_until(sub { !kill(0, $pid) }, '-W0 daemon is gone');
}
}
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-09-25 10:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-25 10:17 [PATCH 0/2] daemon shutdown improvements Eric Wong
2023-09-25 10:17 ` [PATCH 1/2] ds: force event_loop wakeup on final child death Eric Wong
2023-09-25 10:17 ` [PATCH 2/2] tests: add quit_waiter_pipe and wait_for_eof 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).