From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-3.0 required=3.0 tests=ALL_TRUSTED,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NUMERIC_HTTP_ADDR shortcircuit=no autolearn=ham autolearn_force=no version=3.4.6 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 7F0F91F452 for ; Wed, 12 Apr 2023 10:17:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org; s=selector1; t=1681294662; bh=a3wHqsf/DWrAtxpBMpU2+AJWhdAoc6QD/BV7Qa3itxs=; h=Date:From:To:Subject:References:In-Reply-To:From; b=mj2WbUGBjsXjs4mHSC3UCkgBvF1ZG2cek+lAvyvRJy+0oPc9MP53e4R11b1Ahr09q cGpQiDygMv530j+gJrYXVlQGgWLrY3GzZ5S4dmfVUSJ8N4/3apCVd99klQ/7bC4Jww e0oyYMkx5sBW7UonJVuSobp4SWe5MXYshhDmRv9k= Date: Wed, 12 Apr 2023 10:17:42 +0000 From: Eric Wong To: meta@public-inbox.org Subject: [PATCH v2] listener: support multi-accept like nginx Message-ID: <20230412101742.M402032@dcvr> References: <20190513025610.30378-1-e@80x24.org> <20190625064101.iwif2x6l2t6alhre@dcvr> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20190625064101.iwif2x6l2t6alhre@dcvr> List-Id: Eric Wong wrote: > Eric Wong wrote: > > Similar to the nginx "multi_accept" parameter, this allows > > single-worker deployments to accept() multiple clients whenever > > the listen socket reports readiness via epoll_wait/poll/kevent. > > Given the ability to have per-listener cert/key options(*) for > OpenSSL, I think this parameter should be per-listener, too; > and thus specified in the command-line. Done (after a few years :x). Hopefully the explanation makes sense, English brain is not good today :x --------8<--------- Subject: [PATCH] listener: support multi-accept like nginx While accepting a single connection at-a-time is likely best for multi-worker and/or load-balanced deployments; accepting multiple connections at once should be less bad on overloaded single-worker systems. We can't automatically pick the best value here since worker counts are dynamic via SIGTTIN/SIGTTOU. Process managers (e.g. systemd) can also spawn multiple instances sharing a single listener with no knowledge sharing between listeners. --- Documentation/public-inbox-daemon.pod | 17 ++++++++++ lib/PublicInbox/Daemon.pm | 8 +++-- lib/PublicInbox/Listener.pm | 45 +++++++++++++-------------- 3 files changed, 45 insertions(+), 25 deletions(-) diff --git a/Documentation/public-inbox-daemon.pod b/Documentation/public-inbox-daemon.pod index 81a79a10..71216833 100644 --- a/Documentation/public-inbox-daemon.pod +++ b/Documentation/public-inbox-daemon.pod @@ -115,6 +115,23 @@ per-listener C option. The private key may be concatenated into the path used by the cert, in which case this option is not needed. +=item --multi-accept INTEGER + +By default, each worker accepts one connection at-a-time to maximize +fairness and minimize contention across multiple processes on a +shared listen socket. Accepting multiple connections at once may be +useful in constrained deployments with few, heavily-loaded workers. +Negative values enables a worker to accept all available clients at +once, possibly starving others in the process. C<-1> behaves like +C in nginx; while C<0> (the default) is +C in nginx. Positive values allow +fine-tuning without the runaway behavior of C<-1>. + +This may be specified on a per-listener basis via the C +per-listener directive (e.g. C<-l http://127.0.0.1?multi-accept=1>). + +Default: 0 + =back =head1 SIGNALS diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm index 57435421..30442227 100644 --- a/lib/PublicInbox/Daemon.pm +++ b/lib/PublicInbox/Daemon.pm @@ -136,6 +136,8 @@ sub load_mod ($;$$) { } my $err = $tlsd->{err}; $tlsd->{warn_cb} = sub { print $err @_ }; # for local $SIG{__WARN__} + $opt->{'multi-accept'} and + $xn{'multi-accept'} = $opt->{'multi-accept'}->[-1]; \%xn; } @@ -167,6 +169,7 @@ EOF 'u|user=s' => \$user, 'g|group=s' => \$group, 'D|daemonize' => \$daemonize, + 'multi-accept=i' => \$PublicInbox::Listener::MULTI_ACCEPT, 'cert=s' => \$default_cert, 'key=s' => \$default_key, 'help|h' => \(my $show_help), @@ -251,7 +254,7 @@ EOF $s->blocking(0); my $sockname = sockname($s); warn "# bound $scheme://$sockname\n"; - $xnetd->{$sockname} //= load_mod($scheme); + $xnetd->{$sockname} //= load_mod($scheme, $opt); $listener_names->{$sockname} = $s; push @listeners, $s; } @@ -712,7 +715,8 @@ sub daemon_loop ($) { defer_accept($_, $tls_cb ? 'dataready' : $xn->{af_default}); # this calls epoll_create: - PublicInbox::Listener->new($_, $tls_cb || $xn->{post_accept}) + PublicInbox::Listener->new($_, $tls_cb || $xn->{post_accept}, + $xn->{'multi-accept'}) } @listeners; PublicInbox::DS::event_loop($sig, $oldset); } diff --git a/lib/PublicInbox/Listener.pm b/lib/PublicInbox/Listener.pm index 7cedc349..4669cf04 100644 --- a/lib/PublicInbox/Listener.pm +++ b/lib/PublicInbox/Listener.pm @@ -1,14 +1,15 @@ -# Copyright (C) 2015-2021 all contributors +# Copyright (C) all contributors # License: AGPL-3.0+ # # Used by -nntpd for listen sockets package PublicInbox::Listener; -use strict; +use v5.12; use parent 'PublicInbox::DS'; use Socket qw(SOL_SOCKET SO_KEEPALIVE IPPROTO_TCP TCP_NODELAY); use IO::Handle; use PublicInbox::Syscall qw(EPOLLIN EPOLLEXCLUSIVE); use Errno qw(EAGAIN ECONNABORTED); +our $MULTI_ACCEPT = 0; # Warn on transient errors, mostly resource limitations. # EINTR would indicate the failure to set NonBlocking in systemd or similar @@ -16,37 +17,35 @@ my %ERR_WARN = map {; eval("Errno::$_()") => $_ } qw(EMFILE ENFILE ENOBUFS ENOMEM EINTR); -sub new ($$$) { - my ($class, $s, $cb) = @_; +sub new { + my ($class, $s, $cb, $multi_accept) = @_; setsockopt($s, SOL_SOCKET, SO_KEEPALIVE, 1); setsockopt($s, IPPROTO_TCP, TCP_NODELAY, 1); # ignore errors on non-TCP listen($s, 2**31 - 1); # kernel will clamp my $self = bless { post_accept => $cb }, $class; + $self->{multi_accept} = $multi_accept //= $MULTI_ACCEPT; $self->SUPER::new($s, EPOLLIN|EPOLLEXCLUSIVE); } sub event_step { my ($self) = @_; my $sock = $self->{sock} or return; - - # no loop here, we want to fairly distribute clients - # between multiple processes sharing the same socket - # XXX our event loop needs better granularity for - # a single accept() here to be, umm..., acceptable - # on high-traffic sites. - if (my $addr = accept(my $c, $sock)) { - IO::Handle::blocking($c, 0); # no accept4 :< - eval { $self->{post_accept}->($c, $addr, $sock) }; - warn "E: $@\n" if $@; - } elsif ($! == EAGAIN || $! == ECONNABORTED) { - # EAGAIN is common and likely - # ECONNABORTED is common with bad connections - return; - } elsif (my $sym = $ERR_WARN{int($!)}) { - warn "W: accept(): $! ($sym)\n"; - } else { - warn "BUG?: accept(): $!\n"; - } + my $n = $self->{multi_accept}; + do { + if (my $addr = accept(my $c, $sock)) { + IO::Handle::blocking($c, 0); # no accept4 :< + eval { $self->{post_accept}->($c, $addr, $sock) }; + warn "E: $@\n" if $@; + } elsif ($! == EAGAIN || $! == ECONNABORTED) { + # EAGAIN is common and likely + # ECONNABORTED is common with bad connections + return; + } elsif (my $sym = $ERR_WARN{int($!)}) { + warn "W: accept(): $! ($sym)\n"; + } else { + warn "BUG?: accept(): $!\n"; + } + } while ($n--); } 1;