The signer cache maps the signer parameters to a pre-made
signing context.
It is essentially a map of (domain, key) -> signer.
Assuming that the (domain,key) tuple is unique, then this is
a good, effective use of that cache.
However, if multiple domains can share the same key then we
can end up re-parsing the same key data for each of them,
which is moderately expensive and a waste of CPU.
This commit introduces an additional cache for the key source to
the resultant compiled key.
This allows sharing of the same compiled key across signing
parameters that otherwise vary, and should help to shave off
some latency.
We only use the blocking threads portion of the thread pool,
so avoid the default of creating number-of-cores threads
for the IO portion. We have to create one even though we
don't use it, otherwise tokio is unhappy.
This uses our new leaner timeq data structure; that has lower
memory overhead and is a bit more efficient at ticking.
It is not the default at this time, but ideally will become
the default in the future once it has been proven out.
I'm investigating an issue where a block of messages appear to be stuck
in a scheduled state. In looking at the math here, I'm wondering if it
is possible for some overflow to happen when converting from chrono
to std time types, so adjust the logic to be less prone to such a thing,
if it is occurring.
The purpose of this record is to log additional context about why
a message might end up in the scheduled queue when it hasn't
logged a TransientFailure.
There are a few situations around handling throttles and limits
where we might put a message back into the scheduled queue, without also
logging a TransientFailure record. It's possible that we should
reconsider some of those, but for the moment, there is an observability
hole that needs to be filled.
What this commit does is introduce an `InsertContext` which can hold one
or more `InsertReason`s about why a message is being inserted into the
scheduled queue.
There are 3 primary reasons for insertion:
* Received - the message was just received/injected
* Enumerated - the message was discovered in spool enumeration
* DueTimeWasReached - the message is now due for delivery and is being
popped off the scheduled queue
The additional reasons can be added to the context to provide more
color about what happened.
When a message is added to the scheduled queue, the accumulation
in the InsertContext is examined, and if the context doesn't
indicate that the message was Enumerated and it wasn't also
already logged as a TransientFailure, a `Delay` record is
logged.
The `Delay` record includes in its `response.content` the ordered set of
InsertReasons as well as the delay duration and due time.
Logging Delay records might place undesirable pressure on the
logging storage, so you may wish to disable it via:
```lua
kumo.configure_local_logs {
per_record = {
Delay = {
-- Suppress Delay records
enable = false
}
}
}
```
or similar.
This is a step towards supporting SMTPUTF8.
This commit allows MAIL FROM and RCPT TO to contain UTF8.
UTF8 in the domain portion is converted to an A-label form
via the IDNA crate, raising an error if it is invalid.
UTF8 in the local part is unchanged.
The SMTP server now explicitly checks to see if the provided
MAIL FROM or RCPT TO is non-ascii, and rejects it; we won't
try to relay UTF8 local parts as a result of this change,
but will now accept UTF8 domain parts and relay, because we
rewrite those via IDNA.
refs: https://github.com/KumoCorp/kumomta/issues/327
Now that we have doubly-linked-lists of Messages for these things,
it is more efficient to take the whole list when we're doing
bulk ready queue operations, so let's do that.
I've been recently troubleshooting a couple of systems with high memory
usage, and in one of them there were very large amounts of
memory being allocated to ready queues. That could be partially
mitigated by reducing `max_ready` to a more reasonable and small value,
but it is difficult to compute the right balance between large-enough
for high throughput and small-enough to keep memory usage reasonable.
This commit switches away from crossbeam's ArrayQueue, which
pre-allocates sufficient space to hold exactly `max_ready` messages for
each instantiated ready queue, and to a newly introduced MessageList,
which is an intrusive doubly-linked list.
The intrusive list, in exchange for some small additional overhead
per-Message, requires no auxilliary additional memory allocations to
track the membership of that Message in some other list.
That means that `max_ready` is no longer a pre-allocated minimum amount
of additional storage, and changes the memory overhead from
`O(number-of-queues * max_ready)` to `O(number-of-ready-messages)`,
which is typically a lot smaller. This is independent of the individual
messages metadata and bodies that are nominally associated with being in
a ready queue.
This is a reasonably significant upgrade, as it allows for
async functions to be Send, which could unlock more efficiencies
in our overall scheduling.
This commit is just the basic changes required to get the updated
mlua version compiling, it doesn't change anything else.
Two issues here:
1. The maintainer would repeatedly try to shrink the ready queue
each time it woke up during a low memory condition. This has
been fixed to trigger whenever we transition to low memory
instead.
2. At some point during refactoring, we dropped the save part
of the message shrinking processing, so we'd only successfully
shrink messages that had previously been saved and stored to
spool. Messages that we deferred spooling, or otherwise modified
post reception, would not be saved and thus not be eligible to
shrink.
Ideally we'd have an integration test for this, but it is a bit awkward
because we'd need to contrive an appropriate ulimit for just this
instance and generate appropriate load to trip over that limit. I don't
fancy the chances that such a test wouldn't be flakey.
My ad-hoc test procedure was:
ulimit -m 2028527
./target/release/kumod --policy simple_policy.lua
then in another window:
./target/release/traffic-gen --target 127.0.0.1:2025 --body-size 100000 --duration 3600 --throttle 15000/s --http
the generator will eventially experience load shedding http responses,
and you can see the shrink procedure triggering in the kumod output.
curl -s 'http://127.0.0.1:8000/metrics' | grep memory
can also be used to check the usage, limit and how many times it trips.
You can also use smtp for this, but the smtp client in traffic gen will
try hard to reconnect without telling you about the shortage, so you
will need to look at the metrics to see it happening.
We've been examining a system using http injection where it
appears as though dkim signing might benefit from being broken
out into a dedicated cpu-bound thread pool.
This commit is a fairly quick prototype to allow us to explore
that.
It isn't quite what I'd consider to be the end state for this,
because the pool doesn't export metrics like the others do.
That is because this is a blocking pool rather than an async
pool.
Usage is to define the pool in pre_init:
```
kumo.on('pre_init', function()
kumo.dkim.set_signing_threads(24)
end)
```
If you don't define a pool, then the signing happens
on the calling thread, just as in prior versions.
A message in the scheduled queue will thus occupy 32 bytes (with
metadata unloaded), plus 8 bytes per Message reference. For TimerWheel
and SkipList queues, that's just 8 bytes. For SingletonTimerWheel it is
2*8 bytes (one for the queue and one for the singleton wheel). The
singleton usage is overall lower though, because the wheel overheads are
reduced to a single wheel in that mode.
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
Adjust how we track the data in Message to make it a little smaller
and cheaper to maintain multiple clones.
Previously, to avoid locking overhead during debug or other
introspection, we kept the spoolid (a 16-byte uuid) outside of the
Arc<Mutex<Inner>>. This has the consequence of making a fatter
handle type and there are a number of places (notably, every
time we pass the message into lua) where we might clone the
handle.
This commit introduces another intermediate type so that we can make
Message effectively equivalent to an Arc (size == pointer size)
and then the intermediate type has the spoolid outside of the mutex,
with the mutex protected data alongside it.
This changes the size of Message from 16 + 8 to just 8.
For a single Message handle we don't really save anything overall,
because the same amount of data needs to exist, but for each subsequent
clone we save 16 bytes.
We weren't using this outside of the unit tests, and it introduces
additional memory overheads and a layer of indirection that we
can do without, so let's remove it.
I've been holding off on this for a while, because I remember
this upgrade being a bit of a PITA for wezterm. Since then
there is a `derive(FromLua)` that makes the transition much
easier.
This upgrade makes it potentially a bit easier/nicer/cheaper for
memoize, because it is now possible to retrieve references to userdata
values (rather than always cloning them), but I haven't done anything
special to take advantage of that in this commit.
The issue only triggered when there were issues that required
rebuilding the message body in addition to the bad header.
The fix here is to silently ignore any header parsing errors
during a rebuild.
closes: #216
This adjusts how we do write and delete operations so that we
first try to immediately submit the operation to the WAL,
but if that would block, then we push the operation to
the tokio blocking thread pool in order not to hard-block
the tokio schedule thread when rocksdb needs to perform writes.
While in there, hook up the force-sync flag to the closest
approximation in the WriteOptions for the batch (of size 1).
We've had a report that `msg:from_header().domain` is failing
on an input in an unexpected way with this error message,
so let's try to get a bit more context on what is happening.
dependabot wanted to upgrade to base64 0.21, but I resent that
the API for the common case of base64 encoding got more difficult
in the move from 0.13.
So, I'm switching us to data_encoding's implementation instead.
Note that the overall rust ecosystem on which we depend here,
transitively depends on 3 different versions of the base64 crate, so
even though we've removed direct deps on base64 from kumomta, the lock
file still references base64 0.10, 0.13 and 0.21, so that kinda sucks.
This helps to remove "false" deps on things like openssl; now that
is linked in only to those components that need it (eg: things that
use unbound for dnssec, or the smtp client for DANE).
This makes the binaries other than kumod (which still has max deps)
a few MB smaller; not a massive win, but nice all the same.
This method will prepend an Authentication-Results header
to the message with the specified set of results and
local server name.
We need a more convenient way to default the server name;
ideally we'd pick this up from the listener, but that is currently
awkward in the policy.
refs: https://github.com/KumoCorp/kumomta/issues/82
Wez Furlong
Dirkjan Ochtman
ncai