The changes in e4b743cb8f to improve
memoize performance could cause the error message above because
the __newindex method wasn't actually writing the new value to
the unshared table, it was returning the existing value!
Most of this commit is really plumbing to support calling through the
cache layers in the same way that the listener would use, but in the
context of the unit tests in the lua file.
A nice side effect of this plumbing is that it is now possible
to iterate (via pairs) and index fields of the main config
objects that are returned by the most common `kumo.make_xxx`
functions.
kumo-address requires tokio's net feature; previously we were
implicitly working with this because other crates in the build
provided this feature.
but, cargo nextest run -p kumo-log-types doesn't provide this.
So, add it!
We'll accept these in MAIL FROM and RCPT TO, and leave it to the
client to indicate whether 8bit is in use.
This is partially supporting 8BITMIME and SMTPUTF8, without
explicitly claiming support for it via the EHLO capabilities.
This is probably sufficient to approximate support for sites that are
willing to try it and advertise those extensions for themselves via
https://docs.kumomta.com/reference/events/smtp_server_ehlo/
```lua
kumo.on('smtp_server_ehlo', function(domain, conn_meta, extensions)
table.insert(extensions, 'SMTPUTF8')
table.insert(extensions, '8BITMIME')
return extensions
end)
```
refs: https://github.com/KumoCorp/kumomta/issues/327
This is not-quite SMTPUTF8 compliant; if we have 8bit data and
the remote host advertises the relevant capabilities, we'll
add the appropriate parameters to MAIL FROM.
It's non-compliant in that we're supposed to raise an error
if we have 8bit and the destination doesn't support it.
Here we're assuming that the remote host will raise an error
if it won't accept it.
refs: https://github.com/KumoCorp/kumomta/issues/327
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
I was recently investigating slowness around the use of
the queue helper's `apply` method.
What was notable about this was that for this particular deployment,
the `apply` method should have been effectively a NOP but it had
a notable slowdown on the aggregate throughput.
The slowdown was because the queue.toml file was quite large, and
because our memoization cache was deep-copying the table out of the
cache on a cache hit.
What this commit does is introduce a proxy type to provide a
copy-on-write userdata value to act as a stand-in for the raw data.
What this means is that referencing the memoized value is now much
cheaper, and only the code that needs to muta the returned value (such
as in the listener domain helper) pays the cost of a deep copy, and even
then, it only pays that cost for the values that it has referenced out
of the cached value.
Prior to these changes, loading ~2000 line queue.toml file looked
like this:
initial apply: 0.006792s (which loads the data as well)
apply: 0.000576s (cached)
with these changes the number look like this:
initial apply: 0.005616s (which loads the data as well)
apply: 0.000015s (cached)
which is ~40x faster for this particular set of data.
In the previous commit, the new materialize_to_lua_value function
is used by from_lua_value to allow processing userdata into
specific types known at compile time.
In the mod-serde lua module we have a similar desire to handle
userdata, but we do not have a concrete rust type and dont call
into from_lua_value.
This commit causes the serialization functions in mod-serde to
explicitly call into materialize_to_lua_value as part of their
serialization logic.
from_lua_value is used to attempt to deserialize a generic lua
value into a concrete Rust type that is known at compile time.
This uses some support logic inside mlua to allow:
* If it is a userdata created by mlua, mlua will try to retrieve
that concrete rust type if the type of the userdata exactly
matches.
* otherwise: it will use serde to deserialize the lua value
into the rust type. mlua's deserializer cannot process
userdata, so if we still have on at this point, the attempt
to convert it will fail.
That's mostly fine, but our memoize crate will start returning a proxy
userdata type in a future commit, and since that proxy type will never
match the concrete Rust type that is desired by the above logic, it will
become impossible to pass memoized values without some kind of explicit
preprocessing.
What this commit does is add implicit preprocessing of the value: if we
cannot use the default mlua userdata cast, then, if the value is a
userdata, we will iterate over its fields to build a native lua
representation of that value. We also apply this translation by
traversing tables and converting their values, recursively.
While this is "preprocessing", it is only applied in case the regular
from_lua_value logic in mlua (the fast path) fails, so this is fallback
preprocessing rather than eager preprocessing, which should be
performance neutral in the common hot code paths.
In an upcoming change, I want to allow comparing userdata
and tables that serialize the same way, rather than just
using the current logic which decides that a table and
a userdata can never be equal.
This commit removes that check and instead continues through
to iterate the keys/values of both operands regardless of
whether the container is a table or userdata.
The timer object can track latencies in lua code and is helpful for
ad-hoc, explicit "profiling" in your policy script: the latencies are
exported via a prometheus histogram.
I noticed this while reading through the code; we were not updating the
round robin state which we maintain based on the configured egress pool
if the config updates and changes the pool.
We resolve this here by using an arcswap to keep a read-only reference
to the state; since has some async portions it is important to allow it
to continue its management of interior mutability so that it doesn't
acquire a mutex while awaiting on async state.
This is helpful in situations where you need to do things that
are not strictly supported but might just happen to squeak by
if support is claimed for them.
such as `100/123m`.
We could always represent these in the data model, we just didn't
support specifying them as a string.
Doing this also allows us to handle printing them out, which previously
we punted on due to lack of support for reading them in the first place.
With a specific farsi input string, there are one or more codepoints
that encode with 0x20 as part of their subsequence. The qp_encode
function would confuse those with a space and produce the wrong
output.
This commit fixes that by having the encoder iterate by codepoint
rather than by byte, and then emit the appropriate byte sequences
from there.
Move the limit script to a separate file so that we can
format it with stylua.
Make the debug logic conditional on a DEBUG variable so
that it has no real overhead in the common case, but we can
turn it back up when troubleshooting. This is unfortunately
not a runtime option, so we'll need to recompile to activate
the logic, but it at least means that we can preserve it
until we need it.
Since ZRANGEBYSCORE is deprecated in redis 6.2 and up, make
our use of it vs. ZRANGE BYSCORE conditional on the redis
server version.
We were using the newer ZRANGE BYSCORE syntax, but most of the
supported distros are running older versions of redis that may
have bugs or don't fully support this syntax, so let's use
the older version of this functionality for now.
trying to hone in on this, which I think is an edge case when we try to
acquire just as element(s) are expiring and producing an empty set:
```
thread 'limit::test::test_redis' panicked at crates/throttle/src/limit.rs:469:14:
called `Result::unwrap()` on an `Err` value: AnyHow(error invoking redis
lease acquisition script
key=test_redis-4e24b3a0-81f8-4cbe-abfb-16c63ee3e7cf now=1736174679.13151
expires=1736174681.13151 limit=2
uuid=346b46d2-00d6-4109-9775-a7559d484eba
Caused by:
An error was signalled by the server - ResponseError: Error running
script (call to f_2e0bdd7bbe1fa07f6cc6eebfba9ef415044c97a5):
@user_script:13: ERR value is not an integer or out of range)
```
We need to ensure that we quote the name portion of a mailbox
if it contains an @-sign in order for the resulting mailbox to
be valid.
quoting here means that the name is enclosed in quotes, but
not that the @-sign is itself quoted with a backslash, so
we have a little hard-coded special case in the string
quoting function for this.
With the recent changes to acquire_lease, the maintain() method
can now block for a couple of minutes pending acquisition of a lease.
This commit should knock it out of that wait when the system
is either shutting down, or starts running low on memory.
If we don't need to open any new connections, return early.
This just reduces the indentation level in the maintainer
method, and has not functional impact.
Previously, the lease acquire operation was a non-blocking instantaneous
operation, relying on the maintainer to check back and acquire again
later on.
Because that maintainer time interval could be relatively long, this
could lead to multiple queues hanging around not doing very much,
especially when the limit leases were under heavy contention.
This commit introduces a deadline parameter to the lease acquire method,
causing the acquire operation to wait until that deadline before giving
up with a TooManyLeases error.
For local/memory-backed mode that wait operation is provided by the
tokio Notify object; the calling task goes to sleep until awoken or the
timeout expires.
For redis-backed mode there isn't a suitable wakeup mechansim (well,
there is a redis pubsub channel, but the plumbing for this across the
various modes of redis client make integration awkward, and it would
mean that every lease acquire would ping every kumomta instance. That
traffic may not be desirable, so at this time we're not using that
mechanism), so the strategy is a simple
poll-redis-every-3-seconds-while-we-wait.
This commit encompasses the wait inside the throttle crate, and its API
fan-out, but there likely needs to be some additional smarts in the
ready queue so that that wait can be interrupted for low memory and
shutdown.
I plan to make LimitSpec the type we use for the path config
with some options that do not require a duration, so rename
this particular type in preparation for that.
No functional change here.
* add send_batch method to kafka mod
* add error messages to returned value and don't log it as error
* add kafka send_batch doc
---------
Co-authored-by: ncai <ncai@chapsvision.com>
A user reported that one of their users was having trouble
with a message that seemed to point at a multi-line Content-Disposition
header with some specific content.
This commit adds a unit test that shows that the issue was not due
solely to just this header, because it successfully parses.
Their issue is something else that is yet to be understood.
A user reported that constructing certain UTF-8 From headers
in the HTTP injection API could produce a From header that
could not be parsed by the DKIM helper when subsequently
attempting to sign the message.
The issue was that the textwrap crate will try to fill out
the wrap, preferring to break an existing word rather than
generating a new line to accommodate one when it would
produce a line that was too long.
This commit adds our own text wrapping algorithm that is
more forgiving.
This adds connection limit/throttle states to the readyq rows
in `kcli queue-summary`, alongside where we would show the
suspension state.
This makes it easier to understand when a given egress path
might be hitting connection limits.
The issue here is:
* If a connection limit was hit (eg: TooManyLeases) then the
readyq maintainer completes its work for that one wakeup
* readyq maintainer then goes to sleep until either 10 minutes
have elapsed, or a new message is moved into the ready queue
* If the system either has no new messages being injected to
that queue, or all of the messages are currently ready,
then those messages will camp out in the ready queue until
10 minutes have elapsed before we try to make another connection.
This hampers the rate of egress.
What this commit does is:
* Introduce a QueueState concept where we can indicate a kind of
"status effect" that applies to a queue. The state has some
human readable context and a time for when the effect was
most recently observed.
* Adds a `connection_limited` state to indicate when we've hit a
connection limit and a `connection_rate_throttled` state when we've
hit the max_connection_rate.
* The ready queue maintainer will reduce its wakeup interval
if it observes that connections have been limited, so that
we can wakeup sooner.
In a separate commit, an API endpoint will be added to expose
these queue states and augment the summary command output.
We had a report of a stack overflow while using this option in some
heavy load scenarios.
It's not clear that it is 100% correlated, but I'd feel better
not having recursion here so that we can rule that out.
Wez Furlong
Nathalie Caiandncai
Mike Hillyer