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...
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.
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.
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().
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:
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.
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
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().
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.
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.
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.
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.
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.
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.
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.
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.
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...
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.
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.
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.
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)'
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.
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
...
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/
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>
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.
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.
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.
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.
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.
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.
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.
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.
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.
Eric Wong [Mon, 24 Jun 2019 02:52:44 +0000 (02:52 +0000)]
daemon: use SSL_MODE_RELEASE_BUFFERS
34K per idle connection adds up to large amounts of memory;
especially with the speed of malloc nowadays compared to the
cost of cache misses or worse, swapping.
Eric Wong [Mon, 24 Jun 2019 02:52:43 +0000 (02:52 +0000)]
t/nntpd-tls: slow client connection test
We need to ensure slowly negotiating TLS clients don't block
the event loop. This is why I added the size check of
{wbuf} before and after calling the CODE ref in DS::flush_write.
Eric Wong [Mon, 24 Jun 2019 02:52:42 +0000 (02:52 +0000)]
nntp: call SSL_shutdown in normal cases
This is in accordance with TLS standards and will be needed
to support session caching/reuse in the future. However, we
don't issue shutdown(2) since we know not to inadvertantly
share our sockets with other processes.
Eric Wong [Mon, 24 Jun 2019 02:52:41 +0000 (02:52 +0000)]
ds|nntp: use CORE::close on socket
IO::Socket::SSL will try to re-bless back to the original class
on TLS negotiation failure. Unfortunately, the original class
is 'GLOB', and re-blessing to 'GLOB' takes away all the IO::Handle
methods, because Filehandle/IO are a special case in Perl5.
Anyways, since we already use syswrite() and sysread() as functions
on our socket, we might as well use CORE::close(), as well (and
it plays nicely with tied classes).
Eric Wong [Mon, 24 Jun 2019 02:52:40 +0000 (02:52 +0000)]
daemon: map inherited sockets to well-known schemes
I don't want to specify "--listen" in my systemd .service files,
so map 563 to NNTPS automatically (and 443 to HTTPS, but HTTPS
support doesn't work, yet).
Eric Wong [Mon, 24 Jun 2019 02:52:38 +0000 (02:52 +0000)]
nntp: NNTPS and NNTP+STARTTLS working
It kinda, barely works, and I'm most happy I got it working
without any modifications to the main NNTP::event_step callback
thanks to the DS->write(CODE) support we inherited from
Danga::Socket.
Eric Wong [Mon, 24 Jun 2019 02:52:37 +0000 (02:52 +0000)]
nntp: wait for writability before sending greeting
This will be needed for NNTPS support, since we need
to negotiate the TLS connection before writing the
greeting and we can reuse the existing buffer layer
to enqueue writes.
Eric Wong [Mon, 24 Jun 2019 02:52:36 +0000 (02:52 +0000)]
ds: deal better with FS-related errors IO buffers
Instead of ENOMEM (or fragmentation/swap storms), using tempfile
buffers opens us up to filesystem and storage-related errors
(e.g. ENOSPC, EFBIG, EIO, EROFS). Log these errors, drop the
particular client, and try to limp by with whateve we have left.
Eric Wong [Mon, 24 Jun 2019 02:52:35 +0000 (02:52 +0000)]
allow use of PerlIO layers for filesystem writes
It may make sense to use PerlIO::mmap or PerlIO::scalar for
DS write buffering with IO::Socket::SSL or similar (since we can't
use MSG_MORE), so that means we need to go through buffering
in userspace for the common case; while still being easily
compatible with slow clients.
And it also simplifies GitHTTPBackend slightly.
Maybe it can make sense for HTTP input buffering, too...
Eric Wong [Mon, 24 Jun 2019 02:52:34 +0000 (02:52 +0000)]
nntp: simplify re-arming/requeue logic
We can be smarter about requeuing clients to run and avoid
excessive epoll_ctl calls since we can trust event_step to do
the right thing depending on the state of the client.
Eric Wong [Mon, 24 Jun 2019 02:52:27 +0000 (02:52 +0000)]
ds: favor `delete' over assigning fields to `undef'
This is cleaner in most cases and may allow Perl to reuse memory
from unused fields.
We can do this now that we no longer support Perl 5.8; since
Danga::Socket was written with struct-like pseudo-hash support
in mind, and Perl 5.9+ dropped support for pseudo-hashes over
a decade ago.
Eric Wong [Mon, 24 Jun 2019 02:52:21 +0000 (02:52 +0000)]
ds: remove IO::Poll support (for now)
It may be reinstated at a later time if there's interest; but I
want to be able to use one-shot notifications for certain events
while retaining level-triggered notifications others.
OTOH, I intend to fully support kqueue; via IO::KQueue for now,
but via syscall() eventually to take advantage of the syscall
reduction kevent(2) can provide over (current) epoll APIs.
Eric Wong [Mon, 24 Jun 2019 02:52:20 +0000 (02:52 +0000)]
ds: share watch_chg between watch_read/watch_write
There was much duplicate logic between watch_read and
watch_write. Share that logic, and give us room to enable
edge-triggered or one-shot notifications in the future.
Eric Wong [Mon, 24 Jun 2019 02:52:16 +0000 (02:52 +0000)]
syscall: get rid of unused EPOLL* constants
EPOLLRDBAND is used for DECnet; and I'm pretty sure I won't be
updating any of our code to work with DECnet.
I've never found use for EPOLLHUP or EPOLLERR, either; so
disable those for now and add comments for things I might
actually use: EPOLLET and EPOLLONESHOT.
Eric Wong [Mon, 24 Jun 2019 02:52:15 +0000 (02:52 +0000)]
ds: get rid of redundant and unnecessary POLL* constants
EPOLL* constants already match their POLL* counterparts and
there's no way Linux can ever diverge or change the values
of those constants. So we'll favor the EPOLL* ones since we
use EPOLLEXCLUSIVE, already.
For weird stuff like kqueue, we'd need to keep maintaining
the mapping, anyways.
Eric Wong [Mon, 24 Jun 2019 02:52:08 +0000 (02:52 +0000)]
ds: split out from ->flush_write and ->write
Get rid of the confusing $need_queue variable and all
the associated documentation for it. Instead, make it
obvious that we're either skipping the write buffer or
flushing the write buffer by splitting the sub in two.
Eric Wong [Mon, 24 Jun 2019 02:52:05 +0000 (02:52 +0000)]
AddTimer: avoid clock_gettime for the '0' case
We rely on immediate timers often, so we can avoid the overhead
of an extra subroutine call to retrieve the monotonic time (and
a sometimes-system call on some platforms).
Eric Wong [Sun, 23 Jun 2019 17:42:05 +0000 (17:42 +0000)]
manifest: v2 epoch descriptions based on inbox->description
The default $GIT_DIR/description (provided by git.git templates)
isn't very useful for v2 epochs, so use the inbox description
and suffix it with the epoch number if it's otherwise unnamed.
Requested-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>
https://public-inbox.org/meta/20190620190017.GA27175@chatter.i7.local/