user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
thread overview below | download mbox.gz: |
* [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%]

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

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