* [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