| Date | Commit message (Collapse) |
|---|
|
None of the Content-Type attributes are long-lived
(and unlikely to be memory intensive). While these
callsites won't trigger $DB::args segfaults via
confess or longmess, it'll make future code audits
easier.
cf. commit 0795b0906cc81f40
("ds: guard against stack-not-refcounted quirk of Perl 5")
|
|
It seems only triggered by bots trying to steal information.
|
|
Using "make update-copyrights" after setting GNULIB_PATH in my
config.mak
|
|
The ->mset method always returns a Xapian mset nowadays, so
naming a parameter {mset} is too confusing. As it does with
MiscSearch, setting the {relevance} parameter to -1 now sorts by
ascending docid order. -2 is now supported for descending
docid order, too, since it may be useful for lei users.
|
|
Avoid confusing hackers since this conflicts with a method name
provided by (Search::)Xapian::QueryParser.
|
|
Since IMAP search (either with Isearch or traditional per-Inbox
search) only returns UIDs, we can safely set the limit to the
UID slice size(*). With isearch, we can also trust the Xapian
result to fit any docid range we specify.
Limiting Xapian results to 1000 was making ->ALL docid <=>
per-Inbox UID impossible since results could overlap between
ranges unpredictably.
Finally, we can map the ->ALL docids into per-Inbox UIDs and
show them to the client in the UID order of the Inbox, not the
docid order of the ->ALL extindex.
This also lets us get rid of the "uid:" query parser prefix
and use the Xapian::Query API directly to reduce our search
prefix footprint.
For mbox.gz downloads in WWW, we'll also make a best effort to
preserve the order from the Inbox, not the order of extindex;
though it's possible large result sets can have non-overlapping
windows.
(*) by definition, UID slice size is a "safe" value which
shouldn't OOM either the server or clients.
|
|
Apparently they happen (triggered by my -imapd instance), so
bail out by closing the underlying socket rather than stopping
the event loop and daemon process.
|
|
This ought to save a few cycles if a client disconnects while
in the middle of a (UID) FETCH. This avoids:
Can't call method "git" on an undefined value at .../PublicInbox/IMAP.pm
errors in stderr.
|
|
It seems easiest to have a singleton Gcf2Client client object
per daemon worker for all inboxes to use. This reduces overall
FD usage from pipes.
The `public-inbox-gcf2' command + manpage are gone and a `$^X'
one-liner is used, instead. This saves inodes for internal
commands and hopefully makes it easier to avoid mismatched
PERL5LIB include paths (as noticed during development :x).
We'll also make the existing cat-file process management
infrastructure more resilient to BOFHs on process killing
sprees (or in case our libgit2-based code fails on us).
(Rare) PublicInbox::WWW PSGI users NOT using public-inbox-httpd
won't automatically benefit from this change, and extra
configuration will be required (to be documented later).
|
|
This was triggered by blindly trying to FETCH an MSN (not
"UID FETCH") on an empty dummy inbox. It's harmless, and
probably triggered by a wayward client or misbehaving bot.
|
|
We switched to Parse::RecDescent during development and left
some dead code behind.
|
|
Nearly all of the search uses in the production code rely on
a Xapian mset iterator being returned (instead of an array
of $smsg objects). So default to returning the mset and move
the burden of smsg array conversion into the test cases.
|
|
We can avoid importing mdocid() in several places by using
this method, simplifying callers.
|
|
No need to have awkward globrefs for this.
|
|
We can keep the git process more active by sending another
request to it while fetch_run_ops() is running. This
parallelization speeds up mutt's initial FETCH for headers by
around ~35%(!).
|
|
Non-slice mailboxes never have messages themselves,
so we must not assume a message exists when sending
untagged EXISTS messages.
|
|
Since -edit and -purge should be rare and TOCTOU around them
rarer still; missing {blobs} could be indicative of a real bug
elsewhere. Warn on them.
And I somehow ended up with 3 different field names for Inbox
objects. Perhaps they'll be made consistent in the future.
|
|
Since the removal of pseudo-hash support in Perl 5.10, the
"fields" module no longer provides the space or speed benefits
it did in 5.8. It also does not allow for compile-time checks,
only run-time checks.
To me, the extra developer overhead in maintaining "use fields"
args has become a hassle. None of our non-DS-related code uses
fields.pm, nor do any of our current dependencies. In fact,
Danga::Socket (which DS was originally forked from) and its
subclasses are the only fields.pm users I've ever encountered in
the wild. Removing fields may make our code more approachable
to other Perl hackers.
So stop using fields.pm and locked hashes, but continue to
document what fields do for non-trivial classes.
|
|
We need to rely on num_highwater for UIDNEXT since the
highest `num' stored in over.sqlite3 may be rolled back
if the most recent messages were spam.
We also need to load the uo2m immediately on EXAMINE to ensure
EXISTS responses are always consistent with regard to future
updates.
|
|
Clients which are NOT in an IDLE state still need to be
notified of message existence. Unlike the EXPUNGE response,
untagged EXISTS responses seem to be allowed at any time
according to RFC 3501.
We'll also perform uo2m_extend on the NOOP command, since
NOOP is the recommended command for message polling.
|
|
While this circular reference was carefully managed to not leak
memory; it was still triggering a warning at -imapd/-nntpd
shutdown due to the EPOLL_CTL_DEL op failing after the $Epoll FD
gets closed.
So remove the circular reference by providing a ref to `undef',
instead.
|
|
There's no need to loop when the first iteration guarantees
a `return'.
|
|
We need to clear the UID-offset-to-MSN mapping when
leaving mailboxes via EXAMINE/SELECT/CLOSE.
Furthermore, uo2m_last_uid() needs to account for tiny mailboxes
where the scalar representation of {uo2m} may be evaluated to
`false' in a boolean context.
|
|
We no longer pass an arrayref to search_common() or
parse_query(), so handle the CHARSET directive in
the Parse::RecDescent-generated parser directly.
|
|
For properly parsing IMAP search requests, it's easier to use a
recursive descent parser generator to deal with subqueries and
the "OR" statement.
Parse::RecDescent was chosen since it's mature, well-known,
widely available and already used by our optional dependencies:
Inline::C and Mail::IMAPClient. While it's possible to build
Xapian queries without using the Xapian string query parser;
this iteration of the IMAP parser still builds a string which is
passed to Xapian's query parser for ease-of-diagnostics.
Since this is a recursive descent parser dealing with untrusted
inputs, subqueries have a nesting limit of 10. I expect that is
more than adequate for real-world use.
|
|
Since we support MSNs properly, now, it seems acceptable
to support regular SEARCH requests in case there are any
clients which still use non-UID SEARCH.
|
|
stop_idle was a noop when the client issues a "DONE"
continuation or just disconnects. This would not have
led to a long term memory leak since FDs get closed and
reused, anyways, and all of our InboxIdle mappings are
keyed by FD.
|
|
Since IMAP IDLE users aren't expected to issue any commands, we
can terminate their connections immediately on graceful
shutdown.
Furthermore, we need to drop the inotify FD from the epoll set
to avoid warnings during global destruction. Embarassingly,
this required fixing wacky test ordering from 2a717d13f10fcdc6
("nntpd+imapd: detect replaced over.sqlite3")
|
|
"DONE" is a continuation and not a normal IMAP command, so
ensure it can't be called like a normal IMAP command which
has a tag.
|
|
Since we limit our mailboxes slices to 50K and can guarantee a
contiguous UID space for those mailboxes, we can store a mapping
of "UID offsets" (not full UIDs) to Message Sequence Numbers as
an array of 16-bit unsigned integers in a 100K scalar.
For UID-only FETCH responses, we can momentarily unpack the
compact 100K representation to a ~1.6M Perl array of IV/UV
elements for a slight speedup.
Furthermore, we can (ab)use hash key deduplication in Perl5 to
deduplicate this 100K scalar across all clients with the same
mailbox slice open.
Technically we can increase our slice size to 64K w/o increasing
our storage overhead, but I suspect humans are more accustomed
to slices easily divisible by 10.
|
|
This finally seems to make mutt header caching behave properly.
We expect to be able to safely load 50K IV/UVs in memory without
OOM, since that's "only" 1.6 MB that won't live beyond a single
event loop iteration. So create a simple array which can
quickly map MSNs in requests to UIDs and not leave out messages.
MSNs in the FETCH response will NOT be correct, since it's
inefficient to implement properly and mutt doesn't seem to
care.
Since the conversion code is easily shared, "UID SEARCH" can
allow the same MSN => UID mapping non-UID "FETCH" does.
|
|
Supporting MSNs in long-lived connections beyond the lifetime of
a single request/response cycle is not scalable to a C10K
scenario. It's probably not needed, since most clients seem to
use UIDs.
A somewhat efficient implementation I can come up uses
pack("S*" ...) (AKA "uint16_t mapping[50000]") has an overhead
of 100K per-client socket on a mailbox with 50K messages. The
100K is a contiguous scalar, so it could be swapped out for
idle clients on most architectures if THP is disabled.
An alternative could be to use a tempfile as an allocator
partitioned into 100K chunks (or SQLite); but I'll only do that
if somebody presents a compelling case to support MSN SEARCH.
|
|
Note some of our limitations for potential hackers.
We'll be renaming "UID_BLOCK" to "UID_SLICE", since "block" is
overused term and "slice" isn't used in our codebase. Also,
document how "slice" and "epochs" are similar concepts for
different clients.
|
|
Simple queries work, more complex queries involving parentheses,
"OR", "NOT" don't work, yet.
Tested with "=b", "=B", and "=H" search and limits in mutt
on both v1 and v2 with multiple Xapian shards.
|
|
We can share a bit of code with FETCH to refill UID
ranges which hit the SQLite overview.
|
|
We can get exact values for EXISTS, UIDNEXT using SQLite
rather than calculating off $ibx->mm->max ourselves.
Furthermore, $ibx->mm is less useful than $ibx->over for IMAP
(and for our read-only daemons in general) so do not depend on
$ibx->mm outside of startup/reload to save FDs and reduce kernel
page cache footprint.
|
|
This appears to significantly improve header caching behavior
with mutt. With the current public-inbox.org/git mirror(*),
mutt will only re-FETCH the last ~300 or so messages in the
final "inbox.comp.version-control.git.7" mailbox, instead of
~49,000 messages every time.
It's not perfect, but a 500ms query is better than a >10s query
and mutt itself spends as much time loading its header cache.
(*) there are many gaps in NNTP article numbers (UIDs) due to
spam removal from public-inbox-learn.
|
|
Since headers are big and include a lot of lines MUAs don't
care about, we can skip the CRLF_HDR ops and just do the
CRLF conversion in partial_hdr_get and partial_hdr_not.
This is another 10-15% speedup for mutt w/o header caching.
|
|
This speeds up requests from mutt for HEADER.FIELDS by around 10%
since we don't waste time doing CRLF conversion on large message
bodies that get discarded, anyways.
|
|
Ensure {uid_base} is always set, so we don't need to add `//'
checks everywhere. Furthermore, this fixes a hard-to-test bug
where the STATUS command would inadvertantly clobber {uid_base}.
|
|
The performance problem with mutt not using header caches isn't
fixed, yet, but mutt header caching seems to depend on MSNs
(message sequence numbers). We'll switch to storing the 0-based
{uid_base} instead of the 1-based {uid_min} since it simplifies
most of our code.
|
|
RFC 2683 section 3.2.1.5 recommends it:
> For its part, a server should allow for a command line of at least
> 8000 octets. This provides plenty of leeway for accepting reasonable
> length commands from clients. The server should send a BAD response
> to a command that does not end within the server's maximum accepted
> command length.
To conserve memory, we won't bother reading the entire line
before sending the BAD response and disconnecting them.
|
|
While selecting a mailbox is done case-insensitively, "INBOX" is
special for the LIST command, according to RFC 3501 6.3.8:
> The special name INBOX is included in the output from LIST, if
> INBOX is supported by this server for this user and if the
> uppercase string "INBOX" matches the interpreted reference and
> mailbox name arguments with wildcards as described above. The
> criteria for omitting INBOX is whether SELECT INBOX will
> return failure; it is not relevant whether the user's real
> INBOX resides on this or some other server.
Thus, the existing news.public-inbox.org convention of naming
newsgroups starting with "inbox." needs to be special-cased to
not confuse clients.
While we're at it, do not create ".0" for dummy newsgroups if
they're selected, either.
|
|
It seems required based on my reading of RFC 3501 for
the non-UID "FETCH" command.
|
|
Since we started indexing the CRLF-adjusted size of messages,
we can take an order-of-magnitude speedup for certain MUAs
which fetch this attribute without needing much else.
Admins are encouraged to --reindex existing inboxes for IMAP
support, anyways. It won't be fatal if it's not reindexed, but
some client bugs and warnings can be fixed and they'll be able
to support more of IMAP.
|
|
This is one boolean attribute not worth wasting space for.
With 20000 sockets, this reduces RSS by around 5% at a glance,
and locked hashes doesn't do us much good when clients
use compression, anyways.
|
|
RFC 3501 section 5.4 requires this to be >= 30 minutes,
10x higher than what is recommended for NNTP. Fortunately
our design is reasonably memory-efficient despite being Perl.
|
|
We should not waste memory for IDLE unless it's used on the most
recent inbox slice. We also need to keep the IDLE connection
alive regardless of $PublicInbox::DS::EXPTIME.
|
|
We can speed up this common mutt request by another 2-3x by not
loading the entire smsg from SQLite, just the UID.
|
|
We can avoid loading the entire message from git when mutt makes
a "UID FETCH" request for "(UID FLAGS)". This speeds mutt up by
more than an order-of-magnitude in informal measurements.
|