]> Sergey Matveev's repositories - public-inbox.git/log
public-inbox.git
5 years agohttp: remove unnecessary delete
Eric Wong [Tue, 17 Sep 2019 08:31:23 +0000 (08:31 +0000)]
http: remove unnecessary delete

Only removing $http->{env} is needed to prevent circular
references.  $env->{'psgix.io'} does not need to be deleted
since $env will no longer have any references to it when
->close returns.

5 years agohttp: drop unused `$env' variable after delete
Eric Wong [Tue, 17 Sep 2019 08:31:22 +0000 (08:31 +0000)]
http: drop unused `$env' variable after delete

And explain why we need to do that delete in a comment.

5 years agoqspawn: improve variable naming and commenting
Eric Wong [Tue, 17 Sep 2019 08:31:21 +0000 (08:31 +0000)]
qspawn: improve variable naming and commenting

Naming $start_cb consistently helps avoid confusing new readers,
and some comments will help with understanding flow

5 years agoqspawn: shorten lifetime of circular references
Eric Wong [Tue, 17 Sep 2019 08:31:20 +0000 (08:31 +0000)]
qspawn: shorten lifetime of circular references

All of these circular references are designed to clear
themselves, but these will make actual errors from Devel::Cycle
easier-to-spot.

The circular reference in the limiter {run_queue} is not a real
problem, but we can avoid storing the circular reference until
we actually need to spawn the child, reducing the size of the
Qspawn object while it's in the queue, slightly.

We also do not need to have redundant checks to spawn new
processes, we should only spawn new processes when they're
->start-ed or after waitpid reaps them.

5 years agoqspawn: log errors for generic PSGI server users
Eric Wong [Tue, 17 Sep 2019 08:31:19 +0000 (08:31 +0000)]
qspawn: log errors for generic PSGI server users

Generic PSGI servers have $env->{'psgi.errors'}, too,
so ensure they can log errors.

5 years agoqspawn: remove return value from ->finish
Eric Wong [Tue, 17 Sep 2019 08:31:18 +0000 (08:31 +0000)]
qspawn: remove return value from ->finish

We don't use the return value in real code since we do waitpid
asynchronously, now.  So simplify our runtime code at the cost
of making our test slighly more complex.

5 years agodoc: update config manpage for "publicinbox.grokmanifest"
Eric Wong [Sun, 15 Sep 2019 04:20:04 +0000 (04:20 +0000)]
doc: update config manpage for "publicinbox.grokmanifest"

It's a bit of an esoteric option, but maybe somebody out
there can find it useful.

5 years agoqspawn: shorten lifetime of environ and opts args
Eric Wong [Sun, 15 Sep 2019 02:25:34 +0000 (02:25 +0000)]
qspawn: shorten lifetime of environ and opts args

We don't need to hold onto the subprocess environ and
redirects/options for popen_rd after spawning the child process.

I do not expect this to fix problem of leaking unlinked regular
file descriptors (which I still can't reproduce), and it
definitely does not fix the problem of leaking pipe descriptors
(which I also can't reproduce).

This will save an FD sooner on non-public-inbox-httpd servers
which give a non-FD $env->{'psgi.input'}, however

Regardless, it's good to free up memory resources in our own
process ASAP we're done using them.

5 years agoqspawn: clarify and improve error handling
Eric Wong [Sun, 15 Sep 2019 01:00:06 +0000 (01:00 +0000)]
qspawn: clarify and improve error handling

EINTR should not happen when using non-blocking sockets like we
do in our daemons, but maybe some OSes allow it to happen and
edge-triggered notifications won't notify us again.

So always retry immediately on EINTR without relying on kqueue
or epoll to notify us, and log any other unrecoverable errors
which may happen while we're at it.

5 years agot/httpd-corner: use which() sub for detecting curl(1)
Eric Wong [Sun, 15 Sep 2019 00:47:25 +0000 (00:47 +0000)]
t/httpd-corner: use which() sub for detecting curl(1)

We already import `which' for lsof(8), so we might as well
use it to detect curl(1), too.

5 years agodoc: update nntpd with NNTPS and STARTTLS examples
Eric Wong [Sat, 14 Sep 2019 18:28:54 +0000 (18:28 +0000)]
doc: update nntpd with NNTPS and STARTTLS examples

NNTPS and STARTTLS seems to be working for several months
without incident on news.public-inbox.org, so consider it a
success and maybe others can try using it.

HTTPS technically works, too, but isn't documented at
the moment since I can't recommend production deployments
without varnish protecting it.

5 years agot/httpd-corner: check for leaking FDs and pipes
Eric Wong [Fri, 13 Sep 2019 01:50:25 +0000 (01:50 +0000)]
t/httpd-corner: check for leaking FDs and pipes

-W0 (no workers) should not create any pipes on its own,
and we shouldn't have any deleted FDs if no clients are
connected.

This can find if leaks which may be triggered by PublicInbox::HTTP
(and not Qspawn or GitHTTPBackend).

5 years agoqspawn: remove unused WNOHANG import
Eric Wong [Sat, 14 Sep 2019 09:35:13 +0000 (09:35 +0000)]
qspawn: remove unused WNOHANG import

We rely on DS to do waitpid with WNOHANG, now, and the non-DS
code path won't use WNOHANG.

5 years agohttpd/async: improve naming and comments
Eric Wong [Sat, 14 Sep 2019 09:35:39 +0000 (09:35 +0000)]
httpd/async: improve naming and comments

Rename the {cleanup} field to {end}, since it's similar
to END {} and is consistent with the variable in Qspawn.pm

And document how certain subs get called, since we have
many subs named "new" and "close".

5 years agogithttpbackend: use REMOTE_ADDR for deleted identifier
Eric Wong [Thu, 12 Sep 2019 23:16:52 +0000 (23:16 +0000)]
githttpbackend: use REMOTE_ADDR for deleted identifier

REMOTE_HOST is not set by us (it is the reverse DNS name) of
REMOTE_ADDR, and there's few better ways to kill HTTP server
performance than to use standard name resolution APIs like
getnameinfo(3).

5 years agotmpfile: support O_APPEND and use it in DS::tmpio
Eric Wong [Thu, 12 Sep 2019 08:34:21 +0000 (08:34 +0000)]
tmpfile: support O_APPEND and use it in DS::tmpio

Might as well share some code for temporary file creation

5 years agotmpfile: give temporary files meaningful names
Eric Wong [Thu, 12 Sep 2019 08:34:20 +0000 (08:34 +0000)]
tmpfile: give temporary files meaningful names

Although we always unlink temporary files, give them a
meaningful name so that we can we can still make sense
of the pre-unlink name when using lsof(8) or similar
tools on Linux.

5 years agoqspawn: simplify by using PerlIO::scalar
Eric Wong [Sat, 14 Sep 2019 09:21:14 +0000 (09:21 +0000)]
qspawn: simplify by using PerlIO::scalar

I didn't know PerlIO::scalar existed until a few months ago,
but it's been distributed with Perl since 5.8 and doesn't
seem to be split out into it's own package on any distro.

5 years agoadmin: warn and ignore inaccessible inboxes
Eric Wong [Sat, 14 Sep 2019 09:21:05 +0000 (09:21 +0000)]
admin: warn and ignore inaccessible inboxes

For whatever reason, inbox directories can go missing
temporarily or permanently.  Tell the admin about them
and continue on our way.

5 years agosolvergit: don't drop update-index stdin with qspawn
Eric Wong [Thu, 12 Sep 2019 06:54:28 +0000 (06:54 +0000)]
solvergit: don't drop update-index stdin with qspawn

It's possible for Qspawn callers to be deferred, in which case
we must ensure we don't cause the temporary file used for
stdin to become unref-ed and closed.

This can be a problem when we exceed the default Qspawn
limiter of 32 concurrent processes for "git update-index".

5 years agodoc daemon: note the --listen directive is not always required
Eric Wong [Mon, 9 Sep 2019 06:42:39 +0000 (06:42 +0000)]
doc daemon: note the --listen directive is not always required

Users of socket activation don't need it, and hopefully other
init systems support it, too.

5 years agodoc edit: move =for comment after item
Eric Wong [Mon, 9 Sep 2019 05:53:16 +0000 (05:53 +0000)]
doc edit: move =for comment after item

Quiets down pod2man complaining

5 years agorun update-copyrights from gnulib for 2019
Eric Wong [Mon, 9 Sep 2019 05:43:19 +0000 (05:43 +0000)]
run update-copyrights from gnulib for 2019

5 years agodoc config: document indexlevel directive
Eric Wong [Mon, 9 Sep 2019 05:37:45 +0000 (05:37 +0000)]
doc config: document indexlevel directive

It was never documented, before.

5 years agotests: add tcp_connect() helper
Eric Wong [Mon, 2 Sep 2019 04:51:31 +0000 (04:51 +0000)]
tests: add tcp_connect() helper

IO::Socket::INET->new is rather verbose with the options hash,
extract it into a standalone sub

5 years agonntp: regexp always consumes rbuf if "\n" exists
Eric Wong [Sun, 8 Sep 2019 10:41:12 +0000 (10:41 +0000)]
nntp: regexp always consumes rbuf if "\n" exists

We don't want to get hung into a state where we see "\n" via
index(), yet cannot consume rbuf in the while loop.  So tweak
the regexp to ensure we always consume rbuf.

I suspect this is what causes occasional 100% CPU usage of
-nntpd, but reproducing it's been difficult..

5 years agonntp: fix redundant CRLF from "LISTGROUP GROUP RANGE"
Eric Wong [Sun, 8 Sep 2019 10:41:11 +0000 (10:41 +0000)]
nntp: fix redundant CRLF from "LISTGROUP GROUP RANGE"

Since Net::NNTP::listgroup doesn't support the range parameter,
I had to test this manually and noticed extra CRLF were emitted.

5 years agonntpdeflate: reduce overhead of idle clients
Eric Wong [Sun, 14 Jul 2019 02:56:36 +0000 (02:56 +0000)]
nntpdeflate: reduce overhead of idle clients

We don't need to keep an empty buffer around in the common case
when a client is sending us completely inflatable requests and
we're able to read them in one go.

This only seems to save about 2M with 10K NNTPS clients using
COMPRESS, so it's not a huge win, but better than nothing.

5 years agoTODO: remove done items, add some more
Eric Wong [Sat, 13 Jul 2019 21:54:55 +0000 (21:54 +0000)]
TODO: remove done items, add some more

It never ends...

5 years agonntp: support optional [range] arg in LISTGROUP
Eric Wong [Sat, 13 Jul 2019 21:42:46 +0000 (21:42 +0000)]
nntp: support optional [range] arg in LISTGROUP

RFC3977 6.1.2.2 LISTGROUP allows a [range] arg after [group],
and supporting it allows NNTP support in neomutt to work again.

Tested with NeoMutt 20170113 (1.7.2) on Debian stretch
(oldstable)

5 years agonntp: fix LIST OVERVIEW.FMT ordering and format
Eric Wong [Sat, 13 Jul 2019 21:38:11 +0000 (21:38 +0000)]
nntp: fix LIST OVERVIEW.FMT ordering and format

RFC3977 8.4.2 mandates the order of non-standard headers
to be after the first seven standard headers/metadata;
so "Xref:" must appear after "Lines:"|":lines".

Additionally, non-required header names must be followed
by ":full".

Cc: Jonathan Corbet <corbet@lwn.net>
Reported-by: Urs Janßen
<E1hmKBw-0008Bq-8t@akw>

5 years agonntpdeflate: stop relying on SUPER for ->do_read
Eric Wong [Sat, 13 Jul 2019 20:27:57 +0000 (20:27 +0000)]
nntpdeflate: stop relying on SUPER for ->do_read

We won't need further layering after enabling compression.  So
be explicit about which sub we're calling when we hit ->do_read
from NNTP and eliminate the need for the comment.

5 years agonntp: clear local timer on idle client expiry
Eric Wong [Fri, 12 Jul 2019 00:03:41 +0000 (00:03 +0000)]
nntp: clear local timer on idle client expiry

We need to ensure further timers can be registered if there's
currently no idle clients.

5 years agohttp|nntp: avoid recursion inside ->write
Eric Wong [Wed, 10 Jul 2019 06:13:59 +0000 (06:13 +0000)]
http|nntp: avoid recursion inside ->write

In HTTP.pm, we can use the same technique NNTP.pm uses with
long_response with the $long_cb callback and avoid storing
$pull in the per-client structure at all.  We can also reuse
the same logic to push the callback into wbuf from NNTP.

This does NOT introduce a new circular reference, but documents
it more clearly.

5 years agosolver: remove redundant spawn imports
Eric Wong [Fri, 5 Jul 2019 04:18:00 +0000 (04:18 +0000)]
solver: remove redundant spawn imports

We're using Qspawn, now

5 years agohttp|nntp: "use PublicInbox::DS" instead of ->import
Eric Wong [Mon, 8 Jul 2019 07:31:19 +0000 (07:31 +0000)]
http|nntp: "use PublicInbox::DS" instead of ->import

Relying on "use" to import during BEGIN means we get to take
advantage of prototype checking of function args during the rest
of the compilation phase.

5 years agohttpd: (cleanup) use reference instead of *glob
Eric Wong [Mon, 8 Jul 2019 07:22:31 +0000 (07:22 +0000)]
httpd: (cleanup) use reference instead of *glob

*glob notation isn't always necessary, and there's
no need to disable 'once' warnings, this way.

5 years agodaemon: use POSIX and WNOHANG more idiomatically
Eric Wong [Sun, 7 Jul 2019 03:49:19 +0000 (03:49 +0000)]
daemon: use POSIX and WNOHANG more idiomatically

No point in uglifying our code since we need the POSIX
module in many places, anyways.

5 years agoMerge remote-tracking branch 'origin/nntp-compress'
Eric Wong [Mon, 8 Jul 2019 07:11:50 +0000 (07:11 +0000)]
Merge remote-tracking branch 'origin/nntp-compress'

* origin/nntp-compress:
  nntp: improve error reporting for COMPRESS
  nntp: reduce memory overhead of zlib
  nntp: support COMPRESS DEFLATE per RFC 8054
  nntp: move LINE_MAX constant to the top
  nntp: use msg_more as a method

5 years agods: use WNOHANG with waitpid if inside event loop
Eric Wong [Mon, 8 Jul 2019 07:01:59 +0000 (07:01 +0000)]
ds: use WNOHANG with waitpid if inside event loop

While we're usually not stuck waiting on waitpid after
seeing a pipe EOF or even triggering SIGPIPE in the process
(e.g. git-http-backend) we're reading from, it MAY happen
and we should be careful to never hang the daemon process
on waitpid calls.

v2: use "eq" for string comparison against 'DEFAULT'

5 years agonntp: improve error reporting for COMPRESS
Eric Wong [Sun, 7 Jul 2019 06:57:43 +0000 (06:57 +0000)]
nntp: improve error reporting for COMPRESS

Add some checks for errors at initialization, though there's not
much that can be done with ENOMEM-type errors aside from
dropping clients.

We can also get rid of the scary FIXME for MemLevel=8.  It was a
stupid error on my part in the original per-client deflate
stream implementation calling C::R::Z::{Inflate,Deflate} in
array context and getting the extra dualvar error code as a
string result, causing the {zin}/{zout} array refs to have
extra array elements.

5 years agonntp: reduce memory overhead of zlib
Eric Wong [Fri, 5 Jul 2019 22:53:39 +0000 (22:53 +0000)]
nntp: reduce memory overhead of zlib

Using Z_FULL_FLUSH at the right places in our event loop, it
appears we can share a single zlib deflate context across ALL
clients in a process.

The zlib deflate context is the biggest factor in per-client
memory use, so being able to share that across many clients
results in a large memory savings.

With 10K idle-but-did-something NNTP clients connected to a
single process on a 64-bit system, TLS+DEFLATE used around
1.8 GB of RSS before this change.  It now uses around 300 MB.
TLS via IO::Socket::SSL alone uses <200MB in the same situation,
so the actual memory reduction is over 10x.

This makes compression less efficient and bandwidth increases
around 45% in informal testing, but it's far better than no
compression at all.  It's likely around the same level of
compression gzip gives on the HTTP side.

Security implications with TLS?  I don't know, but I don't
really care, either...  public-inbox-nntpd doesn't support
authentication and it's up to the client to enable compression.
It's not too different than Varnish caching gzipped responses
on the HTTP side and having responses go to multiple HTTPS
clients.

5 years agonntp: support COMPRESS DEFLATE per RFC 8054
Eric Wong [Fri, 5 Jul 2019 22:53:38 +0000 (22:53 +0000)]
nntp: support COMPRESS DEFLATE per RFC 8054

This is only tested so far with my patches to Net::NNTP at:
https://rt.cpan.org/Ticket/Display.html?id=129967

Memory use in C10K situations is disappointing, but that's
the nature of compression.

gzip compression over HTTPS does have the advantage of not
keeping zlib streams open when clients are idle, at the
cost of worse compression.

5 years agonntp: move LINE_MAX constant to the top
Eric Wong [Fri, 5 Jul 2019 22:53:37 +0000 (22:53 +0000)]
nntp: move LINE_MAX constant to the top

It'll be accessible from other places, and there was no real
point in having it declared inside a sub.

5 years agonntp: use msg_more as a method
Eric Wong [Fri, 5 Jul 2019 22:53:36 +0000 (22:53 +0000)]
nntp: use msg_more as a method

It's a tad slower, but we'll be able to subclass this to rely
on zlib deflate buffering.  This is advantageous for TLS clients
since (AFAIK) IO::Socket::SSL/OpenSSL doesn't give us ways to use
MSG_MORE or writev(2) like like GNUTLS does.

5 years agowatch: allow multiple spam watch directories
Eric Wong [Mon, 1 Jul 2019 02:18:48 +0000 (02:18 +0000)]
watch: allow multiple spam watch directories

Given most folks have multiple mail accounts, there's no reason
we can't support multiple Maildirs.

5 years agowatch: remove some indirectly-used imports
Eric Wong [Sun, 30 Jun 2019 22:56:34 +0000 (22:56 +0000)]
watch: remove some indirectly-used imports

We can drop some unnecessary imports and now that we switched
to InboxWritable.

5 years agoviewdiff: do not anchor using diffstat comments
Eric Wong [Fri, 5 Jul 2019 04:03:11 +0000 (04:03 +0000)]
viewdiff: do not anchor using diffstat comments

Diffstat summary comments were added to git last year and
we need to filter them out to get anchors working properly.

Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
  https://public-inbox.org/meta/20190704231123.GF20404@szeder.dev/

5 years agot/nntpd*.t: require IO::Socket::SSL 2.007 for Net::NNTP tests
Eric Wong [Mon, 1 Jul 2019 08:56:51 +0000 (08:56 +0000)]
t/nntpd*.t: require IO::Socket::SSL 2.007 for Net::NNTP tests

Net::NNTP won't attempt to use older versions of IO::Socket::SSL
because 2.007 is the "first version with default CA on most platforms"
according to comments in Net::NNTP.  But then again we don't make
remote requests when testing...

5 years agoqspawn: retry sysread when parsing headers, too
Eric Wong [Thu, 4 Jul 2019 10:02:06 +0000 (10:02 +0000)]
qspawn: retry sysread when parsing headers, too

We need to ensure the BIN_DETECT (8000 byte) check in
ViewVCS can be handled properly when sending giant
files.  Otherwise, EPOLLET won't notify us, again,
and responses can get stuck.

While we're at it, bump up the read-size up to 4096
bytes so we make fewer trips to the kernel.

5 years agoMerge remote-tracking branch 'origin/nntp'
Eric Wong [Sun, 30 Jun 2019 22:37:00 +0000 (22:37 +0000)]
Merge remote-tracking branch 'origin/nntp'

* origin/nntp:
  nntp: add support for CAPABILITIES command
  nntp: remove DISABLED hash checks

5 years agonntp: add support for CAPABILITIES command
Eric Wong [Sun, 30 Jun 2019 04:27:55 +0000 (04:27 +0000)]
nntp: add support for CAPABILITIES command

Some clients may rely on this for STARTTLS support.

5 years agonntp: remove DISABLED hash checks
Eric Wong [Sun, 30 Jun 2019 01:00:59 +0000 (01:00 +0000)]
nntp: remove DISABLED hash checks

Before I figured out the long_response API, I figured there'd
be expensive, process-monopolizing commands which admins might
want to disable.  Nearly 4 years later, we've never needed it
and running a server without commands such as OVER/XOVER is
unimaginable.

5 years agot/httpd-unix.t: avoid race in between bind() and listen()
Eric Wong [Sun, 30 Jun 2019 22:32:32 +0000 (22:32 +0000)]
t/httpd-unix.t: avoid race in between bind() and listen()

We need to be able to successfully connect() to the socket
before attempting further tests.  Merely testing for the
existence of a socket isn't enough, since the server may've
only done bind(), not listen().

5 years agodaemon: warn on inheriting blocking listeners
Eric Wong [Sun, 30 Jun 2019 22:19:39 +0000 (22:19 +0000)]
daemon: warn on inheriting blocking listeners

For users relying on socket activation via service manager (e.g.
systemd) and running multiple service instances (@1, @2),
we need to ensure configuration of the socket is NonBlocking.
Otherwise, service managers such as systemd may clear the
O_NONBLOCK flag for a small window where accept/accept4
blocks:

public-inbox-nntpd@1      |systemd         |public-inbox-nntpd@2
--------------------------+----------------+--------------------
F_SETFL,O_NONBLOCK|O_RDWR |                | (not running, yet)
                          |F_SETFL, O_RDWR |
                          |fork+exec @2... |
accept(...) # blocks!     |                |(started by systemd)
                          |                |F_SETFL,O_NONBLOCK|O_RDWR
                          |                |accept(...) non-blocking

It's a very small window where O_NONBLOCK can be cleared,
but it exists, and I finally hit it after many years.

5 years agotests: common tcp_server and unix_server helpers
Eric Wong [Sun, 30 Jun 2019 22:19:38 +0000 (22:19 +0000)]
tests: common tcp_server and unix_server helpers

IO::Socket:*->new options are verbose and we can save
a bunch of code by putting this into t/common.perl,
since the related spawn_listener stuff is already there.

5 years agot/perf-nntpd.t: fix off-by-one if NEWNEWS_DATE is unset
Eric Wong [Sun, 30 Jun 2019 22:24:25 +0000 (22:24 +0000)]
t/perf-nntpd.t: fix off-by-one if NEWNEWS_DATE is unset

20190431 isn't real, NNTP.pm failed to parse it when our
test client sent it.

5 years agoMerge remote-tracking branch 'origin/email-simple-mem' into master
Eric Wong [Sun, 30 Jun 2019 17:13:30 +0000 (17:13 +0000)]
Merge remote-tracking branch 'origin/email-simple-mem' into master

* origin/email-simple-mem:
  nntp: reduce syscalls for ARTICLE and BODY
  mbox: split header and body processing
  mbox: use Email::Simple->new to do in-place modifications
  nntp: rework and simplify art_lookup response

5 years agoexamples/*@.service: sockets MUST be NonBlocking
Eric Wong [Sun, 30 Jun 2019 07:11:00 +0000 (07:11 +0000)]
examples/*@.service: sockets MUST be NonBlocking

For users running multiple (-nntpd@1, -nntpd@2) instances of
either -httpd or -nntpd via systemd to implement zero-downtime
restarts; it's possible for a listen socket to become blocking
for a moment during an accept syscall and cause a daemons to
get stuck in a blocking accept() during
PublicInbox::Listener::event_step (event_read in previous
versions).

Since O_NONBLOCK is a file description flag, systemd clearing
O_NONBLOCK momentarily (before PublicInbox::Listener::new
re-enables it) creates a window for another instance of our
daemon to get stuck in accept().

cf. systemd.service(5)

5 years agods: fix return values of do_read and *_tls_step
Eric Wong [Sun, 30 Jun 2019 05:15:56 +0000 (05:15 +0000)]
ds: fix return values of do_read and *_tls_step

We need to ensure all these subroutines return false on
incomplete.

5 years agods: rely on refcounting to close descriptors
Eric Wong [Sun, 30 Jun 2019 03:54:26 +0000 (03:54 +0000)]
ds: rely on refcounting to close descriptors

Since we have EPOLL_CTL_DEL implemented for the poll(2) and
kqueue backends, we can rely on Perl refcounting to gently
close(2) the underlying file descriptors as references get
dropped.

This may be beneficial in the future if we want to drop a
descriptor from the event loop without actually closing it.

5 years agot/nntpd*.t: skip TLS tests for old Net::NNTP
Eric Wong [Sun, 30 Jun 2019 00:01:55 +0000 (00:01 +0000)]
t/nntpd*.t: skip TLS tests for old Net::NNTP

Perl prior to 5.22 did not bundle a Net::NNTP (or libnet)
capable of handling TLS.

5 years agodskqxs: more closely match epoll semantics
Eric Wong [Sat, 29 Jun 2019 20:19:29 +0000 (20:19 +0000)]
dskqxs: more closely match epoll semantics

EV_DISPATCH is actually a better match for EPOLLONESHOT
semantics than EV_ONESHOT in that it doesn't require EV_ADD
for every mod operation.

Blindly using EV_ADD everywhere forces the FreeBSD kernel to
do extra allocations up front, so it's best avoided.

5 years agohttp: use bigger, but shorter-lived buffers for pipes
Eric Wong [Fri, 28 Jun 2019 19:26:56 +0000 (19:26 +0000)]
http: use bigger, but shorter-lived buffers for pipes

Linux pipes default to 65536 bytes in size, and we want to read
external processes as fast as possible now that we don't use
Danga::Socket or buffer to heap.

However, drop the buffer ASAP if we need to wait on anything;
since idle buffers can be idle for eons.  This lets other
execution contexts can reuse that memory right away.

5 years agohttpd/async: switch to buffering-as-fast-as-possible
Eric Wong [Fri, 28 Jun 2019 18:58:36 +0000 (18:58 +0000)]
httpd/async: switch to buffering-as-fast-as-possible

With DS buffering to a temporary file nowadays, applying
backpressure to git-http-backend(1) hurts overall memory
usage of the system.  Instead, try to get git-http-backend(1)
to finish as quickly as possible and use edge-triggered
notifications to reduce wakeups on our end.

5 years agoparentpipe: make the ->close call more obvious
Eric Wong [Fri, 28 Jun 2019 18:55:09 +0000 (18:55 +0000)]
parentpipe: make the ->close call more obvious

We can close directly in event_step without bad side effects,
and then we also don't need to take a reason arg from worker_quit,
since we weren't logging it anywhere.

5 years agoparentpipe: document and use one-shot wakeups
Eric Wong [Fri, 28 Jun 2019 18:46:10 +0000 (18:46 +0000)]
parentpipe: document and use one-shot wakeups

The master process only dies once and we close ourselves right
away.  So it doesn't matter if it's level-triggered or
edge-triggered, actually, but one-shot is most consistent with
our use and keeps the kernel from doing extra work.

5 years agohttp: support HTTPS (kinda)
Eric Wong [Fri, 28 Jun 2019 08:18:51 +0000 (08:18 +0000)]
http: support HTTPS (kinda)

It's barely any effort at all to support HTTPS now that we have
NNTPS support and can share all the code for writing daemons.

However, we still depend on Varnish to avoid hug-of-death
situations, so supporting reverse-proxying will be required.

5 years agods: consolidate IO::Socket::SSL checks
Eric Wong [Fri, 28 Jun 2019 07:37:26 +0000 (07:37 +0000)]
ds: consolidate IO::Socket::SSL checks

We need to be careful about handling EAGAIN on write(2)
failures deal with SSL_WANT_READ vs SSL_WANT_WRITE as
appropriate.

5 years agods: handle deferred DS->close after timers
Eric Wong [Fri, 28 Jun 2019 05:25:40 +0000 (05:25 +0000)]
ds: handle deferred DS->close after timers

Our hacks in EvCleanup::next_tick and EvCleanup::asap were due
to the fact "closed" sockets were deferred and could not wake
up the event loop, causing certain actions to be delayed until
an event fired.

Instead, ensure we don't sleep if there are pending sockets to
close.

We can then remove most of the EvCleanup stuff

While we're at it, split out immediate timer handling into a
separate array so we don't need to deal with time calculations
for the event loop.

5 years agolistener: use edge-triggered notifications
Eric Wong [Thu, 27 Jun 2019 21:21:05 +0000 (21:21 +0000)]
listener: use edge-triggered notifications

We don't need extra wakeups from the kernel when we know a
listener is already active.

5 years agohttp: use requeue instead of watch_in1
Eric Wong [Thu, 27 Jun 2019 21:21:04 +0000 (21:21 +0000)]
http: use requeue instead of watch_in1

Don't use epoll or kqueue to watch for anything unless we hit
EAGAIN, since we don't know if a socket is SSL or not.

5 years agods: move requeue logic over from NNTP
Eric Wong [Thu, 27 Jun 2019 21:21:03 +0000 (21:21 +0000)]
ds: move requeue logic over from NNTP

We'll be reusing requeue in other places to reduce trips to
the kernel to retrieve "hot" descriptors.

5 years agods: share lazy rbuf handling between HTTP and NNTP
Eric Wong [Wed, 26 Jun 2019 08:11:10 +0000 (08:11 +0000)]
ds: share lazy rbuf handling between HTTP and NNTP

Doing this for HTTP cuts the memory usage of 10K
idle-after-one-request HTTP clients from 92 MB to 47 MB.

The savings over the equivalent NNTP change in commit
6f173864f5acac89769a67739b8c377510711d49,
("nntp: lazily allocate and stash rbuf") seems down to the
size of HTTP requests and the fact HTTP is a client-sends-first
protocol where as NNTP is server-sends-first.

5 years agot/ds-leak: fix race
Eric Wong [Sat, 29 Jun 2019 06:34:40 +0000 (06:34 +0000)]
t/ds-leak: fix race

We need to ensure we run lsof on the sleep(1) process, and not
the fork of ourselves before execve(2).  This race applies when
we're using the default pure-Perl spawn() implementation.

5 years agowatchmaildir: show the current path on spamcheck failures
Eric Wong [Wed, 26 Jun 2019 09:00:43 +0000 (09:00 +0000)]
watchmaildir: show the current path on spamcheck failures

Knowing which message failed a spam check is tough when I have
many Maildirs and don't have a search indexing tool setup for
spam mail.

5 years agonntp: reduce syscalls for ARTICLE and BODY
Eric Wong [Thu, 27 Jun 2019 17:31:30 +0000 (17:31 +0000)]
nntp: reduce syscalls for ARTICLE and BODY

Chances are we already have extra buffer space following the
expensive LF => CRLF conversion that we can safely append an
extra CRLF in those places without incurring a copy of the
full string buffer.

While we're at it, document where our pain points are in terms
of memory usage, since tracking/controlling memory use isn't
exactly obvious in high-level languages.

Perhaps we should start storing messages in git as CRLF...

5 years agombox: split header and body processing
Eric Wong [Thu, 27 Jun 2019 09:23:39 +0000 (09:23 +0000)]
mbox: split header and body processing

When dealing with ~30MB messages, we can save another ~30MB by
splitting the header and body processing and not appending the
body string back to the header.

We'll rely on buffering in gzip or kernel (via MSG_MORE)
to prevent silly packet sizes.

5 years agombox: use Email::Simple->new to do in-place modifications
Eric Wong [Thu, 27 Jun 2019 08:40:19 +0000 (08:40 +0000)]
mbox: use Email::Simple->new to do in-place modifications

Email::Simple->new will split the head from the body in-place,
and we can avoid using Email::Simple::body.  This saves us from
holding an extra copy of the message in memory, and saves us
around ~30MB when operating on ~30MB messages.

5 years agonntp: rework and simplify art_lookup response
Eric Wong [Thu, 27 Jun 2019 04:11:22 +0000 (04:11 +0000)]
nntp: rework and simplify art_lookup response

We don't need some of the array elements returned from
art_lookup, anymore (and haven't used them in years).

We can also shorten the lifetime of the Email::Simple object by
relying on the fact Email::Simple->new will modify it's arg if
given a SCALARREF and allow us to avoid Email::Simple::body
calls.

Unfortunately, this doesn't seem to provide any noticeable
improvement in memory usage when dealing with a 30+ MB test
message, since our previous use of ->body_set('') was saving
some memory, but forcing a LF-only body to be CRLF was making
Perl allocate extra space for s///sg.

5 years agocerts/create-certs: create certs in 'certs/' directory
Eric Wong [Mon, 24 Jun 2019 08:16:43 +0000 (08:16 +0000)]
certs/create-certs: create certs in 'certs/' directory

If running in our top-level source tree, use the 'certs/'
directory as  the prefix so we can just invoke `./certs/create-certs.perl'
instead of `(cd certs && ./create-certs.perl)'

5 years agods: cleanup poll test and avoid clobbering imports
Eric Wong [Wed, 26 Jun 2019 07:58:15 +0000 (07:58 +0000)]
ds: cleanup poll test and avoid clobbering imports

On Linux systems with epoll support, we don't want to be
clobbering defined subs in the t/ds-poll.t test; so use
OO ->method dispatch instead and require users to explicitly
import subs via EXPORT_OK.

5 years agoMerge remote-tracking branch 'origin/nntp-tls'
Eric Wong [Wed, 26 Jun 2019 06:36:27 +0000 (06:36 +0000)]
Merge remote-tracking branch 'origin/nntp-tls'

* origin/nntp-tls: (59 commits)
  ds: ->write must not clobber empty wbuf array
  Makefile: skip DSKQXS in global syntax check
  ds: reduce overhead of tempfile creation
  Revert "ci: require IO::KQueue on FreeBSD, for now"
  ds: reimplement IO::Poll support to look like epoll
  ds: split out IO::KQueue-specific code
  daemon: use FreeBSD accept filters on non-NNTP
  daemon: set TCP_DEFER_ACCEPT on everything but NNTP
  nntp: send greeting immediately for plain sockets
  ci: require IO::KQueue on FreeBSD, for now
  nntp: lazily allocate and stash rbuf
  ds: flush_write runs ->write callbacks even if closed
  nntp: simplify long response logic and fix nesting
  ds: always use EV_ADD with EV_SET
  nntp: reduce allocations for greeting
  ds: allow ->write callbacks to syswrite directly
  daemon: use SSL_MODE_RELEASE_BUFFERS
  t/nntpd-tls: slow client connection test
  nntp: call SSL_shutdown in normal cases
  ds|nntp: use CORE::close on socket
  ...

5 years agosearchview: avoid displaying full paths on errors
Eric Wong [Tue, 25 Jun 2019 04:08:14 +0000 (04:08 +0000)]
searchview: avoid displaying full paths on errors

Displaying full path names of installed modules could expose
unnecessary information about user home directory names or other
potentially sensitive information.  However, displaying a module
name could still be useful for diagnosing problems, so map full
paths to the relevant part of the path name which is relevant to
the package name.

Reported-by: Ali Alnubani <alialnu@mellanox.com>
  https://public-inbox.org/meta/20190611193815.c4uovtlp574bid6x@dcvr/

5 years agomsgmap: mid_insert: use plain "INSERT" to detect duplicates
Eric Wong [Mon, 24 Jun 2019 23:29:37 +0000 (23:29 +0000)]
msgmap: mid_insert: use plain "INSERT" to detect duplicates

"INSERT OR IGNORE" still bumps the auto-increment counter in
SQLite, which causes gaps to appear in NNTP article numbering.

This bug appeared in v2 repos where V2Writable may call ->add
repeatedly on the same message.  This bug is apparent with
public-inbox-watch and work-in-progress IMAP watchers which may
rescan and (attempt to) reinsert the same message on mailbox
changes.

Most uses of public-inbox-mda were not affected, unless the
same message is actually delivered multiple times to the mda.
v1 is not affected, either, since deduplication is only based
on Message-ID and msgmap never sees the duplicate.

Reported-by: "Eric W. Biederman" <ebiederm@xmission.com>
5 years agods: ->write must not clobber empty wbuf array
Eric Wong [Mon, 24 Jun 2019 18:18:18 +0000 (18:18 +0000)]
ds: ->write must not clobber empty wbuf array

We need to account for ->write(CODE) calls doing ->write(SCALARREF),
otherwise flush_write may see the wrong ->{wbuf} field.

5 years agoMakefile: skip DSKQXS in global syntax check
Eric Wong [Mon, 24 Jun 2019 04:06:42 +0000 (04:06 +0000)]
Makefile: skip DSKQXS in global syntax check

IO::KQueue isn't easily installable on Linux systems.

5 years agods: reduce overhead of tempfile creation
Eric Wong [Mon, 24 Jun 2019 02:52:58 +0000 (02:52 +0000)]
ds: reduce overhead of tempfile creation

We end up buffering giant things to the FS sometimes, and open()
is not a cheap syscall; so being forced to do it twice to get a
file description with O_APPEND is gross when we can just use
O_EXCL ourselves and loop on EEXIST.

5 years agoRevert "ci: require IO::KQueue on FreeBSD, for now"
Eric Wong [Mon, 24 Jun 2019 02:52:57 +0000 (02:52 +0000)]
Revert "ci: require IO::KQueue on FreeBSD, for now"

Now that we support IO::Poll once again, we can remove
the IO::KQueue requirement.

5 years agods: reimplement IO::Poll support to look like epoll
Eric Wong [Mon, 24 Jun 2019 02:52:56 +0000 (02:52 +0000)]
ds: reimplement IO::Poll support to look like epoll

At least the subset of epoll we use.  EPOLLET might be
difficult to emulate if we end up using it.

5 years agods: split out IO::KQueue-specific code
Eric Wong [Mon, 24 Jun 2019 02:52:55 +0000 (02:52 +0000)]
ds: split out IO::KQueue-specific code

We don't need to code multiple event loops or have branches in
watch() if we can easily make the IO::KQueue-based interface
look like our lower-level epoll_* API.

5 years agodaemon: use FreeBSD accept filters on non-NNTP
Eric Wong [Mon, 24 Jun 2019 02:52:54 +0000 (02:52 +0000)]
daemon: use FreeBSD accept filters on non-NNTP

Similar to TCP_DEFER_ACCEPT on Linux, FreeBSD has a 'dataready'
accept filter which we can use to reduce wakeups when doing
TLS negotiation or plain HTTP.  There's also a 'httpready'
which we can use for plain HTTP connections.

5 years agodaemon: set TCP_DEFER_ACCEPT on everything but NNTP
Eric Wong [Mon, 24 Jun 2019 02:52:53 +0000 (02:52 +0000)]
daemon: set TCP_DEFER_ACCEPT on everything but NNTP

This Linux-specific option can save us some wakeups during
the TLS negotiation phase, and it can help with ordinary HTTP,
too.

Plain NNTP (and in the future, POP3) are the only things which
require the server send messages, first.

5 years agonntp: send greeting immediately for plain sockets
Eric Wong [Mon, 24 Jun 2019 02:52:52 +0000 (02:52 +0000)]
nntp: send greeting immediately for plain sockets

A tiny write() for the greeting on a just accept()-ed TCP socket
won't fail with EAGAIN, so we can avoid the extra epoll syscall
traffic with plain sockets.

5 years agoci: require IO::KQueue on FreeBSD, for now
Eric Wong [Mon, 24 Jun 2019 02:52:51 +0000 (02:52 +0000)]
ci: require IO::KQueue on FreeBSD, for now

We'll likely replace IO::KQueue (at least on FreeBSD) using
a pure-Perl syscall()-based version since syscall numbers are
consistent across architectures on FreeBSD and easy to maintain.

IO::KQueue->EV_SET is also shockingly inefficient in that it
calls kqueue() as much as epoll_ctl.

5 years agonntp: lazily allocate and stash rbuf
Eric Wong [Mon, 24 Jun 2019 02:52:50 +0000 (02:52 +0000)]
nntp: lazily allocate and stash rbuf

Allocating a per-client buffer up front is unnecessary and
wastes a hash slot.  For the majority of (non-malicious)
clients, we won't need to store rbuf in a long-lived object
associated with a client socket at all.

This saves around 10M on 64-bit with 20K connected-but-idle
clients.

5 years agods: flush_write runs ->write callbacks even if closed
Eric Wong [Mon, 24 Jun 2019 02:52:49 +0000 (02:52 +0000)]
ds: flush_write runs ->write callbacks even if closed

We may need to rely on cleanup code running in enqueued
callbacks, so ensure we call it when flush_write happens.

5 years agonntp: simplify long response logic and fix nesting
Eric Wong [Mon, 24 Jun 2019 02:52:48 +0000 (02:52 +0000)]
nntp: simplify long response logic and fix nesting

We can get rid of the {long_res} field and reuse the write
buffer ordering logic to prevent nesting of responses from
requeue.

On FreeBSD, this fixes a problem of callbacks firing twice
because kqueue as event_step is now our only callback entry
point.

There's a slight change in the stdout "logging" format, in
that we can no longer distinguish between writes blocked
due to slow clients or deferred long responses.  Not sure
if this affects anybody parsing logs or not, but preserving
the old format could prove expensive and not worth the
effort.

5 years agods: always use EV_ADD with EV_SET
Eric Wong [Mon, 24 Jun 2019 02:52:47 +0000 (02:52 +0000)]
ds: always use EV_ADD with EV_SET

kqueue EV_ONESHOT semantics are different than epoll
EPOLLONESHOT.  epoll only disables watches for that event while
keeping the item in the rbtree for future EPOLL_CTL_MOD.  kqueue
removes the watch from the filter set entirely, necessitating
the use of EV_ADD for future modifications.

5 years agonntp: reduce allocations for greeting
Eric Wong [Mon, 24 Jun 2019 02:52:46 +0000 (02:52 +0000)]
nntp: reduce allocations for greeting

No need to allocate a new PerlIO::scalar filehandle for every
client, instead we can now pass the same CODE reference which
calls DS->write on a reused string reference.