When using throttles with a long period, a porting issue from the JS
based throttle code meant that we'd compute a TTL in *seconds* but tell
redis that its value is in *milliseconds*, resulting in a much shorter
expiration time than desired, allowing more messages to pass the
throttling condition.
This issue doesn't apply to the redis-cell based throttle
implementation.
The fix here is simply to switch the `PX` (TTL in milliseconds) to
`EX` (TTL in seconds).
Possibly this was the source of the flakeyness that we could never
run down in https://github.com/KumoCorp/kumomta/issues/297
TL;DR: you can easily halve your system performance by logging headers
vs. logging meta.
This is one of those things that is easy to overlook or forget,
but: whenever you need to operate on the message data, rather
than its metadata, the aggregate cost is high.
In this case, we were recently troubleshooting a system where
the CPU was bogged down and we traced it to the logging configuration: a
number of message headers were being logged in a configuration that
made heavy use of throttles and limits in its traffic shaping, and
thus had a large number of Delayed and TransientFailure events being
written to the logs.
When logging headers, each one of those events requires loading
the message from the spool and parsing out the headers. When the
average message size is ~100KB this imposes a notable overhead
on the CPU and IO utilization of the system.
What we recommend instead of logging headers directly is capturing
the information that you want to log into the message metadata
at the time that the message is received.
The message meta is usually already loaded, but is also typically
much smaller and easier to decode than the full message content
in the cases where it is not loaded.
As a result, it is much cheaper to log meta than to log headers.
This commit adds some warnings and cross links to help folks
be aware of this, and to generally navigate related meta and logging
topics more easily via tags.
These allow optionally reducing how aggressively the dispatcher and
maintainers will be awoken when message(s) are submitted to the ready
queue.
The default behavior remains the same; the new thing here is the
ability to make it more relaxed, which should reduce some CPU
overheads for very busy systems with many queues.
Making things more relaxed does introduce a possibility for higher
outbound latency in some edge cases with low or bursty traffic.
We were blanket-applying the text charset conversion and it was
interpreting the binary bytes as windows-1252 content, which could cause
us to then re-encode the content as the utf8 equivalent of the 1252
interpretation of those bytes.
In a pathological configuration (essentially unbounded and continually
increasing number of campaigns, tenants, coupled with a synthetic
10% rate of triggering drastic bounce automation rules) I observed
that tsa could trip over itself when locking the sqlite db to
maintain the event history table. This in turn causes delays
in responding to the shaping data endpoint, leading to timeout
errors in kumod.
This commit removes the sqlite-based event_history table that was
the source of that contention and replaces it with a much more
compact and easier to reason about set of in-memory data structures
built around a sharded hash map. This makes it very quick to
insert and return the current count when assessing rules with
triggering thresholds.
This new state is serialized using msgpack and periodically saved
in the background, as well as on shutdown.
For sites with large numbers of admin bounce entries, it is
desirable to negatively cache the lookup so that we can avoid
extraneous matching overhead.
We use a generation counter to determine if any rules have changed
and allow that to drive the invalidation of negative caching.
Otherwise, we cache the last lookup and, if the generation is the
same, then we cache the prior result and respect its expiration.
After running this at a couple of sites, this appears to be
good overall, so let's remove the option, reduce complexity
and make things better by default.
I saw an instance where the throttle_insert_ready_queue event was
trying to inspect the metadata but failing because the metadata was
not loaded.
This should avoid that problem.
This commit does two things:
* Migrate the limit of 128 concurrent lookups from the shaping code
and into the general mx lookup code
* Adjust the mx lookup implementation to run inside the cache
getter, which applies thundering herd protection to the lookup.
These together will constrain the amount of queries being sent
to the upstream dns resolver and reduce the chances of it being
overwhelmed, especially if we are doing duplicate queries from
multiple concurrent contexts simultaneously.
We cannot default it to enabled because a fully working
dane setup requires additional configuration in the resolver
and we cannot guarantee that that has happened here.
There are a lot of metrics these days, we need to scroll through them!
Use the arrow keys, page up/down and home/end for this purpose.
closes: https://github.com/KumoCorp/kumomta/issues/372
This was probably the casualty of some earlier refactoring
that has gone unnoticed until now.
Since we don't have explicit context on which key format to
parse in this helper function that is used in multiple places,
let's just make it try to parse both rsa and ed25519.
closes: #368
Occasionally we'll have someone report that systemd timed out
and sigkill'd their kumo on shutdown.
One possible scenario for this is a lua delivery handler that
is taking too long, presumably because the other end of it
(eg: webhook or other custom endpoint) is not responding in
a timely fashion.
The way that we handle shutdown is that we compute a maximum
theoretical timeout value by summing up all of the smtp client
timeout values. Some of those can be several minutes in
duration because the are using default values derived from
a very conservative set of values suggested by the SMTP
RFCs from the '70s.
Those obviously should not apply to a custom delivery handler,
but also, in the context of an established SMTP session, we
should not add in the connection-establishment-specific values
when we're just waiting for a per-message send.
This commit addresses this situation on two fronts:
* Introduce a new system_shutdown_timeout value that allows the
user to conveniently express their desired timeout value
in a single option. This is *not* set by default!
* The default value for system_shutdown_timeout is computed by
summing the per-message-delivery smtp timeout options, which
is a much more reasonable, and more importantly, shorter than
our 300s TimeoutStopSec value in kumomta.service
Previously, we'd use the qmaint pool to spawn both the scheduled
and ready queue maintenance tasks.
This commit splits them apart in order to avoid the potential for
cross-task contention on the same threads if a scheduled queue
and ready queue pair are communicating with each other.
We recently observed a system where tsa-daemon run out of file
descriptors because it hit a systemd default of 1024 on the host system,
which had 192 cores. We spawn a thread per core for internal processing
in tsa-daemon, and that has associated with it a number of kernel
objects that each have an fd.
1024 is an unreasonably small limit for the number of fds, so let's
just try to raise to match the normally much higher hard limit, just as
we do in kumod.
We recently observed a system running on an over-committed VM that
reported 4x the actually available parallelism.
Since we scale our thread pool sizes from this value, it resulted
in an extra-over-committed configuration for kumod.
You may now set KUMO_AVAILABLE_PARALLELISM in the environment to
override the value that we see both interally and expose via
the lua function with the same name.
Wez Furlong
Mike Hillyer
Mike HillyerandWez Furlong
Edgar Vaitkevicius