* [PATCH 03/12] imapd+nntpd: drop timer-based expiration
2021-10-16 1:00 7% [PATCH 00/16] some yak-shaving and annoyance fixes Eric Wong
@ 2021-10-16 1:00 5% ` Eric Wong
0 siblings, 0 replies; 2+ results
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 [relevance 5%]
* [PATCH 00/16] some yak-shaving and annoyance fixes
@ 2021-10-16 1:00 7% Eric Wong
2021-10-16 1:00 5% ` [PATCH 03/12] imapd+nntpd: drop timer-based expiration Eric Wong
0 siblings, 1 reply; 2+ results
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 [relevance 7%]
Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2021-10-16 1:00 7% [PATCH 00/16] some yak-shaving and annoyance fixes Eric Wong
2021-10-16 1:00 5% ` [PATCH 03/12] imapd+nntpd: drop timer-based expiration Eric Wong
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).