Make things a bit more compact while still informative when
displaying the size of the different parts of the state on
both startup and shutdown.
Similarly, show when we prune records on startup.
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.
This moves another set of data from the sqlite db and into our
own data structures.
The sqlite data will be copied into the new structure on startup
if we haven't already populated that data.
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.
This is lingering from the past; we don't need LocalSet any
more. I thought this might be causing some scheduling issues,
but it was neutral overall, so this commit is really just
a code cleanup.
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 noticed on a busy system with over 1500 active admin bounce
entries (created via automation) that cpu was quite busy in qmaint.
I believe that is because we were potentially spawning a new
async qmaint task for every message moving from the wheel and into
the ready queue.
This commit removes that logic, and replaces it with a per-message
check in insert_ready.
Queue insertion can generate log records if queues are full,
so holding a backlog permit could lead to blocking-like
behavior when the system is relatively saturated.
Let's release the permit prior to insertion; that way
it still provides some benefit (limiting the should-enq hook
calls) while not getting in the way.
The pattern used in the logic was that in the Pending case,
once we had acquired the semaphore, we'd do some prep work,
then await the completion of the populator function, then
store the result and then close the semaphore to force all
wakers to kick out and consult the return value.
The await is a cancellation point, which means that if the
call stack above the population function encounters a timeout
(eg: data_processing_timeout) then the whole state is dropped
and never consulted again.
The drop would release the semaphore permit and allow another
actor to acquire it, which is partially OK, but we really do
want to close the semaphore to wake up everyone in that case.
This commit switches from the explicit close at the end of
the scope to using defer! to handle it in all exits from
that particular scope, including the drop on cancellation.
Doing that highlighted a busy loop in response to the cancellation; to
deal with that, while we have the entry locked, if the semaphore is
already closed we'll treat it the same way as an expired ttl and allow
kicking off a new resolve.
I've observed a system where we had more than 4 outstanding
shaping_data lookups from different ConfigEpochs; that system
had slow dns resolution and we ended up looping for these because
the capacity was not high enough and the population of the result
always took too long.
Let's just give ourselves a bit more headroom; this coupled
with the earlier commits that adjust retry behavior should
help to smooth things out in this situation.
It's not a good idea to automatically retry internally at the lruttl
layer, as there is no bound on the retries and not enough context in the
cache layer to decide if we really should retry.
This commit removes the `retry_on_sema_timeout` flag from lruttl
and replaces the implementation of the mod-memoize
retry_on_populate_timeout flag with a simple 3 attempt retry
loop in the failure case.
Importantly, those retries in mod-memoize have the opportunity
to adjust to a bumped ConfigEpoch, which is important for the
pathological situation where a flood of changes stream in from
TSA daemon and there is a delay in handling populate, and the
cache capacity is not sufficient to accommodate that run
of epoch bumps.
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.
It's a little awkward to reason about whether you're running the latest
build or not after an upgrade, so let's explicitly print the version
to the journal.
For posterity, regardless of whether you have this particular commit or
not, you can determine the running/online version by looking at the
openapi schema document:
curl -s 'http://127.0.0.1:8000/api-docs/openapi.json' | jq .info.version
There is a potential race between the timerwheel tick and bulk queue
operations (flushing, bouncing). The logic in the tick case has
some accommodations for this, but there is another outstanding
that can result in racing to manage the metadata load state
on a message.
In the tick case, we load it if needed, then resolve the queue
name, so that we can resolve the Queue handle.
We need the queue handle to decide whether we are responsible
for the message, or whether we might be racing with a concurrent
action.
In the race case, the other actor may have decided to release
the message metadata, which will cause the queue name resolution
to fail.
While we could just shrug and silently ignore the error in that
case, that makes me uneasy.
What this commit does is adjust the v1 wheel entry so that we
capture weak references to both the message and its containing
queue.
Then when we tick, we can simply upgrade both of those and
reconcile without needing to manipulate any message metadata.
These changes unfortunately result in re-duplication of some
of the logic that I recently refactored to be shared with the
v2 tick implementation.
The v2 tick implementation has the same edge case around
metadata, but cannot be easily adjusted to use the same
pattern.
So this commit sticks a comment in the code; nobody is using
v2 AFAIK, and there have been reports of some wonky behavior
with it. My recommendation at this time is to avoid using
the v2 wheel and we can fix this all up for it later.
Some of the series ended up with bland labels like "lua" with
no context. Let's include the overall metric name for those.
The consequence of this is that the labels column gets wider,
but I think that now that we have scrolling, we can adjust
that presentation in a follow up commit.
Wez Furlong
Mike Hillyer