This is an area where conformance is inconsistently applied by
various clients, so we're relaxing our parser to try the current
strict rules first, which can normalize stuff like `<"foo"@id>`,
but then allow falling back to anything between `<>`.
We will still blow up for wonky stuff like `<<wat>>`, as proven
by our existing test coverage.
closes: #259
As mentioned in a prior commit, when there are very large numbers
of scheduled queues (hundreds of thousands), the tokio timer/task
infrastructure becomes over-burdened and the system bogs down.
This commit introduces a SingletonTimerWheel strategy for the
scheduled queues. When this strategy is in use, each scheduled
queue maintains a local HashSet of Messages rather than an
individual wheel or skip list. The time ordering is instead
handled by a global singleton timerwheel that holds weak references
to the messages.
The definitive ownership of the Message belongs to the per-Queue
HashSet.
The global wheel ticks over once every 3 seconds for ALL queues
that have delayed messages, popping off the weak references
and attempting to upgrade them to a full Message reference.
From there, the message is resolved back to its containing queue,
and if it is found in the HashSet then it is reinserted into the
ready queue.
If either the upgrade or HashSet check fails then the message
has been either bounced or rebound and requires no further
processing.
This change makes the number of scheduled tasks for delayed
messages O(1), rather than O(number-of-queues), at the cost
of a small loss in precision of promoting the delayed messages
to the ready queue, and double the amount of Message handles;
previously it would be approx:
number-of-delayed-messages * sizeof(pointer)
now it is 2x that amount (~ + some misc overhead)
I don't plan to add a SingletonSkipList strategy: since the list is
global it can have potentially millions of delayed messages (the entire
server's delayed mail) so we really do want O(1) insertion for this
rather than the skip lists worse insertion complexity.
Since this change eliminates the maintainer task, we need an alternative
approach for processing configuration refreshes, so this commit moves
that out of the per-queue maintainer and into a separate task that wakes
up periodically to assess all scheduled queues to see if they are due
for a refresh (according to their individual refresh_intervals) and
perform the refresh.
This change makes the config refresh a sequential operation:
only the refresh task will zip through and perform the refreshes.
Previously, you could expect to see most/all of the qmaint threads
doing this.
This commit adds disk space monitors for the configured spool
and local log file locations.
Incoming messages will be rejected when the available space is below
the configured amount.
The default minimum is 10%.
For large classifier rulesets (more than 3,000 rules), the cost
of classifying any single response can be as high as 2ms.
This is significant; given that the loggers are each single threaded a
2ms overhead limits the maximum throughput to 500 messages per second.
This commit improves the structure of the code in order to mitigate
the potential for a bottleneck:
* Don't bother categorizing Reception records. They are not bounces
and are generated by the local machine. It's a waste of CPU and
introduces the potential to put back pressure on the injector.
* Introduce a cache for classification results. The cache is split
into two parts so that unclassified results don't churn out the
successfully classified results.
* Introduce a bounce-classification thread pool. If we are unlucky
and encounter a long series of "random" responses with no cache
hits, and have up to 2ms per classification constraining us to
around 500 msgs/s, then we need to apply more CPU cores to the
classifier to achieve multiples of that throughput, and that is
what we have here. We spawn 1/4 the number of cores threads
into this thread pool.
This is a bit esoteric... previously, the http authentication
process would essentially short-cut things when a request was
made from a trusted host, ignoring any Auth header
that may have been present.
The rationale was that the host was trusted, so the peer would
succeed simply by omitting that header.
This commit changes the flow so that we only set the trusted
host status if no Auth header was provided.
If an Authentication header is present, it will be validated
before allowing the request to continue.
If the header is invalid then the request will be denied, even
if the peer is a trusted host.
This tightens things up slightly by making the auth validation
behavior most consistent, but it doesn't materially affect
the security posture: the trusted host can still successfully
make calls by simply omitting the authentication header.
It does mean that what was previously allowed (an invalid Auth header
from a trusted host) is no longer allowed.
Previously we were using only our timeq module, which is built on top of
hashed hierarchical timer wheels.
Timer wheels have O(1) insertion and removal which are excellent
properties for larger delayed queues.
However, they do not know how to answer the question "when is the
next item due", but only "what is due in the next tick".
The underlying timer wheel implementation assumes a 1ms granularity
which is a little bit at-odds with our pragmatic view of the scheduled
queue, which is "if it's in there, precision timing isn't important, and
it's generally fine to consider once per minute", requiring that we
either aggressively scheduled a maintainer task to wake up every 1ms per
scheduled queue (untenable!) or have it wake up no more than once per
second but ideally closer to once per minute to then tick however
milliseconds are necessary to advance the wheel to the current slot.
For small numbers of scheduled queues with sufficiently large retry
intervals this hasn't bubbled up as an issue so far, but it bothers
me that it isn't as efficient as it could be because we have to wake up
reasonably frequently to keep things ticking over, and that introduces
higher continual CPU utilization. It's small, but I worry about
the aggregate cost spread over very large numbers of scheduled queues.
What I really want here is a a timer wheel that I can tick with
arbitrary granularity and with that in mind I took a look at adapting
the handful of existing implementations and found that we're already
using the cleanest implementation, and it would take some effort that I
didn't really want to spend right now.
I opted for a reasonably simple alternative option, which is to adopt a
skiplist for the queue. This has O(log n) insertion to maintain ordering
with O(1) removal and can answer "when is the next item due". What this
means is that we pay a slightly higher insertion cost one-time in
exchange for being able to put the maintainer for the queue asleep until
we need it, and not have to keep waking up between times, which should
scale better.
What this means in practice is that we now wake up the maintainer either
when the next message is due, or once per minute to re-evaluate the
queue configuration hook, so we're slightly better off, but totally
where I'd like to be.
I've introduced a reap_interval (default 10 minutes) and a
refresh_interval (default 1 minute) as parameters in get_queue_config so
that you can increase that 1m interval for reloading.
What I'd like to do in a follow up commit is introduce a way to define
the refresh policy. For example, it would be neat to say "watch my
policy directory and refresh when it changes", which would make things
the most efficient for many users. For those that are loading their
config from a remote datasource, we'd need to consider some other
mechanism for this; maybe some kind of long-poll or pubsub, but will
obviously still be able to support the current interval based polling.
Now, with all of that said: I didn't want to switch the product
default over and hope for best, so what I did was add a strategy
option to allow this to be adopted on a per-queue basis.
Since I was in here adding some options, I also added an option
that allows explicitly setting the interval used for timerwheel
ticks, so you now have a lot more opportunities for tuning this
stuff.
When hooks were developed, they were essentially "forked" from the log
file implementation and the back_pressure setting and semantics were
carried over from there.
Those semantics were to put an upper bound on the number of log records
that could be submitted to the logger, causing the submitter to block
until the logger could catch up and reducing system pressure overall by
slowing things down.
With log hooks we're spawning a parallel task in the tail end which
meant that the back pressure was only partially effective: it didn't
apply to the end-to-end processing of a log hook.
This commit addresses that issue by introducing a semaphore that permits
up to the configured back_pressure number of concurrent log hook tasks
to be running at once, slowing things down when that limit is reached.
Previously we'd go as far as the OS would let us.
This introduces the ability to set an upper bound, and our default for
this is 32k connections, or half of the possible u16 port space. I
picked half because the ideal for an MTA is using half of its resources
to receive and the other half to send. The math for that is more
complex for multi-homed machines or systems using proxies, but it feels
like a reasonable default; not too low that it will be painfully
surprising for existing users to discover on upgrade, and not so high
that new installations are as easily overwhelmed in the face of high
incoming traffic.
A new total_connections_denied counter is added that is bumped
whenever this or the over-memory limit are reached and we turn
away a session with a 421. The idea is that the operator may
want to investigate these events, and we don't otherwise have
a stream of information about them because they are deliberately
not logged to the normal delivery logs.
Per commentary in b1330171be, move
away from mobc and standardize on deadpool.
This means that a couple of the newly added pool parameters
are no longer present, but nobody has had a chance to embrace
those yet anyway.
Make a note in the changelog about the recent changes: I fixed a bug
that no one had noticed before, which was that the username/password
parameters for single node redis were ignored. This was fixed by
52ca4d8be0
Rather than queueing up a complete copy of all logs to send
to the TSA daemon, we now perform a match on the client side.
If the record doesn't match any rules then we won't bother
sending it to the daemon.
Otherwise, we'll queue and send.
This reduces the amount of data that we'd need to queue, thus
reducing IO pressure, and reducing inter-node bandwidth.
This new behavior is currently opt-in, but will likely
be the default by the time we cut the next release.
To opt-in, add `pre_filter = true` to the `setup_with_automation`
parameters.
Previously we'd sequentially walk through the list of domains to resolve
their MX records. This can be slow, especially in the context of `kumod
--validate` or when running `validate-shaping` standalone where we start
with empty MX caches.
This commit runs the lookups in parallel, with up to 128 concurrent
lookups taking place at once, so that we can get through a large
shaping configuration in much less time.
This was deliberately adjusted to only change when a queue
was reapead in a prior release, in the interests of raw performance.
While that might work for most domains, it's not ideal for busy
domains that have continual traffic.
This change adopts an ArcSwap to atomically swap out the underlying
ArrayQueue. This introduces a small amount of overhead in the hot
path to atomically acquire the current version of the queue, but
it shouldn't be enough to be noticeable in practice.
We need to explicitly launch the connection attempt into a separate
task, otherwise the select! macro won't run it in parallel with
awaiting the shutdown notification.
Remove the r2d2 dep (which is synchronous only) and replace with
mobc which is a bit easier to use, async, and seems more developed.
Update the interface to expose more of mobc's connection pool
options.
Add explicit redis cluster integration tests.
We've been trying to run down an issue where a user has reported that
some sessions that are running via the proxy seem to hang waiting for a
response from the peer. The common theme is that the size of the
payload is approximately 1MB in size, and that the proxy is in use.
I haven't been able to get it to reproduce at all, but in looking
carefully at the code here, my splice(2) implementation used a pipe
buffer that was 1MB in size, and doing non-blocking IO outside of
tokio's internals is a bit of a black art, so I'd buy the theory
that we might be getting stuck somewhere if we did fill up that
pipe buffer.
Since I couldn't catch it in the act, I've opted to go for the simple
and safer route here, as a speculative remediation:
* Added a `--no-splice` command line parameter to opt out of using
`splice(2)` completely on Linux so that we can rule out weirdness
with splice completely. The result will have lower theoretical
max throughput, but a simpler internal implementation.
* There now exists a `tokio-splice` crate that has the same
functionality as my own splice_copy code does, but a different
implementation. Adopt that for the `splice(2)` mode.
* Switch away from splitting the stream into read/write halves: there
are now utility functions available in tokio and tokio_splice that
don't require splitting the streams, which further simplifies
the implementation.
try to nudge folks away from adopting copypasta of the advanced
section; we've seen more than few people using this when they
should just use the sources policy helper.
Add the note about the weighted robin implementation to make_egress_pool
as well.
Some http servers would canonicalize the trailing dot from the policy
domain by issuing a redirect. Since we don't follow redirects for
MTA-STS policies (per the RFC: redirects MUST NOT be followed), this
would result in no policy being loaded for that domain.
Correct this by stripping off that trailing dot.
To facilitate this, some adjustments needed to be made to the time
calculation in the maintainer because we previously didn't consider it
to be valid to have a retry_interval below 1 minute, but in order for
this integration test to be viable to run as part of the CI it needs to
run in significantly less time than 1 minute.
The approach taken here is to avoid considering 1 minute as the
baseline, but rather take 1/20th of the retry_interval. In the default
configuration, the numbers work out the same as previously, but they
will scale down as the retry_interval is reduced.
Care is taken to avoid a couple of borderline busy wait scenarios where
we might otherwise have woken up at unrealistically small intervals:
timeq can suggest 1ms in a few scenarios, and we just round those up to
the next second to avoid that.
It's worth noting that we do not consider the scheduled queue to be a
realtime, high granularity queue: anything that lands there is
considered to be bulk/batch and will be handled later: it isn't worth
prioritizing with high granularity because messages that land there are
generally not going to be delivered quickly.
Previously we were very tight-lipped. We now will log some
context to the journal for the proxy service for issues that
we couldn't propagate back to the client.
Change the field from a SocketAddr to a struct with distinct fields:
```json
// For SMTP delivery, the source address (and port) that was used.
// (*Since: Dev Builds Only*)
"source_address": {
// The source address. The port number may be unknown and reported
// as zero when using a proxy protocol.
"address": "10.0.0.1:53210",
// If a proxy protocol was used, this field will be
// set to its name. It may be null/not set for no proxy,
// "haproxy" or "socks5".
"protocol": "socks5",
// If a proxy protocol was used, this field will be
// set to the proxy server address. It will be null/not set
// when no proxy was used.
"server": "192.168.1.1:5000"
},
```
In #154, the request was to log configuration information here, but I
opted against this as there can be a number of different configuration
fields and the combinatorics for future changes make me uncomfortable
from a code maintainance perspective--it will already be heavy to
try to pass thu all of the existing config information, and as we
add more options in the future it will be awful not just to look at,
but also from a memory and storage overhead.
The approach taken here is to make a little struct that is flexible
enough to convey the desired information without it being too much of a
burden.
closes: #40closes: #154
Mike Hillyer
Wez Furlong
Tom Mairs
Daniel Schaaff