docs: perf roadmap remainder — outcomes + follow-up register

This commit is contained in:
Joseph Doherty
2026-08-16 05:08:53 -04:00
parent 451273c860
commit e75bb38734
5 changed files with 335 additions and 6 deletions
+56 -1
View File
@@ -438,13 +438,68 @@ Behaviours worth knowing before enabling it:
Read, Browse and subscription creation ask the same question many times per request — one Browse of a
device folder asks once per child. Those seams go through a per-`(session, node, operation, realm)`
decision cache, flushed wholesale whenever the authority generation advances.
decision cache (`SessionAclDecisionCache`), cleared wholesale whenever the authority generation
advances.
⚠️ **The cache holds the grant walk, not the decision.** Grants, principals and the trie are all fixed
for the life of a generation, so memoizing the walk is sound. A key's `ExpiresUtc` passes with the
*clock*, not with a deployment — so revocation and expiry are re-checked on every call, outside the
cache. Caching them would have let an expired key keep working until the next deploy.
**The cache is sharded per session, because a flat one let any client evict every other client's
decisions.** It used to be a single dictionary with the session identity folded *into* the composite key
and one process-wide bound of `MaxEntries = 100_000`, whose only eviction was a wholesale `Clear()` at
the cap. So one machine consumer crawling a large address space past 100k entries wiped the memoized
walks of every other session on the server, and each store paid a `ConcurrentDictionary.Count` — an
all-locks snapshot — to test the cap. It is now a dictionary of per-session shards keyed by the session's
`ApiKeyId ?? Principal`, each holding its own `(operation, realm, nodeIdentifier)` entries, with two
bounds:
| Bound | Value | What it clears |
|---|---|---|
| `MaxEntriesPerSession` | 20 000 | that one session's shard |
| `MaxEntries` | 100 000 | every shard (the shard *objects* are dropped, not emptied) |
Both are compile-time constants, not appsettings keys. Five sessions simultaneously at their per-session
ceiling reach the global cap; **the number of shards is not itself bounded** — the bound is entries.
Things to know before changing it:
- **There is no session-close eviction hook**, deliberately. A closed session's shard is shed the next
time a wholesale clear happens — a generation bump, or the global cap tripping. Dropping the shard
objects rather than emptying them is what makes that sweep work.
- **The counters are approximate under concurrency, by design.** The cap test is a check-then-act over
`Volatile.Read` of `Interlocked` counters rather than a lock, so concurrent stores can overshoot a cap
by roughly one shard's worth. Every clear resets the counters, so drift cannot ratchet. Inserts use
`TryAdd` (not an indexer assignment) so that only a genuine insert moves a counter — otherwise two
threads racing the same key would inflate them and permanently pull the ceilings in.
- **Only the CAS winner clears on a generation bump.** `InvalidateIfStale` compare-exchanges the
generation, so a post-deploy burst of requests repopulates the cache once instead of each thread
clearing it out from under the others.
**The hot path looks the entry up before it builds anything.** The gate used to construct the node scope
— splitting the `s=` identifier and allocating a `NodeScope` — and then allocate a five-capture closure
for the walk, on *every* call including cache hits. Now the identifier is extracted once, the cache is
consulted immediately, and a hit returns having built no scope, no closure and no
`UserAuthorizationState`; the scope and the walk happen only on a miss. `GetOrAdd` was split into an
allocation-free `TryGet` plus a `Set` to make that possible, and `TryGet` deliberately does not
create a shard, so a hit-only session never mints one and a just-cleared shard is not resurrected until
something is actually stored.
⚠️ **Revocation is checked ahead of the cache, not behind it** — the key-usability lookup runs *above*
the `TryGet`, so a cache hit cannot serve an allow for a key that has since been disabled or expired.
Reversing those two lines is the subtle way to reintroduce the bug the paragraph above warns about: the
generation never moves when a key merely expires, so nothing would ever drop the stale allow. Two tests
pin the ordering (`Key_revocation_is_rechecked_on_a_cache_hit` and
`An_expired_key_is_denied_even_though_its_allow_is_cached`).
**Deny reasons are composed lazily.** A denial on the cached path logs at `Debug`, and the interpolated
reason strings (which generation, which key, which operation) used to be built at every call site before
the log level was consulted — so a Browse denied across a thousand children formatted a thousand strings
with `Debug` off. The interpolating denials now guard on `_log.IsEnabled(level)` first, and the
`BadUserAccessDenied` `ServiceResult` is a shared static instance rather than one allocation per denied
node.
#### Everything denies when something is missing
No identity, no principal on the identity, **no cluster identity on the node**, a NodeId with no string