user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH 2/2] t/httpd-unix: eliminate some busy waits
Date: Mon, 12 Dec 2022 04:22:01 +0000	[thread overview]
Message-ID: <20221212042201.510414-3-e@80x24.org> (raw)
In-Reply-To: <20221212042201.510414-1-e@80x24.org>

A small step towards making our test suite as sleep-less as
possible.  We can use FIFOs to coordinate processes in a few
places, while other spots can take advantage of disabling
FD_CLOEXEC to further eliminate back-and-forth traffic between
processes.

This speeds up t/httpd-unix.t by ~20 ms on my system.
---
 t/httpd-corner.psgi |  19 ++++-
 t/httpd-unix.t      | 202 ++++++++++++++++++++++++++------------------
 2 files changed, 138 insertions(+), 83 deletions(-)

diff --git a/t/httpd-corner.psgi b/t/httpd-corner.psgi
index 10cf8350..ea643024 100644
--- a/t/httpd-corner.psgi
+++ b/t/httpd-corner.psgi
@@ -3,9 +3,22 @@
 # corner case tests for the generic PSGI server
 # Usage: plackup [OPTIONS] /path/to/this/file
 use v5.12;
-use warnings;
 use Plack::Builder;
 require Digest::SHA;
+if (defined(my $f = $ENV{TEST_OPEN_FIFO})) {
+	open my $fh, '>', $f or die "open($f): $!";
+	say $fh 'hi';
+	close $fh;
+}
+
+END {
+	if (defined(my $f = $ENV{TEST_EXIT_FIFO})) {
+		open my $fh, '>', $f or die "open($f): $!";
+		say $fh "bye from $$";
+		close $fh;
+	}
+}
+
 my $pi_config = $ENV{PI_CONFIG} // 'unset'; # capture ASAP
 my $app = sub {
 	my ($env) = @_;
@@ -118,6 +131,10 @@ my $app = sub {
 	} elsif ($path eq '/PI_CONFIG') {
 		$code = 200;
 		push @$body, $pi_config; # show value at ->refresh_groups
+	} elsif ($path =~ m!\A/exit-fifo(.+)\z!) {
+		$code = 200;
+		$ENV{TEST_EXIT_FIFO} = $1; # for END {}
+		push @$body, "fifo $1 registered";
 	}
 	[ $code, $h, $body ]
 };
diff --git a/t/httpd-unix.t b/t/httpd-unix.t
index e915ea2c..c45ff163 100644
--- a/t/httpd-unix.t
+++ b/t/httpd-unix.t
@@ -8,8 +8,10 @@ use PublicInbox::TestCommon;
 use Errno qw(EADDRINUSE);
 use Cwd qw(abs_path);
 use Carp qw(croak);
+use Fcntl qw(FD_CLOEXEC F_SETFD);
 require_mods(qw(Plack::Util Plack::Builder HTTP::Date HTTP::Status));
 use IO::Socket::UNIX;
+use POSIX qw(mkfifo);
 my ($tmpdir, $for_destroy) = tmpdir();
 my $unix = "$tmpdir/unix.sock";
 my $psgi = './t/httpd-corner.psgi';
@@ -17,6 +19,15 @@ my $out = "$tmpdir/out.log";
 my $err = "$tmpdir/err.log";
 my $td;
 
+my $register_exit_fifo = sub {
+	my ($s, $f) = @_;
+	my $sock = new_sock($s);
+	ok($sock->write("GET /exit-fifo$f HTTP/1.0\r\n\r\n"),
+		'request exit-fifo');
+	ok($sock->read(my $buf, 4096), 'read exit-fifo response');
+	like($buf, qr!\r\n\r\nfifo \Q$f\E registered\z!, 'set exit fifo');
+};
+
 my $spawn_httpd = sub {
 	my (@args) = @_;
 	my $cmd = [ '-httpd', @args, "--stdout=$out", "--stderr=$err", $psgi ];
@@ -32,18 +43,24 @@ my $spawn_httpd = sub {
 }
 
 ok(!-S $unix, 'UNIX socket does not exist, yet');
-$spawn_httpd->("-l$unix", '-W0');
-my %o = (Peer => $unix, Type => SOCK_STREAM);
-for (1..1000) {
-	last if -S $unix && IO::Socket::UNIX->new(%o);
-	tick(0.02);
+my $f1 = "$tmpdir/f1";
+mkfifo($f1, 0600);
+{
+	local $ENV{TEST_OPEN_FIFO} = $f1;
+	$spawn_httpd->("-l$unix", '-W0');
+	open my $fh, '<', $f1 or xbail "open($f1): $!";
+	is(my $hi = <$fh>, "hi\n", 'got FIFO greeting');
 }
-
 ok(-S $unix, 'UNIX socket was bound by -httpd');
+
+sub new_sock ($) {
+	IO::Socket::UNIX->new(Peer => $_[0], Type => SOCK_STREAM)
+		// xbail "E: $! connecting to $_[0]";
+}
+
 sub check_sock ($) {
 	my ($unix) = @_;
-	my $sock = IO::Socket::UNIX->new(Peer => $unix, Type => SOCK_STREAM)
-		// BAIL_OUT "E: $! connecting to $unix";
+	my $sock = new_sock($unix);
 	ok($sock->write("GET /host-port HTTP/1.0\r\n\r\n"),
 		'wrote req to server');
 	ok($sock->read(my $buf, 4096), 'read response');
@@ -107,95 +124,116 @@ 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
 		# wait for daemonization
 		$spawn_httpd->("-l$unix", '-D', '-P', $pid_file, $w);
+		close $p1 or xbail "close: $!";
 		$td->join;
 		is($?, 0, "daemonized $w process");
 		check_sock($unix);
 		ok(-s $pid_file, "$w pid file written");
 		my $pid = $read_pid->($pid_file);
 		is(kill('TERM', $pid), 1, "signaled daemonized $w process");
-		delay_until(sub { !kill(0, $pid) });
-		is(kill(0, $pid), 0, "daemonized $w process exited");
+		vec(my $rvec = '', fileno($p0), 1) = 1;
+		is(select($rvec, undef, undef, 1), 1, 'timeout for pipe HUP');
+		is(my $undef = <$p0>, undef, 'process closed pipe writer at exit');
 		ok(!-e $pid_file, "$w pid file unlinked at exit");
 	}
 
-	# try a USR2 upgrade with workers:
 	my $httpd = abs_path('blib/script/public-inbox-httpd');
 	$psgi = abs_path($psgi);
 	my $opt = { run_mode => 0 };
-
 	my @args = ("-l$unix", '-D', '-P', $pid_file, -1, $out, -2, $err);
-	$td = start_script([$httpd, @args, $psgi], undef, $opt);
-	$td->join;
-	is($?, 0, "daemonized process again");
-	check_sock($unix);
-	ok(-s $pid_file, 'pid file written');
-	my $pid = $read_pid->($pid_file);
-
-	# stop worker to ensure check_sock below hits $new_pid
-	kill('TTOU', $pid) or die "TTOU failed: $!";
-
-	kill('USR2', $pid) or die "USR2 failed: $!";
-	delay_until(sub {
-		$pid != (eval { $read_pid->($pid_file) } // $pid)
-	});
-	my $new_pid = $read_pid->($pid_file);
-	isnt($new_pid, $pid, 'new child started');
-	ok($new_pid > 0, '$new_pid valid');
-	delay_until(sub { -s "$pid_file.oldbin" });
-	my $old_pid = $read_pid->("$pid_file.oldbin");
-	is($old_pid, $pid, '.oldbin pid file written');
-	ok($old_pid > 0, '$old_pid valid');
-
-	check_sock($unix); # ensures $new_pid is ready to receive signals
-
-	# first, back out of the upgrade
-	kill('QUIT', $new_pid) or die "kill new PID failed: $!";
-	delay_until(sub {
-		$pid == (eval { $read_pid->($pid_file) } // 0)
-	});
-	is($read_pid->($pid_file), $pid, 'old PID file restored');
-	ok(!-f "$pid_file.oldbin", '.oldbin PID file gone');
-
-	# retry USR2 upgrade
-	kill('USR2', $pid) or die "USR2 failed: $!";
-	delay_until(sub {
-		$pid != (eval { $read_pid->($pid_file) } // $pid)
-	});
-	$new_pid = $read_pid->($pid_file);
-	isnt($new_pid, $pid, 'new child started again');
-	$old_pid = $read_pid->("$pid_file.oldbin");
-	is($old_pid, $pid, '.oldbin pid file written');
-
-	# drop the old parent
-	kill('QUIT', $old_pid) or die "QUIT failed: $!";
-	delay_until(sub { !kill(0, $old_pid) });
-	ok(!-f "$pid_file.oldbin", '.oldbin PID file gone');
-
-	# drop the new child
-	check_sock($unix);
-	kill('QUIT', $new_pid) or die "QUIT failed: $!";
-	delay_until(sub { !kill(0, $new_pid) });
-	ok(!-f $pid_file, 'PID file is gone');
-
-
-	# try USR2 without workers (-W0)
-	$td = start_script([$httpd, @args, '-W0', $psgi], undef, $opt);
-	$td->join;
-	is($?, 0, 'daemonized w/o workers');
-	check_sock($unix);
-	$pid = $read_pid->($pid_file);
-
-	# replace running process
-	kill('USR2', $pid) or die "USR2 failed: $!";
-	delay_until(sub { !kill(0, $pid) });
-
-	check_sock($unix);
-	$pid = $read_pid->($pid_file);
-	kill('QUIT', $pid) or die "USR2 failed: $!";
-	delay_until(sub { !kill(0, $pid) });
-	ok(!-f $pid_file, 'PID file is gone');
+
+	if ('USR2 upgrades with workers') {
+		pipe(my ($p0, $p1)) or xbail "pipe: $!";
+		fcntl($p1, F_SETFD, 0) or xbail "fcntl: $!"; # clear FD_CLOEXEC
+
+		$td = start_script([$httpd, @args, $psgi], undef, $opt);
+		close($p1) or xbail "close: $!";
+		$td->join;
+		is($?, 0, "daemonized process again");
+		check_sock($unix);
+		ok(-s $pid_file, 'pid file written');
+		my $pid = $read_pid->($pid_file);
+
+		# stop worker to ensure check_sock below hits $new_pid
+		kill('TTOU', $pid) or die "TTOU failed: $!";
+
+		kill('USR2', $pid) or die "USR2 failed: $!";
+		delay_until(sub {
+			$pid != (eval { $read_pid->($pid_file) } // $pid)
+		});
+		my $new_pid = $read_pid->($pid_file);
+		isnt($new_pid, $pid, 'new child started');
+		ok($new_pid > 0, '$new_pid valid');
+		delay_until(sub { -s "$pid_file.oldbin" });
+		my $old_pid = $read_pid->("$pid_file.oldbin");
+		is($old_pid, $pid, '.oldbin pid file written');
+		ok($old_pid > 0, '$old_pid valid');
+
+		check_sock($unix); # ensures $new_pid is ready to receive signals
+
+		# first, back out of the upgrade
+		kill('QUIT', $new_pid) or die "kill new PID failed: $!";
+		delay_until(sub {
+			$pid == (eval { $read_pid->($pid_file) } // 0)
+		});
+		is($read_pid->($pid_file), $pid, 'old PID file restored');
+		ok(!-f "$pid_file.oldbin", '.oldbin PID file gone');
+
+		# retry USR2 upgrade
+		kill('USR2', $pid) or die "USR2 failed: $!";
+		delay_until(sub {
+			$pid != (eval { $read_pid->($pid_file) } // $pid)
+		});
+		$new_pid = $read_pid->($pid_file);
+		isnt($new_pid, $pid, 'new child started again');
+		$old_pid = $read_pid->("$pid_file.oldbin");
+		is($old_pid, $pid, '.oldbin pid file written');
+
+		# drop the old parent
+		kill('QUIT', $old_pid) or die "QUIT failed: $!";
+		delay_until(sub { !kill(0, $old_pid) }); # UGH
+
+		ok(!-f "$pid_file.oldbin", '.oldbin PID file gone');
+
+		# drop the new child
+		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');
+
+		ok(!-f $pid_file, 'PID file is gone');
+	}
+
+	if ('try USR2 without workers (-W0)') {
+		pipe(my ($p0, $p1)) or xbail "pipe: $!";
+		fcntl($p1, F_SETFD, 0) or xbail "fcntl: $!"; # clear FD_CLOEXEC
+		$td = start_script([$httpd, @args, '-W0', $psgi], undef, $opt);
+		close $p1 or xbail "close: $!";
+		$td->join;
+		is($?, 0, 'daemonized w/o workers');
+		$register_exit_fifo->($unix, $f1);
+		my $pid = $read_pid->($pid_file);
+
+		# replace running process
+		kill('USR2', $pid) or xbail "USR2 failed: $!";
+		open my $fh, '<', $f1 or xbail "open($f1): $!";
+		is(my $bye = <$fh>, "bye from $pid\n", 'got FIFO bye');
+
+		check_sock($unix);
+		$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');
+		ok(!-f $pid_file, 'PID file is gone');
+	}
 }
 
 done_testing();

      parent reply	other threads:[~2022-12-12  4:22 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-12  4:21 [PATCH 0/2] some sleep elimination Eric Wong
2022-12-12  4:22 ` [PATCH 1/2] tests: replace select/usleep calls with tick() Eric Wong
2022-12-12  4:22 ` Eric Wong [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://public-inbox.org/README

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221212042201.510414-3-e@80x24.org \
    --to=e@80x24.org \
    --cc=meta@public-inbox.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

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