Date | Commit message (Collapse) |
|
We'll be reusing requeue in other places to reduce trips to
the kernel to retrieve "hot" descriptors.
|
|
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.
|
|
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...
|
|
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.
|
|
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.
|
|
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.
|
|
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.
|
|
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.
|
|
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.
|
|
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).
|
|
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.
|
|
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.
|
|
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.
|
|
Both NNTP and HTTP have common needs and we can factor
out some common code to make dealing with IO::Socket::SSL
easier.
|
|
It should not matter because our rbuf is always from
a socket without encoding layers, but this makes things
easier to follow.
|
|
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.
|
|
Integer comparisions of "$!" are faster than hash lookups.
See commit 6fa2b29fcd0477d126ebb7db7f97b334f74bbcbc
("ds: cleanup Errno imports and favor constant comparisons")
for benchmarks.
|
|
We don't need to keep track of that field since we always
know what events we're interested in when using one-shot
wakeups.
|
|
We can avoid the EPOLL_CTL_ADD && EPOLL_CTL_MOD sequence with
a single EPOLL_CTL_ADD.
|
|
No sense in having similar Linux-specific functionality in
both our NNTP.pm and HTTP.pm
|
|
We don't need write buffering unless we encounter slow clients
requesting large responses. So don't waste a hash slot or
(empty) arrayref for it.
|
|
All of our internal timing code should use monotonic clocks
for consistency against system clock adjustments.
This can be shared by our Daemon and NNTP packages.
|
|
Merely checking the presence of the {sock} field is
enough, and having multiple sources of truth increases
confusion and the likelyhood of bugs.
|
|
Having separate read/write callbacks in every class is too
confusing to my easily-confused mind. Instead, give every class
an "event_step" callback which is easier to wrap my head around.
This will make future code to support IO::Socket::SSL-wrapped
sockets easier-to-digest, since SSL_write() can require waiting
on POLLIN events, and SSL_read() can require waiting on POLLOUT
events.
|
|
* origin/ds:
ds: stop caring about event flags set by epoll/poll/kqueue
ds: do not distinguish between POLLHUP and POLLERR
ds: remove read method, here, too
nntp: use sysread to append to existing buffer
ds: remove steal_socket method
ds: remove {fd} field
ds: reduce Errno imports and drop ->close reason
ds: cleanup Errno imports and favor constant comparisons
ds: simplify write buffer accounting
|
|
It's the unfortunate reality that there are some clients which
reuse Message-IDs (in which we generate + use another) or set
multiple Message-IDs on their own. While the v2 format
addresses that, NNTP clients such as leafnode are not always
prepared to deal with that case.
So, ensure NNTP clients only see a single Message-ID, and
show the others as 'X-Alt-Message-ID'.
|
|
Leafnode cannot handle Message-ID headers which are too long and
require folding via Email::Simple::Header. Since there are
already many of these messages in git with the header already
folded, we need to handle the unfolding when emitting the
message via NNTP.
As far as we know, Leafnode is the only client software
incapable of handling this case.
|
|
Apparently leafnode just needs any junk in the Path: header.
Lets not waste bandwidth and just use a single byte to keep
leafnode happy.
Cc: Dave Taht <dave@taht.net>
|
|
In my experience, both are worthless as any normal read/write
call path will be wanting to check errors and deal with them
appropriately; so we can just call event_read, for now.
Eventually, there'll probably be only one callback for dealing
with all in/out/err/hup events to simplify logic, especially w.r.t
TLS socket negotiation.
|
|
We already do this in PublicInbox::HTTP, as it's superior to
DS::read in this regard. Initially (when I started writing
NNTP.pm, I wanted to use Danga::Socket's read buffering and
push_back_read (removed in DS) but quickly figured out it wasn't
useful at all for dealing with trickling clients.
|
|
Storing the file descriptor was redundant as we can quickly call
fileno($self->{sock}) and not have to store an extra hash table
entry. Multiple sources of truth leads to confusion, confusion
leads to bugs.
|
|
Keeping track of write_buf_size was redundant and pointless when
we can simply check the number of elements in the buffer array.
Multiple sources of truth leads to confusion; confusion leads to
bugs.
Finally, rename the prefixes to 'wbuf' to ensure we loudly
(instead of silently) break any external dependencies being
ported over from Danga::Socket, as further changes are pending.
|
|
RFC3977 does not have provisions for whitespace beyond ASCII
TAB, SP, CR and LF. I doubt there's any NNTP clients broken
enough to be sending non-ASCII whitespace delimiters.
We're probably excessively liberal regarding TAB acceptance,
even; but it's probably too late to change at this point...
|
|
We aren't able to make sense of non-ASCII digits
cf. perlrecharclass(1) / "Digits" section
|
|
None of the NNTP code actually relies on Xapian, anymore.
|
|
* origin/danga-bundle:
DS: epoll: fix misordered EPOLL_CTL_DEL call
DS: drop unused "_undef" sub
syscall: drop readahead wrapper
build: do not manify DS and Syscall pods
DS: handle EINTR in IO::Poll path, too
DS: workaround IO::Kqueue EINTR (mis-)handling
DS: drop profiling support
DS: remove unused fields and functions
listener: use EPOLLEXCLUSIVE for listen sockets
bundle Danga::Socket and Sys::Syscall
|
|
We'll ignore blank lines from clients, since that's what innd
seems to do.
|
|
These modules are unmaintained upstream at the moment, but I'll
be able to help with the intended maintainer once/if CPAN
ownership is transferred. OTOH, we've been waiting for that
transfer for several years, now...
Changes I intend to make:
* EPOLLEXCLUSIVE for Linux
* remove unused fields wasting memory
* kqueue bugfixes e.g. https://rt.cpan.org/Ticket/Display.html?id=116615
* accept4 support
And some lower priority experiments:
* switch to EV_ONESHOT / EPOLLONESHOT (incompatible changes)
* nginx-style buffering to tmpfile instead of string array
* sendfile off tmpfile buffers
* io_uring maybe?
|
|
Unused since commit b8c41362f2a5c8fcc6b1846a79c72bfa77565297
("nntp: simplify the long_response API")
|
|
Alpine is apparently stricter than other clients I've tried
w.r.t. using CRLF for headers. So do the same thing we do for
bodies to ensure we only emit CRLFs and no bare LFs.
Reported-by: Wang Kang <i@scateu.me>
https://public-inbox.org/meta/alpine.DEB.2.21.99.1901161043430.29788@la.scateu.me/
|
|
do_write must return 0 or 1.
|
|
When a client starts pipelining requests to us which trigger
long responses, we need to keep socket readiness checks disabled
and only enable them when our socket rbuf is drained.
Failure to do this caused aborted clients with
"BUG: nested long response" when Danga::Socket calls event_read
for read-readiness after our "next_tick" sub fires in the
same event loop iteration.
Reported-by: Jonathan Corbet <corbet@lwn.net>
cf. https://public-inbox.org/meta/20181013124658.23b9f9d2@lwn.net/
|
|
Putting the Xref field into xover lines allows newsreaders to mark
cross-posted messages read when catching up a group. That, in turn,
massively improves the life of crazy people who try to follow dozens of
kernel lists, where emails are often heavily cross-posted.
|
|
RFC 5536 sec 3.2.14 says that the server-name in an Xref line is "which
news server generated the header field"; indeed, that is necessary for
newsreaders like gnus to handle references properly. So pick up the server
name from the config if available (the first name if there's more than
one), from the host name otherwise, and use it rather than the domain
name of the list server.
Tests have been adjusted to match the new behavior.
|
|
* origin/master:
nntp: allow and ignore empty commands
mbox: do not barf on queries which return no results
nntp: fix NEWNEWS command
searchview: fix non-numeric comparison
Allow specification of the number of search results to return
githttpbackend: avoid infinite loop on generic PSGI servers
http: fix modification of read-only value
extmsg: use news.gmane.org for Message-ID lookups
extmsg: rework partial MID matching to favor current inbox
Update the installation instructions with Fedora package names
nntp: do not drain rbuf if there is a command pending
nntp: improve fairness during XOVER and similar commands
searchidx: do not modify Xapian DB while iterating
Don't use LIMIT in UPDATE statements
|
|
Somebody hitting "\n" into telnet shouldn't hold a client up
indefinitely and prevent shutdown.
|
|
Since we only query the SQLite over DB for OVER/XOVER; do not
need to waste space storing fields To/Cc/:bytes/:lines or the
XNUM term. We only use From/Subject/References/Message-ID/:blob
in various places of the PSGI code.
For reindexing, we will take advantage of docid stability
in "xapian-compact --no-renumber" to ensure duplicates do not
show up in search results. Since the PSGI interface is the
only consumer of Xapian at the moment, it has no need to
search based on NNTP article number.
|
|
Noted by Jonathan Corbet in https://lwn.net/Articles/748184/
|
|
We we worked around the default range/termination conditions of
long_response in many cases to reduce calls to SQLite or Xapian.
So continue that trend and become more like the PSGI API
which doesn't force callers to specify an article range or
work inside a loop.
|
|
id_batch had a an overly complicated interface, replace it
with id_batch which is simpler and takes advantage of
selectcol_arrayref in DBI. This allows simplification of
callers and the diffstat agrees with me.
|