unofficial mirror of meta@public-inbox.org
 help / color / mirror / Atom feed
* [PATCH 00/16] some yak-shaving and annoyance fixes
@ 2021-10-16  1:00 Eric Wong
  2021-10-16  1:00 ` [PATCH 01/12] smsg: add ->oidbin method Eric Wong
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: Eric Wong @ 2021-10-16  1:00 UTC (permalink / raw)
  To: meta

Hopefully having less code will make bug-hunting easier and
and more robust in the future at handling failures and
unexpected interrupts (e.g. Ctrl-C).

There's a lot of YAGNI elimination and more to come.

Eric Wong (12):
  smsg: add ->oidbin method
  dir_idle: do not add watches in ->new
  imapd+nntpd: drop timer-based expiration
  httpd: move pipeline logic into event_step
  lei: golf PATH2CFG cleanup
  lei: always keep cwd fd {3} for ->fchdir
  lei: more eval guards for die on failure
  extindex: prune invalid alternate entries on --gc
  lei_overview: die rather than lei->fail
  lei_to_mail: quiet down abort messages
  inbox + search: use 5.10.1 and do some golfing
  httpd/async: switch to level-triggered epoll

 Documentation/technical/ds.txt  |  3 +-
 lib/PublicInbox/DS.pm           | 37 +------------------
 lib/PublicInbox/Daemon.pm       |  8 ++---
 lib/PublicInbox/DirIdle.pm      |  8 +----
 lib/PublicInbox/ExtSearch.pm    |  2 +-
 lib/PublicInbox/ExtSearchIdx.pm | 19 +++++-----
 lib/PublicInbox/HTTP.pm         | 64 +++++++++------------------------
 lib/PublicInbox/HTTPD/Async.pm  | 16 +++------
 lib/PublicInbox/IMAP.pm         | 17 +++------
 lib/PublicInbox/Import.pm       |  2 +-
 lib/PublicInbox/Inbox.pm        | 11 +++---
 lib/PublicInbox/LEI.pm          | 29 +++++++--------
 lib/PublicInbox/LeiLcat.pm      | 21 +++++------
 lib/PublicInbox/LeiOverview.pm  | 24 ++++++-------
 lib/PublicInbox/LeiQuery.pm     | 24 ++++++-------
 lib/PublicInbox/LeiToMail.pm    |  1 +
 lib/PublicInbox/LeiXSearch.pm   |  9 ++---
 lib/PublicInbox/MultiGit.pm     |  6 +++-
 lib/PublicInbox/NNTP.pm         | 12 ++-----
 lib/PublicInbox/OverIdx.pm      |  3 +-
 lib/PublicInbox/Qspawn.pm       |  1 -
 lib/PublicInbox/Search.pm       |  7 ++--
 lib/PublicInbox/Smsg.pm         |  6 ++--
 lib/PublicInbox/Watch.pm        |  3 +-
 t/dir_idle.t                    |  3 +-
 25 files changed, 118 insertions(+), 218 deletions(-)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 01/12] smsg: add ->oidbin method
  2021-10-16  1:00 [PATCH 00/16] some yak-shaving and annoyance fixes Eric Wong
@ 2021-10-16  1:00 ` Eric Wong
  2021-10-16  1:00 ` [PATCH 02/12] dir_idle: do not add watches in ->new Eric Wong
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2021-10-16  1:00 UTC (permalink / raw)
  To: meta

This makes some of our code less noisy by reducing the
amount of pack('H*', ...) use.
---
 lib/PublicInbox/ExtSearch.pm    |  2 +-
 lib/PublicInbox/ExtSearchIdx.pm | 11 +++++------
 lib/PublicInbox/Import.pm       |  2 +-
 lib/PublicInbox/OverIdx.pm      |  3 +--
 lib/PublicInbox/Smsg.pm         |  6 ++++--
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/lib/PublicInbox/ExtSearch.pm b/lib/PublicInbox/ExtSearch.pm
index 7520e71c3c12..2460d74f5bdb 100644
--- a/lib/PublicInbox/ExtSearch.pm
+++ b/lib/PublicInbox/ExtSearch.pm
@@ -64,7 +64,7 @@ SELECT ibx_id FROM inboxes WHERE eidx_key = ? LIMIT 1
 	$sth = $dbh->prepare_cached(<<'', undef, 1);
 SELECT docid FROM xref3 WHERE oidbin = ? AND xnum = ? AND ibx_id = ? LIMIT 1
 
-	$sth->bind_param(1, pack('H*', $xsmsg->{blob}), SQL_BLOB);
+	$sth->bind_param(1, $xsmsg->oidbin, SQL_BLOB);
 
 	# NNTP::cmd_over can set {num} to zero according to RFC 3977 8.3.2
 	$sth->bind_param(2, $xsmsg->{num} || $xsmsg->{-orig_num});
diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index 3d7a6e7d7622..ba7588d53254 100644
--- a/lib/PublicInbox/ExtSearchIdx.pm
+++ b/lib/PublicInbox/ExtSearchIdx.pm
@@ -122,7 +122,7 @@ sub apply_boost ($$) {
 		$a->[1] <=> $b->[1] # break ties with {xnum}
 	} @$xr3;
 	my $new_smsg = $req->{new_smsg};
-	return if $xr3->[0]->[2] ne pack('H*', $new_smsg->{blob}); # loser
+	return if $xr3->[0]->[2] ne $new_smsg->oidbin; # loser
 
 	# replace the old smsg with the more boosted one
 	$new_smsg->{num} = $smsg->{num};
@@ -146,7 +146,7 @@ sub _unref_doc ($$$$$;$) {
 	}
 	if (defined($oidbin) && defined($xnum) && blessed($ibx) && $ibx->over) {
 		my $smsg = $ibx->over->get_art($xnum);
-		if ($smsg && pack('H*', $smsg->{blob}) eq $oidbin) {
+		if ($smsg && $smsg->oidbin eq $oidbin) {
 			carp("BUG: (non-fatal) ".$ibx->eidx_key.
 				" #$xnum $smsg->{blob} still valid");
 			return;
@@ -270,9 +270,8 @@ sub _blob_missing ($$) { # called when a known $smsg->{blob} is gone
 	my ($req, $smsg) = @_;
 	# xnum and ibx are unknown, we only call this when an entry from
 	# /ei*/over.sqlite3 is bad, not on entries from xap*/over.sqlite3
-	my $oidbin = pack('H*', $smsg->{blob});
 	$req->{self}->git->async_wait_all;
-	_unref_doc($req, $smsg, undef, undef, $oidbin);
+	_unref_doc($req, $smsg, undef, undef, $smsg->oidbin);
 }
 
 sub ck_existing { # git->cat_async callback
@@ -569,7 +568,7 @@ sub _reindex_finalize ($$$) {
 	for my $ary (values %$by_chash) {
 		for my $x (reverse @$ary) {
 			warn "removing #$docid xref3 $x->{blob}\n";
-			my $bin = pack('H*', $x->{blob});
+			my $bin = $x->oidbin;
 			my $n = _unref_doc($sync, $docid, undef, undef, $bin);
 			die "BUG: $x->{blob} invalidated #$docid" if $n == 0;
 		}
@@ -987,7 +986,7 @@ sub dd_smsg { # git->cat_async callback
 		my $oidx = $self->{oidx};
 		for my $smsg (@$ary) {
 			my $gone = $smsg->{num};
-			$oidx->merge_xref3($keep->{num}, $gone, $smsg->{blob});
+			$oidx->merge_xref3($keep->{num}, $gone, $smsg->oidbin);
 			remove_doc($self, $gone);
 		}
 	}
diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index 17adfabd0d25..60ce7b66419b 100644
--- a/lib/PublicInbox/Import.pm
+++ b/lib/PublicInbox/Import.pm
@@ -423,7 +423,7 @@ sub add {
 				$x->dbh;
 				$x;
 			};
-			return if !$u->set_maybe(pack('H*', $smsg->{blob}), 1);
+			return if !$u->set_maybe($smsg->oidbin, 1);
 			return if (!$oidx->vivify_xvmd($smsg) &&
 					$eidx_git->check($smsg->{blob}));
 		}
diff --git a/lib/PublicInbox/OverIdx.pm b/lib/PublicInbox/OverIdx.pm
index 9fdb26c0d5c2..e7c96e143749 100644
--- a/lib/PublicInbox/OverIdx.pm
+++ b/lib/PublicInbox/OverIdx.pm
@@ -620,8 +620,7 @@ UPDATE over SET ddd = ? WHERE num = ?
 }
 
 sub merge_xref3 { # used for "-extindex --dedupe"
-	my ($self, $keep_docid, $drop_docid, $oidhex) = @_;
-	my $oidbin = pack('H*', $oidhex);
+	my ($self, $keep_docid, $drop_docid, $oidbin) = @_;
 	my $sth = $self->{dbh}->prepare_cached(<<'');
 UPDATE OR IGNORE xref3 SET docid = ? WHERE docid = ? AND oidbin = ?
 
diff --git a/lib/PublicInbox/Smsg.pm b/lib/PublicInbox/Smsg.pm
index a2f54507425f..260ce6bb065e 100644
--- a/lib/PublicInbox/Smsg.pm
+++ b/lib/PublicInbox/Smsg.pm
@@ -9,13 +9,15 @@
 # large threads in our WWW UI and the NNTP range responses.
 package PublicInbox::Smsg;
 use strict;
-use warnings;
-use base qw(Exporter);
+use v5.10.1;
+use parent qw(Exporter);
 our @EXPORT_OK = qw(subject_normalized);
 use PublicInbox::MID qw(mids references);
 use PublicInbox::Address;
 use PublicInbox::MsgTime qw(msg_timestamp msg_datestamp);
 
+sub oidbin { pack('H*', $_[0]->{blob}) }
+
 sub to_doc_data {
 	my ($self) = @_;
 	join("\n",

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 02/12] dir_idle: do not add watches in ->new
  2021-10-16  1:00 [PATCH 00/16] some yak-shaving and annoyance fixes Eric Wong
  2021-10-16  1:00 ` [PATCH 01/12] smsg: add ->oidbin method Eric Wong
@ 2021-10-16  1:00 ` Eric Wong
  2021-10-16  1:00 ` [PATCH 03/12] imapd+nntpd: drop timer-based expiration Eric Wong
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2021-10-16  1:00 UTC (permalink / raw)
  To: meta

There's no savings in having two ways to add watches to an
inotify nor kqueue descriptor.
---
 lib/PublicInbox/DirIdle.pm | 8 +-------
 lib/PublicInbox/LEI.pm     | 5 +++--
 lib/PublicInbox/Watch.pm   | 3 ++-
 t/dir_idle.t               | 3 ++-
 4 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/lib/PublicInbox/DirIdle.pm b/lib/PublicInbox/DirIdle.pm
index c9a293e9355a..270d3829bc3e 100644
--- a/lib/PublicInbox/DirIdle.pm
+++ b/lib/PublicInbox/DirIdle.pm
@@ -32,7 +32,7 @@ if ($^O eq 'linux' && eval { require Linux::Inotify2; 1 }) {
 }
 
 sub new {
-	my ($class, $dirs, $cb, $gone) = @_;
+	my ($class, $cb) = @_;
 	my $self = bless { cb => $cb }, $class;
 	my $inot;
 	if ($ino_cls) {
@@ -43,13 +43,7 @@ sub new {
 		require PublicInbox::FakeInotify;
 		$inot = PublicInbox::FakeInotify->new; # starts timer
 	}
-
-	# Linux::Inotify2->watch or similar
-	my $fl = $MAIL_IN;
-	$fl |= $MAIL_GONE if $gone;
-	$inot->watch($_, $fl) for @$dirs;
 	$self->{inot} = $inot;
-	PublicInbox::FakeInotify::poll_once($self) if !$ino_cls;
 	$self;
 }
 
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 7dfd33989186..a526a91f8035 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -1305,10 +1305,11 @@ sub lazy_start {
 		USR2 => \&noop,
 	};
 	require PublicInbox::DirIdle;
-	local $dir_idle = PublicInbox::DirIdle->new([$sock_dir], sub {
+	local $dir_idle = PublicInbox::DirIdle->new(sub {
 		# just rely on wakeup to hit PostLoopCallback set below
 		dir_idle_handler($_[0]) if $_[0]->fullname ne $path;
-	}, 1);
+	});
+	$dir_idle->add_watches([$sock_dir]);
 	PublicInbox::DS->SetPostLoopCallback(sub {
 		my ($dmap, undef) = @_;
 		if (@st = defined($path) ? stat($path) : ()) {
diff --git a/lib/PublicInbox/Watch.pm b/lib/PublicInbox/Watch.pm
index c6bebce32edb..b48d9cccc3e2 100644
--- a/lib/PublicInbox/Watch.pm
+++ b/lib/PublicInbox/Watch.pm
@@ -282,7 +282,8 @@ sub watch_fs_init ($) {
 	};
 	require PublicInbox::DirIdle;
 	# inotify_create + EPOLL_CTL_ADD
-	PublicInbox::DirIdle->new([keys %{$self->{mdmap}}], $cb);
+	my $dir_idle = PublicInbox::DirIdle->new($cb);
+	$dir_idle->add_watches([keys %{$self->{mdmap}}]);
 }
 
 sub net_cb { # NetReader::(nntp|imap)_each callback
diff --git a/t/dir_idle.t b/t/dir_idle.t
index 8e7f3b70eec4..19e54967bf80 100644
--- a/t/dir_idle.t
+++ b/t/dir_idle.t
@@ -9,7 +9,8 @@ my ($tmpdir, $for_destroy) = tmpdir();
 make_path("$tmpdir/a/b", "$tmpdir/c");
 my @x;
 my $cb = sub { push @x, \@_ };
-my $di = PublicInbox::DirIdle->new(["$tmpdir/a", "$tmpdir/c"], $cb, 1);
+my $di = PublicInbox::DirIdle->new($cb);
+$di->add_watches(["$tmpdir/a", "$tmpdir/c"], 1);
 PublicInbox::DS->SetLoopTimeout(1000);
 my $end = 3 + now;
 PublicInbox::DS->SetPostLoopCallback(sub { scalar(@x) == 0 && now < $end });

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 03/12] imapd+nntpd: drop timer-based expiration
  2021-10-16  1:00 [PATCH 00/16] some yak-shaving and annoyance fixes Eric Wong
  2021-10-16  1:00 ` [PATCH 01/12] smsg: add ->oidbin method Eric Wong
  2021-10-16  1:00 ` [PATCH 02/12] dir_idle: do not add watches in ->new Eric Wong
@ 2021-10-16  1:00 ` Eric Wong
  2021-10-16  1:00 ` [PATCH 04/12] httpd: move pipeline logic into event_step Eric Wong
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2021-10-16  1:00 UTC (permalink / raw)
  To: meta

It's needlessly complex and O(n), so it doesn't scale well to a
high number of clients nor is it easy-to-scale with the data
structures available to us in pure Perl.

In any case, I see no evidence of either -imapd nor -nntpd
experiencing high connection loads on public-facing sites.
-httpd has never had its own timer-based expiration, either.

Fwiw, public-inbox.org itself has been running a public-facing
HTTP/HTTPS server with no userspace idle client expiration for
the past 8 years or with no ill effect.  Clients can come and go
as they wish, and SO_KEEPALIVE takes care of truly broken
connections if they're gone for ~2 hours.

Internet connections drop all time, so it should be harmless to
drop connections w/o warning since both NNTP and IMAP protocols
have well-defined semantics for determining if a message was
truncated (as does HTTP/1.1+).
---
 lib/PublicInbox/DS.pm     | 37 +------------------------------------
 lib/PublicInbox/Daemon.pm |  8 +++-----
 lib/PublicInbox/HTTP.pm   |  5 ++---
 lib/PublicInbox/IMAP.pm   | 17 ++++-------------
 lib/PublicInbox/NNTP.pm   | 12 +++---------
 5 files changed, 13 insertions(+), 66 deletions(-)

diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 9cca02d7ffe9..bf8c4466218a 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -37,9 +37,7 @@ our @EXPORT_OK = qw(now msg_more dwaitpid add_timer add_uniq_timer);
 my %Stack;
 my $nextq; # queue for next_tick
 my $wait_pids; # list of [ pid, callback, callback_arg ]
-my $EXPMAP; # fd -> idle_time
-our $EXPTIME = 180; # 3 minutes
-my ($reap_armed);
+my $reap_armed;
 my $ToClose; # sockets to close when event loop is done
 our (
      %DescriptorMap,             # fd (num) -> PublicInbox::DS object
@@ -76,7 +74,6 @@ sub Reset {
 		# we may be iterating inside one of these on our stack
 		my @q = delete @Stack{keys %Stack};
 		for my $q (@q) { @$q = () }
-		$EXPMAP = undef;
 		$wait_pids = $nextq = $ToClose = undef;
 		$ep_io = undef; # closes real $Epoll FD
 		$Epoll = undef; # may call DSKQXS::DESTROY
@@ -251,9 +248,6 @@ sub PostEventLoop () {
 		$ToClose = undef; # will be autovivified on push
 		@$close_now = map { fileno($_) } @$close_now;
 
-		# order matters, destroy expiry times, first:
-		delete @$EXPMAP{@$close_now};
-
 		# ->DESTROY methods may populate ToClose
 		delete @DescriptorMap{@$close_now};
 	}
@@ -655,35 +649,6 @@ sub dwaitpid ($;$$) {
 	}
 }
 
-sub expire_old () {
-	my $cur = $EXPMAP or return;
-	$EXPMAP = undef;
-	my $old = now() - $EXPTIME;
-	while (my ($fd, $idle_at) = each %$cur) {
-		if ($idle_at < $old) {
-			my $ds_obj = $DescriptorMap{$fd};
-			$EXPMAP->{$fd} = $idle_at if !$ds_obj->shutdn;
-		} else {
-			$EXPMAP->{$fd} = $idle_at;
-		}
-	}
-	add_uniq_timer('expire', 60, \&expire_old) if $EXPMAP;
-}
-
-sub update_idle_time {
-	my ($self) = @_;
-	my $sock = $self->{sock} or return;
-	$EXPMAP->{fileno($sock)} = now();
-	add_uniq_timer('expire', 60, \&expire_old);
-}
-
-sub not_idle_long {
-	my ($self, $now) = @_;
-	my $sock = $self->{sock} or return;
-	my $idle_at = $EXPMAP->{fileno($sock)} or return;
-	($idle_at + $EXPTIME) > $now;
-}
-
 1;
 
 =head1 AUTHORS (Danga::Socket)
diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index 90b77412f96a..505235864c0b 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -271,13 +271,11 @@ sub worker_quit { # $_[0] = signal name or number (unused)
 		my ($dmap, undef) = @_;
 		my $n = 0;
 		my $now = now();
-
-		foreach my $s (values %$dmap) {
+		for my $s (values %$dmap) {
 			$s->can('busy') or next;
-			if ($s->busy($now)) {
+			if ($s->busy) {
 				++$n;
-			} else {
-				# close as much as possible, early as possible
+			} else { # close as much as possible, early as possible
 				$s->close;
 			}
 		}
diff --git a/lib/PublicInbox/HTTP.pm b/lib/PublicInbox/HTTP.pm
index 8a89dd73b031..8057481d1ce5 100644
--- a/lib/PublicInbox/HTTP.pm
+++ b/lib/PublicInbox/HTTP.pm
@@ -459,10 +459,9 @@ sub close {
 	$self->SUPER::close; # PublicInbox::DS::close
 }
 
-# for graceful shutdown in PublicInbox::Daemon:
-sub busy () {
+sub busy { # for graceful shutdown in PublicInbox::Daemon:
 	my ($self) = @_;
-	($self->{rbuf} || exists($self->{env}) || $self->{wbuf});
+	defined($self->{rbuf}) || exists($self->{env}) || defined($self->{wbuf})
 }
 
 # runs $cb on the next iteration of the event loop at earliest
diff --git a/lib/PublicInbox/IMAP.pm b/lib/PublicInbox/IMAP.pm
index bc34f4feea1e..41bcf9af295d 100644
--- a/lib/PublicInbox/IMAP.pm
+++ b/lib/PublicInbox/IMAP.pm
@@ -99,9 +99,6 @@ undef %FETCH_NEED;
 my $valid_range = '[0-9]+|[0-9]+:[0-9]+|[0-9]+:\*';
 $valid_range = qr/\A(?:$valid_range)(?:,(?:$valid_range))*\z/;
 
-# RFC 3501 5.4. Autologout Timer needs to be >= 30min
-$PublicInbox::DS::EXPTIME = 60 * 30;
-
 sub greet ($) {
 	my ($self) = @_;
 	my $capa = capa($self);
@@ -124,7 +121,6 @@ sub new ($$$) {
 	} else {
 		greet($self);
 	}
-	$self->update_idle_time;
 	$self;
 }
 
@@ -323,7 +319,6 @@ sub idle_tick_all {
 	$IDLERS = undef;
 	for my $i (values %$old) {
 		next if ($i->{wbuf} || !exists($i->{-idle_tag}));
-		$i->update_idle_time or next;
 		$IDLERS->{fileno($i->{sock})} = $i;
 		$i->write(\"* OK Still here\r\n");
 	}
@@ -1226,8 +1221,6 @@ sub long_step {
 		out($self, " deferred[$fd] aborted - %0.6f", $elapsed);
 		$self->close;
 	} elsif ($more) { # $self->{wbuf}:
-		$self->update_idle_time;
-
 		# control passed to ibx_async_cat if $more == \undef
 		requeue_once($self) if !ref($more);
 	} else { # all done!
@@ -1269,7 +1262,6 @@ sub event_step {
 
 	return unless $self->flush_write && $self->{sock} && !$self->{long_cb};
 
-	$self->update_idle_time;
 	# only read more requests if we've drained the write buffer,
 	# otherwise we can be buffering infinitely w/o backpressure
 
@@ -1295,7 +1287,6 @@ sub event_step {
 
 	return $self->close if $r < 0;
 	$self->rbuf_idle($rbuf);
-	$self->update_idle_time;
 
 	# maybe there's more pipelined data, or we'll have
 	# to register it for socket-readiness notifications
@@ -1334,14 +1325,14 @@ sub cmd_starttls ($$) {
 	undef;
 }
 
-# for graceful shutdown in PublicInbox::Daemon:
-sub busy {
-	my ($self, $now) = @_;
+sub busy { # for graceful shutdown in PublicInbox::Daemon:
+	my ($self) = @_;
 	if (defined($self->{-idle_tag})) {
 		$self->write(\"* BYE server shutting down\r\n");
 		return; # not busy anymore
 	}
-	($self->{rbuf} || $self->{wbuf} || $self->not_idle_long($now));
+	defined($self->{rbuf}) || defined($self->{wbuf}) ||
+		!$self->write(\"* BYE server shutting down\r\n");
 }
 
 sub close {
diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
index aa0193687dd6..b36722d72745 100644
--- a/lib/PublicInbox/NNTP.pm
+++ b/lib/PublicInbox/NNTP.pm
@@ -65,7 +65,6 @@ sub new ($$$) {
 	} else {
 		greet($self);
 	}
-	$self->update_idle_time;
 	$self;
 }
 
@@ -651,8 +650,6 @@ sub long_step {
 		out($self, " deferred[$fd] aborted - %0.6f", $elapsed);
 		$self->close;
 	} elsif ($more) { # $self->{wbuf}:
-		$self->update_idle_time;
-
 		# COMPRESS users all share the same DEFLATE context.
 		# Flush it here to ensure clients don't see
 		# each other's data
@@ -1050,7 +1047,6 @@ sub event_step {
 
 	return unless $self->flush_write && $self->{sock} && !$self->{long_cb};
 
-	$self->update_idle_time;
 	# only read more requests if we've drained the write buffer,
 	# otherwise we can be buffering infinitely w/o backpressure
 
@@ -1072,17 +1068,15 @@ sub event_step {
 	out($self, "[$fd] %s - %0.6f$pending", $line, now() - $t0);
 	return $self->close if $r < 0;
 	$self->rbuf_idle($rbuf);
-	$self->update_idle_time;
 
 	# maybe there's more pipelined data, or we'll have
 	# to register it for socket-readiness notifications
 	$self->requeue unless $pending;
 }
 
-# for graceful shutdown in PublicInbox::Daemon:
-sub busy {
-	my ($self, $now) = @_;
-	($self->{rbuf} || $self->{wbuf} || $self->not_idle_long($now));
+sub busy { # for graceful shutdown in PublicInbox::Daemon:
+	my ($self) = @_;
+	defined($self->{rbuf}) || defined($self->{wbuf})
 }
 
 1;

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 04/12] httpd: move pipeline logic into event_step
  2021-10-16  1:00 [PATCH 00/16] some yak-shaving and annoyance fixes Eric Wong
                   ` (2 preceding siblings ...)
  2021-10-16  1:00 ` [PATCH 03/12] imapd+nntpd: drop timer-based expiration Eric Wong
@ 2021-10-16  1:00 ` Eric Wong
  2021-10-16  1:00 ` [PATCH 05/12] lei: golf PATH2CFG cleanup Eric Wong
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2021-10-16  1:00 UTC (permalink / raw)
  To: meta

Most of the HTTP server code was written for Danga::Socket and
not fully-transitioned to take advantage of PublicInbox::DS.
This change brings it up-to-date with the style of pipeline
handling used for -imapd and -nntpd.
---
 lib/PublicInbox/HTTP.pm | 59 +++++++++++------------------------------
 1 file changed, 15 insertions(+), 44 deletions(-)

diff --git a/lib/PublicInbox/HTTP.pm b/lib/PublicInbox/HTTP.pm
index 8057481d1ce5..0f4b5047784a 100644
--- a/lib/PublicInbox/HTTP.pm
+++ b/lib/PublicInbox/HTTP.pm
@@ -37,16 +37,6 @@ use constant {
 };
 use Errno qw(EAGAIN);
 
-my $pipelineq = [];
-sub process_pipelineq () {
-	my $q = $pipelineq;
-	$pipelineq = [];
-	foreach (@$q) {
-		next unless $_->{sock};
-		rbuf_process($_);
-	}
-}
-
 # Use the same configuration parameter as git since this is primarily
 # a slow-client sponge for git-http-backend
 # TODO: support per-respository http.maxRequestBuffer somehow...
@@ -86,32 +76,24 @@ sub event_step { # called by PublicInbox::DS
 	# otherwise we can be buffering infinitely w/o backpressure
 
 	return read_input($self) if ref($self->{env});
-	my $rbuf = $self->{rbuf} // (\(my $x = ''));
-	$self->do_read($rbuf, 8192, length($$rbuf)) or return;
-	rbuf_process($self, $rbuf);
-}
-
-sub rbuf_process {
-	my ($self, $rbuf) = @_;
-	$rbuf //= $self->{rbuf} // (\(my $x = ''));
 
+	my $rbuf = $self->{rbuf} // (\(my $x = ''));
 	my %env = %{$self->{httpd}->{env}}; # full hash copy
-	my $r = parse_http_request($$rbuf, \%env);
-
-	# We do not support Trailers in chunked requests, for now
-	# (they are rarely-used and git (as of 2.7.2) does not use them)
-	if ($r == -1 || $env{HTTP_TRAILER} ||
-			# this length-check is necessary for PURE_PERL=1:
-			($r == -2 && length($$rbuf) > 0x4000)) {
-		return quit($self, 400);
-	}
-	if ($r < 0) { # incomplete
-		$self->rbuf_idle($rbuf);
-		return $self->requeue;
+	my $r;
+	while (($r = parse_http_request($$rbuf, \%env)) < 0) {
+		# We do not support Trailers in chunked requests, for
+		# now (they are rarely-used and git (as of 2.7.2) does
+		# not use them).
+		# this length-check is necessary for PURE_PERL=1:
+		if ($r == -1 || $env{HTTP_TRAILER} ||
+				($r == -2 && length($$rbuf) > 0x4000)) {
+			return quit($self, 400);
+		}
+		$self->do_read($rbuf, 8192, length($$rbuf)) or return;
 	}
 	$$rbuf = substr($$rbuf, $r);
-	my $len = input_prepare($self, \%env);
-	defined $len or return write_err($self, undef); # EMFILE/ENFILE
+	my $len = input_prepare($self, \%env) //
+		return write_err($self, undef); # EMFILE/ENFILE
 
 	$len ? read_input($self, $rbuf) : app_dispatch($self, undef, $rbuf);
 }
@@ -238,22 +220,11 @@ sub identity_write ($$) {
 	$self->write(\($_[1])) if $_[1] ne '';
 }
 
-sub next_request ($) {
-	my ($self) = @_;
-	if ($self->{rbuf}) {
-		# avoid recursion for pipelined requests
-		PublicInbox::DS::requeue(\&process_pipelineq) if !@$pipelineq;
-		push @$pipelineq, $self;
-	} else { # wait for next request
-		$self->requeue;
-	}
-}
-
 sub response_done {
 	my ($self, $alive) = @_;
 	delete $self->{env}; # we're no longer busy
 	$self->write(\"0\r\n\r\n") if $alive == 2;
-	$self->write($alive ? \&next_request : \&close);
+	$self->write($alive ? $self->can('requeue') : \&close);
 }
 
 sub getline_pull {

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 05/12] lei: golf PATH2CFG cleanup
  2021-10-16  1:00 [PATCH 00/16] some yak-shaving and annoyance fixes Eric Wong
                   ` (3 preceding siblings ...)
  2021-10-16  1:00 ` [PATCH 04/12] httpd: move pipeline logic into event_step Eric Wong
@ 2021-10-16  1:00 ` Eric Wong
  2021-10-16  1:00 ` [PATCH 06/12] lei: always keep cwd fd {3} for ->fchdir Eric Wong
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2021-10-16  1:00 UTC (permalink / raw)
  To: meta

More code means more bugs.
---
 lib/PublicInbox/LEI.pm | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index a526a91f8035..e7f37efaf0a3 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -850,9 +850,7 @@ sub _lei_cfg ($;$) {
 	}
 	if (scalar(keys %PATH2CFG) > 5) {
 		# FIXME: use inotify/EVFILT_VNODE to detect unlinked configs
-		for my $k (keys %PATH2CFG) {
-			delete($PATH2CFG{$k}) unless -f $k
-		}
+		delete(@PATH2CFG{grep(!-f, keys %PATH2CFG)});
 	}
 	$self->{cfg} = $PATH2CFG{$f} = $cfg;
 	refresh_watches($self);

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 06/12] lei: always keep cwd fd {3} for ->fchdir
  2021-10-16  1:00 [PATCH 00/16] some yak-shaving and annoyance fixes Eric Wong
                   ` (4 preceding siblings ...)
  2021-10-16  1:00 ` [PATCH 05/12] lei: golf PATH2CFG cleanup Eric Wong
@ 2021-10-16  1:00 ` Eric Wong
  2021-10-16  1:00 ` [PATCH 07/12] lei: more eval guards for die on failure Eric Wong
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2021-10-16  1:00 UTC (permalink / raw)
  To: meta

The extra FD shouldn't cause noticeable overhead in short-lived
workers, and it lets us simplify lei->rel2abs.  Get rid of a
2-argument form of open() while we're at it, since it's been
considered for warning+deprecation by Perl for safety reasons.
---
 lib/PublicInbox/LEI.pm | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index e7f37efaf0a3..0cdcf4492885 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -77,19 +77,16 @@ sub rel2abs {
 		return $p;
 	}
 	my $pwd = $self->{env}->{PWD};
-	my $cwd;
 	if (defined $pwd) {
-		my $xcwd = $self->{3} //
-			($cwd = getcwd() // die "getcwd(PWD=$pwd): $!");
 		if (my @st_pwd = stat($pwd)) {
-			my @st_cwd = stat($xcwd) or die "stat($xcwd): $!";
+			my @st_cwd = stat($self->{3}) or die "stat({3}): $!";
 			"@st_pwd[1,0]" eq "@st_cwd[1,0]" or
 				$self->{env}->{PWD} = $pwd = undef;
 		} else { # PWD was invalid
 			$self->{env}->{PWD} = $pwd = undef;
 		}
 	}
-	$pwd //= $self->{env}->{PWD} = $cwd // getcwd() // die "getcwd: $!";
+	$pwd //= $self->{env}->{PWD} = getcwd() // die "getcwd: $!";
 	File::Spec->rel2abs($p, $pwd);
 }
 
@@ -558,7 +555,8 @@ sub _lei_atfork_child {
 	my ($self, $persist) = @_;
 	# we need to explicitly close things which are on stack
 	if ($persist) {
-		chdir '/' or die "chdir(/): $!";
+		open $self->{3}, '<', '/' or die "open(/) $!";
+		fchdir($self) or die;
 		close($_) for (grep(defined, delete @$self{qw(0 1 2 sock)}));
 		if (my $cfg = $self->{cfg}) {
 			delete @$cfg{qw(-lei_store -watches -lei_note_event)};
@@ -568,7 +566,7 @@ sub _lei_atfork_child {
 		STDERR->autoflush(1);
 		POSIX::setpgid(0, $$) // die "setpgid(0, $$): $!";
 	}
-	close($_) for (grep(defined, delete @$self{qw(3 old_1 au_done)}));
+	close($_) for (grep(defined, delete @$self{qw(old_1 au_done)}));
 	delete $self->{-socks};
 	if (my $op_c = delete $self->{pkt_op_c}) {
 		close(delete $op_c->{sock});
@@ -1190,7 +1188,7 @@ sub cfg2lei ($) {
 	open($lei->{0}, '<&', \*STDIN) or die "dup 0: $!";
 	open($lei->{1}, '>>&', \*STDOUT) or die "dup 1: $!";
 	open($lei->{2}, '>>&', \*STDERR) or die "dup 2: $!";
-	open($lei->{3}, '/') or die "open /: $!";
+	open($lei->{3}, '<', '/') or die "open /: $!";
 	my ($x, $y);
 	socketpair($x, $y, AF_UNIX, SOCK_SEQPACKET, 0) or die "socketpair: $!";
 	$lei->{sock} = $x;

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 07/12] lei: more eval guards for die on failure
  2021-10-16  1:00 [PATCH 00/16] some yak-shaving and annoyance fixes Eric Wong
                   ` (5 preceding siblings ...)
  2021-10-16  1:00 ` [PATCH 06/12] lei: always keep cwd fd {3} for ->fchdir Eric Wong
@ 2021-10-16  1:00 ` Eric Wong
  2021-10-16  1:00 ` [PATCH 08/12] extindex: prune invalid alternate entries on --gc Eric Wong
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2021-10-16  1:00 UTC (permalink / raw)
  To: meta

Relying on $lei->fail is unsustainable since there'll always
be parts of our code and dependencies which can trigger die()
and break the event loop.
---
 lib/PublicInbox/LEI.pm        |  6 +++---
 lib/PublicInbox/LeiLcat.pm    | 21 +++++++++------------
 lib/PublicInbox/LeiQuery.pm   | 24 +++++++++++-------------
 lib/PublicInbox/LeiXSearch.pm |  9 +++++----
 4 files changed, 28 insertions(+), 32 deletions(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 0cdcf4492885..511b2c1d03a7 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -556,7 +556,7 @@ sub _lei_atfork_child {
 	# we need to explicitly close things which are on stack
 	if ($persist) {
 		open $self->{3}, '<', '/' or die "open(/) $!";
-		fchdir($self) or die;
+		fchdir($self);
 		close($_) for (grep(defined, delete @$self{qw(0 1 2 sock)}));
 		if (my $cfg = $self->{cfg}) {
 			delete @$cfg{qw(-lei_store -watches -lei_note_event)};
@@ -779,7 +779,7 @@ sub lazy_cb ($$$) {
 
 sub dispatch {
 	my ($self, $cmd, @argv) = @_;
-	fchdir($self) or return;
+	fchdir($self);
 	local %ENV = %{$self->{env}};
 	local $current_lei = $self; # for __WARN__
 	$self->{2}->autoflush(1); # keep stdout buffered until x_it|DESTROY
@@ -1381,7 +1381,7 @@ sub wq_done_wait { # dwaitpid callback
 sub fchdir {
 	my ($lei) = @_;
 	my $dh = $lei->{3} // die 'BUG: lei->{3} (CWD) gone';
-	chdir($dh) || $lei->fail("fchdir: $!");
+	chdir($dh) || die "fchdir: $!";
 }
 
 sub wq_eof { # EOF callback for main daemon
diff --git a/lib/PublicInbox/LeiLcat.pm b/lib/PublicInbox/LeiLcat.pm
index d553b18733da..191f6f244857 100644
--- a/lib/PublicInbox/LeiLcat.pm
+++ b/lib/PublicInbox/LeiLcat.pm
@@ -124,18 +124,15 @@ could not extract Message-ID from $x
 
 sub _stdin { # PublicInbox::InputPipe::consume callback for --stdin
 	my ($lei) = @_; # $_[1] = $rbuf
-	if (defined($_[1])) {
-		$_[1] eq '' and return eval {
-			$lei->fchdir or return;
-			my @argv = split(/\s+/, $lei->{mset_opt}->{qstr});
-			$lei->{mset_opt}->{qstr} = extract_all($lei, @argv)
-				or return;
-			$lei->_start_query;
-		};
-		$lei->{mset_opt}->{qstr} .= $_[1];
-	} else {
-		$lei->fail("error reading stdin: $!");
-	}
+	$_[1] // return $lei->fail("error reading stdin: $!");
+	return $lei->{mset_opt}->{qstr} .= $_[1] if $_[1] ne '';
+	eval {
+		$lei->fchdir;
+		my @argv = split(/\s+/, $lei->{mset_opt}->{qstr});
+		$lei->{mset_opt}->{qstr} = extract_all($lei, @argv) or return;
+		$lei->_start_query;
+	};
+	$lei->fail($@) if $@;
 }
 
 sub lei_lcat {
diff --git a/lib/PublicInbox/LeiQuery.pm b/lib/PublicInbox/LeiQuery.pm
index c65b00ca0986..ec2ece051492 100644
--- a/lib/PublicInbox/LeiQuery.pm
+++ b/lib/PublicInbox/LeiQuery.pm
@@ -55,19 +55,17 @@ sub _start_query { # used by "lei q" and "lei up"
 }
 
 sub qstr_add { # PublicInbox::InputPipe::consume callback for --stdin
-	my ($self) = @_; # $_[1] = $rbuf
-	if (defined($_[1])) {
-		$_[1] eq '' and return eval {
-			$self->fchdir or return;
-			$self->{mset_opt}->{q_raw} = $self->{mset_opt}->{qstr};
-			$self->{lse}->query_approxidate($self->{lse}->git,
-						$self->{mset_opt}->{qstr});
-			_start_query($self);
-		};
-		$self->{mset_opt}->{qstr} .= $_[1];
-	} else {
-		$self->fail("error reading stdin: $!");
-	}
+	my ($lei) = @_; # $_[1] = $rbuf
+	$_[1] // $lei->fail("error reading stdin: $!");
+	return $lei->{mset_opt}->{qstr} .= $_[1] if $_[1] ne '';
+	eval {
+		$lei->fchdir;
+		$lei->{mset_opt}->{q_raw} = $lei->{mset_opt}->{qstr};
+		$lei->{lse}->query_approxidate($lei->{lse}->git,
+						$lei->{mset_opt}->{qstr});
+		_start_query($lei);
+	};
+	$lei->fail($@) if $@;
 }
 
 sub lxs_prepare {
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index 4aa2a81c0025..8ab84b15c00b 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -460,10 +460,11 @@ sub do_post_augment {
 	my ($lei) = @_;
 	local $PublicInbox::LEI::current_lei = $lei;
 	my $l2m = $lei->{l2m} or return; # client disconnected
-	$lei->fchdir or return;
-	my $err;
-	eval { $l2m->post_augment($lei) };
-	$err = $@;
+	eval {
+		$lei->fchdir;
+		$l2m->post_augment($lei);
+	};
+	my $err = $@;
 	if ($err) {
 		if (my $lxs = delete $lei->{lxs}) {
 			$lxs->wq_kill('-TERM');

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 08/12] extindex: prune invalid alternate entries on --gc
  2021-10-16  1:00 [PATCH 00/16] some yak-shaving and annoyance fixes Eric Wong
                   ` (6 preceding siblings ...)
  2021-10-16  1:00 ` [PATCH 07/12] lei: more eval guards for die on failure Eric Wong
@ 2021-10-16  1:00 ` Eric Wong
  2021-10-16  1:01 ` [PATCH 09/12] lei_overview: die rather than lei->fail Eric Wong
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2021-10-16  1:00 UTC (permalink / raw)
  To: meta

Seeing the same warning over and over again gets annoying.
---
 lib/PublicInbox/ExtSearchIdx.pm | 8 ++++----
 lib/PublicInbox/MultiGit.pm     | 6 +++++-
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index ba7588d53254..a08a94515582 100644
--- a/lib/PublicInbox/ExtSearchIdx.pm
+++ b/lib/PublicInbox/ExtSearchIdx.pm
@@ -1192,7 +1192,7 @@ sub idx_init { # similar to V2Writable
 	$self->git->cleanup;
 	my $mode = 0644;
 	my $ALL = $self->git->{git_dir}; # topdir/ALL.git
-	my ($has_new, $alt, $seen);
+	my ($has_new, $alt, $seen, $prune, $prune_nr);
 	if ($opt->{-private}) { # LeiStore
 		my $local = "$self->{topdir}/local"; # lei/store
 		$self->{mg} //= PublicInbox::MultiGit->new($self->{topdir},
@@ -1208,8 +1208,8 @@ sub idx_init { # similar to V2Writable
 	} else { # extindex has no epochs
 		$self->{mg} //= PublicInbox::MultiGit->new($self->{topdir},
 							'ALL.git');
-		($alt, $seen) = $self->{mg}->read_alternates(\$mode,
-							$opt->{-idx_gc});
+		$prune = $opt->{-idx_gc} ? \$prune_nr : undef;
+		($alt, $seen) = $self->{mg}->read_alternates(\$mode, $prune);
 		PublicInbox::Import::init_bare($ALL);
 	}
 
@@ -1243,7 +1243,7 @@ sub idx_init { # similar to V2Writable
 		}
 		$new .= "$d\n";
 	}
-	($has_new || $new ne '') and
+	($has_new || $prune_nr || $new ne '') and
 		$self->{mg}->write_alternates($mode, $alt, $new);
 	$git_midx and $self->with_umask(sub {
 		my @cmd = ('multi-pack-index');
diff --git a/lib/PublicInbox/MultiGit.pm b/lib/PublicInbox/MultiGit.pm
index 91d7998aa51f..9429a00ca64c 100644
--- a/lib/PublicInbox/MultiGit.pm
+++ b/lib/PublicInbox/MultiGit.pm
@@ -45,7 +45,11 @@ sub read_alternates {
 				$alt{$rel} = $score;
 			} else {
 				warn "W: stat($dir) failed: $! ($f)";
-				$alt{$rel} = $score unless $prune;
+				if ($prune) {
+					++$$prune;
+				} else {
+					$alt{$rel} = $score;
+				}
 			}
 		}
 	} elsif (!$!{ENOENT}) {

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 09/12] lei_overview: die rather than lei->fail
  2021-10-16  1:00 [PATCH 00/16] some yak-shaving and annoyance fixes Eric Wong
                   ` (7 preceding siblings ...)
  2021-10-16  1:00 ` [PATCH 08/12] extindex: prune invalid alternate entries on --gc Eric Wong
@ 2021-10-16  1:01 ` Eric Wong
  2021-10-16  1:01 ` [PATCH 10/12] lei_to_mail: quiet down abort messages Eric Wong
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2021-10-16  1:01 UTC (permalink / raw)
  To: meta

This will make our code more flexible in case it gets used in
non-lei things.
---
 lib/PublicInbox/LEI.pm         |  2 +-
 lib/PublicInbox/LeiOverview.pm | 24 +++++++++++-------------
 2 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 511b2c1d03a7..876598f9530e 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -1108,7 +1108,7 @@ sub accept_dispatch { # Listener {post_accept} callback
 	my %env = map { split(/=/, $_, 2) } splice(@argv, $argc);
 	$self->{env} = \%env;
 	eval { dispatch($self, @argv) };
-	send($sock, $@, MSG_EOR) if $@;
+	$self->fail($@) if $@;
 }
 
 sub dclose {
diff --git a/lib/PublicInbox/LeiOverview.pm b/lib/PublicInbox/LeiOverview.pm
index 223db22244ec..1b9dc9701c95 100644
--- a/lib/PublicInbox/LeiOverview.pm
+++ b/lib/PublicInbox/LeiOverview.pm
@@ -37,16 +37,16 @@ sub ovv_out_lk_cancel ($) {
 	unlink($lock_path);
 }
 
-sub detect_fmt ($$) {
-	my ($lei, $dst) = @_;
+sub detect_fmt ($) {
+	my ($dst) = @_;
 	if ($dst =~ m!\A([:/]+://)!) {
-		$lei->fail("$1 support not implemented, yet\n");
+		die "$1 support not implemented, yet\n";
 	} elsif (!-e $dst || -d _) {
 		'maildir'; # the default TODO: MH?
 	} elsif (-f _ || -p _) {
-		$lei->fail("unable to determine mbox family of $dst\n");
+		die "unable to determine mbox family of $dst\n";
 	} else {
-		$lei->fail("unable to determine format of $dst\n");
+		die "unable to determine format of $dst\n";
 	}
 }
 
@@ -60,20 +60,19 @@ sub new {
 	my $fmt = $opt->{$ofmt_key};
 	$fmt = lc($fmt) if defined $fmt;
 	if ($dst =~ m!\A([a-z0-9\+]+)://!is) {
-		defined($fmt) and return $lei->fail(<<"");
+		defined($fmt) and die <<"";
 --$ofmt_key=$fmt invalid with URL $dst
 
 		$fmt = lc $1;
 	} elsif ($dst =~ s/\A([a-z0-9]+)://is) { # e.g. Maildir:/home/user/Mail/
 		my $ofmt = lc $1;
 		$fmt //= $ofmt;
-		return $lei->fail(<<"") if $fmt ne $ofmt;
+		die <<"" if $fmt ne $ofmt;
 --$ofmt_key=$fmt and --output=$ofmt conflict
 
 	}
-
 	my $devfd = $lei->path_to_fd($dst) // return;
-	$fmt //= $devfd >= 0 ? 'json' : (detect_fmt($lei, $dst) or return);
+	$fmt //= $devfd >= 0 ? 'json' : detect_fmt($dst);
 
 	if (index($dst, '://') < 0) { # not a URL, so assume path
 		 $dst = $lei->canonpath_harder($dst);
@@ -90,7 +89,7 @@ sub new {
 		$opt->{pretty} //= $isatty;
 		if (!$isatty && -f _) {
 			my $fl = fcntl($lei->{$devfd}, F_GETFL, 0) //
-				return $lei->fail("fcntl(stdout): $!");
+					die("fcntl(/dev/fd/$devfd): $!\n");
 			ovv_out_lk_init($self) unless ($fl & O_APPEND);
 		} else {
 			ovv_out_lk_init($self);
@@ -101,14 +100,13 @@ sub new {
 	if ($json) {
 		$lei->{dedupe} //= PublicInbox::LeiDedupe->new($lei);
 	} else {
-		$lei->{l2m} = eval { PublicInbox::LeiToMail->new($lei) };
-		return $lei->fail($@) if $@;
+		$lei->{l2m} = PublicInbox::LeiToMail->new($lei);
 		if ($opt->{mua} && $lei->{l2m}->lock_free) {
 			$lei->{early_mua} = 1;
 			$opt->{alert} //= [ ':WINCH,:bell' ] if -t $lei->{1};
 		}
 	}
-	return $lei->fail('--shared is only for v2 inbox output') if
+	die("--shared is only for v2 inbox output\n") if
 		$self->{fmt} ne 'v2' && $lei->{opt}->{shared};
 	$self;
 }

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 10/12] lei_to_mail: quiet down abort messages
  2021-10-16  1:00 [PATCH 00/16] some yak-shaving and annoyance fixes Eric Wong
                   ` (8 preceding siblings ...)
  2021-10-16  1:01 ` [PATCH 09/12] lei_overview: die rather than lei->fail Eric Wong
@ 2021-10-16  1:01 ` Eric Wong
  2021-10-16  1:01 ` [PATCH 11/12] inbox + search: use 5.10.1 and do some golfing Eric Wong
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2021-10-16  1:01 UTC (permalink / raw)
  To: meta

We don't need to flood the terminal with "W: $oid is  (!= blob)\n"
messages when somebody nukes a git cat-file process from under
us.
---
 lib/PublicInbox/LeiToMail.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index 76e103c7c235..ca4e92de48b7 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -132,6 +132,7 @@ sub eml2mboxcl2 {
 
 sub git_to_mail { # git->cat_async callback
 	my ($bref, $oid, $type, $size, $arg) = @_;
+	$type // return; # called by git->async_abort
 	my ($write_cb, $smsg) = @$arg;
 	if ($type eq 'missing' && $smsg->{-lms_ro}) {
 		if ($bref = $smsg->{-lms_ro}->local_blob($oid, 1)) {

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 11/12] inbox + search: use 5.10.1 and do some golfing
  2021-10-16  1:00 [PATCH 00/16] some yak-shaving and annoyance fixes Eric Wong
                   ` (9 preceding siblings ...)
  2021-10-16  1:01 ` [PATCH 10/12] lei_to_mail: quiet down abort messages Eric Wong
@ 2021-10-16  1:01 ` Eric Wong
  2021-10-16  1:01 ` [PATCH 12/12] httpd/async: switch to level-triggered epoll Eric Wong
  2021-10-16  1:42 ` [PATCH 00/12] some yak-shaving and annoyance fixes Eric Wong
  12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2021-10-16  1:01 UTC (permalink / raw)
  To: meta

Some yak-shaving while I try to track down other bugs...
---
 lib/PublicInbox/Inbox.pm  | 11 +++++------
 lib/PublicInbox/Search.pm |  7 +++----
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm
index 74b8a74f8856..b7b71268187e 100644
--- a/lib/PublicInbox/Inbox.pm
+++ b/lib/PublicInbox/Inbox.pm
@@ -4,6 +4,7 @@
 # Represents a public-inbox (which may have multiple mailing addresses)
 package PublicInbox::Inbox;
 use strict;
+use v5.10.1;
 use PublicInbox::Git;
 use PublicInbox::MID qw(mid2path);
 use PublicInbox::Eml;
@@ -293,17 +294,15 @@ sub msg_by_smsg ($$) {
 
 	# ghosts may have undef smsg (from SearchThread.node) or
 	# no {blob} field
-	return unless defined $smsg;
-	defined(my $blob = $smsg->{blob}) or return;
-
-	$self->git->cat_file($blob);
+	$smsg // return;
+	$self->git->cat_file($smsg->{blob} // return);
 }
 
 sub smsg_eml {
 	my ($self, $smsg) = @_;
 	my $bref = msg_by_smsg($self, $smsg) or return;
 	my $eml = PublicInbox::Eml->new($bref);
-	$smsg->populate($eml) unless exists($smsg->{num}); # v1 w/o SQLite
+	$smsg->{num} // $smsg->populate($eml);
 	$eml;
 }
 
@@ -313,7 +312,7 @@ sub smsg_by_mid ($$) {
 	my $smsg;
 	if (my $mm = $self->mm) {
 		# favor the Message-ID we used for the NNTP article number:
-		defined(my $num = $mm->num_for($mid)) or return;
+		my $num = $mm->num_for($mid) // return;
 		$smsg = $over->get_art($num);
 	} else {
 		my ($id, $prev);
diff --git a/lib/PublicInbox/Search.pm b/lib/PublicInbox/Search.pm
index 145fb56ce750..600e6400d4b6 100644
--- a/lib/PublicInbox/Search.pm
+++ b/lib/PublicInbox/Search.pm
@@ -5,6 +5,7 @@
 # Read-only search interface for use by the web and NNTP interfaces
 package PublicInbox::Search;
 use strict;
+use v5.10.1;
 use parent qw(Exporter);
 our @EXPORT_OK = qw(retry_reopen int_val get_pct xap_terms);
 use List::Util qw(max);
@@ -398,12 +399,10 @@ sub retry_reopen {
 	my ($self, $cb, @arg) = @_;
 	for my $i (1..10) {
 		if (wantarray) {
-			my @ret;
-			eval { @ret = $cb->($self, @arg) };
+			my @ret = eval { $cb->($self, @arg) };
 			return @ret unless $@;
 		} else {
-			my $ret;
-			eval { $ret = $cb->($self, @arg) };
+			my $ret = eval { $cb->($self, @arg) };
 			return $ret unless $@;
 		}
 		# Exception: The revision being read has been discarded -

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 12/12] httpd/async: switch to level-triggered epoll
  2021-10-16  1:00 [PATCH 00/16] some yak-shaving and annoyance fixes Eric Wong
                   ` (10 preceding siblings ...)
  2021-10-16  1:01 ` [PATCH 11/12] inbox + search: use 5.10.1 and do some golfing Eric Wong
@ 2021-10-16  1:01 ` Eric Wong
  2021-10-16  1:42 ` [PATCH 00/12] some yak-shaving and annoyance fixes Eric Wong
  12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2021-10-16  1:01 UTC (permalink / raw)
  To: meta

We'll save ourselves some code here and let the kernel do more
work, instead.
---
 Documentation/technical/ds.txt |  3 +--
 lib/PublicInbox/HTTPD/Async.pm | 16 +++++-----------
 lib/PublicInbox/Qspawn.pm      |  1 -
 3 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/Documentation/technical/ds.txt b/Documentation/technical/ds.txt
index 7bc1ad79ce0c..5a1655a1450e 100644
--- a/Documentation/technical/ds.txt
+++ b/Documentation/technical/ds.txt
@@ -77,8 +77,7 @@ New features
   which (if any) events it's interested in for the next loop iteration.
 
 * Edge-triggering available via EPOLLET or EV_CLEAR.  These reduce wakeups
-  for unidirectional classes (e.g. PublicInbox::Listener sockets,
-  and pipes via PublicInbox::HTTPD::Async).
+  for unidirectional classes when throughput is more important than fairness.
 
 * IO::Socket::SSL support (for NNTPS, STARTTLS+NNTP, HTTPS)
 
diff --git a/lib/PublicInbox/HTTPD/Async.pm b/lib/PublicInbox/HTTPD/Async.pm
index 7238650aff97..1651da88ac03 100644
--- a/lib/PublicInbox/HTTPD/Async.pm
+++ b/lib/PublicInbox/HTTPD/Async.pm
@@ -17,7 +17,7 @@ package PublicInbox::HTTPD::Async;
 use strict;
 use parent qw(PublicInbox::DS);
 use Errno qw(EAGAIN);
-use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
+use PublicInbox::Syscall qw(EPOLLIN);
 
 # This is called via: $env->{'pi-httpd.async'}->()
 # $io is a read-only pipe ($rpipe) for now, but may be a
@@ -39,7 +39,7 @@ sub new {
 	}, $class;
 	my $pp = tied *$io;
 	$pp->{fh}->blocking(0) // die "$io->blocking(0): $!";
-	$self->SUPER::new($io, EPOLLIN | EPOLLET);
+	$self->SUPER::new($io, EPOLLIN);
 }
 
 sub event_step {
@@ -54,15 +54,12 @@ sub event_step {
 		my $r = sysread($sock, my $buf, 65536);
 		if ($r) {
 			$self->{fh}->write($buf); # may call $http->close
-			if ($http->{sock}) { # !closed
-				$self->requeue;
-				# let other clients get some work done, too
-				return;
-			}
+			# let other clients get some work done, too
+			return if $http->{sock}; # !closed
 
 			# else: fall through to close below...
 		} elsif (!defined $r && $! == EAGAIN) {
-			return; # EPOLLET means we'll be notified
+			return; # EPOLLIN means we'll be notified
 		}
 
 		# Done! Error handling will happen in $self->{fh}->close
@@ -89,9 +86,6 @@ sub async_pass {
 
 	$self->{http} = $http;
 	$self->{fh} = $fh;
-
-	# either hit EAGAIN or ->requeue to keep EPOLLET happy
-	event_step($self);
 }
 
 # may be called as $forward->close in PublicInbox::HTTP or EOF (event_step)
diff --git a/lib/PublicInbox/Qspawn.pm b/lib/PublicInbox/Qspawn.pm
index a1ff65b65324..53d0ad55ee84 100644
--- a/lib/PublicInbox/Qspawn.pm
+++ b/lib/PublicInbox/Qspawn.pm
@@ -192,7 +192,6 @@ sub event_step {
 sub rd_hdr ($) {
 	my ($self) = @_;
 	# typically used for reading CGI headers
-	# we must loop until EAGAIN for EPOLLET in HTTPD/Async.pm
 	# We also need to check EINTR for generic PSGI servers.
 	my $ret;
 	my $total_rd = 0;

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 00/12] some yak-shaving and annoyance fixes
  2021-10-16  1:00 [PATCH 00/16] some yak-shaving and annoyance fixes Eric Wong
                   ` (11 preceding siblings ...)
  2021-10-16  1:01 ` [PATCH 12/12] httpd/async: switch to level-triggered epoll Eric Wong
@ 2021-10-16  1:42 ` Eric Wong
  12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2021-10-16  1:42 UTC (permalink / raw)
  To: meta

$Subject =~ s/16/12/
there were 4 patches which weren't ready, yet :x

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-10-16  1:42 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-16  1:00 [PATCH 00/16] some yak-shaving and annoyance fixes Eric Wong
2021-10-16  1:00 ` [PATCH 01/12] smsg: add ->oidbin method Eric Wong
2021-10-16  1:00 ` [PATCH 02/12] dir_idle: do not add watches in ->new Eric Wong
2021-10-16  1:00 ` [PATCH 03/12] imapd+nntpd: drop timer-based expiration Eric Wong
2021-10-16  1:00 ` [PATCH 04/12] httpd: move pipeline logic into event_step Eric Wong
2021-10-16  1:00 ` [PATCH 05/12] lei: golf PATH2CFG cleanup Eric Wong
2021-10-16  1:00 ` [PATCH 06/12] lei: always keep cwd fd {3} for ->fchdir Eric Wong
2021-10-16  1:00 ` [PATCH 07/12] lei: more eval guards for die on failure Eric Wong
2021-10-16  1:00 ` [PATCH 08/12] extindex: prune invalid alternate entries on --gc Eric Wong
2021-10-16  1:01 ` [PATCH 09/12] lei_overview: die rather than lei->fail Eric Wong
2021-10-16  1:01 ` [PATCH 10/12] lei_to_mail: quiet down abort messages Eric Wong
2021-10-16  1:01 ` [PATCH 11/12] inbox + search: use 5.10.1 and do some golfing Eric Wong
2021-10-16  1:01 ` [PATCH 12/12] httpd/async: switch to level-triggered epoll Eric Wong
2021-10-16  1:42 ` [PATCH 00/12] some yak-shaving and annoyance fixes 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).