docs(code): honest rationales — Akka restart semantics, per-key shards, EF literal inlining; retire or annotate orphaned members

This commit is contained in:
Joseph Doherty
2026-08-16 05:33:33 -04:00
parent dbc9223206
commit b9e9df4081
11 changed files with 133 additions and 58 deletions
@@ -32,13 +32,16 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Security;
/// deploy costs one repopulation and cannot be wrong. Deployments are rare relative to reads.
/// </para>
/// <para>
/// <b>Storage is sharded per session, under a two-level bound.</b> Each session key owns its own
/// dictionary, capped at <see cref="MaxEntriesPerSession"/>; overflowing it clears <i>that shard
/// only</i>. A second, global <see cref="MaxEntries"/> bound clears every shard, and exists purely
/// so many sessions each staying under their own cap cannot add up to unbounded growth. The flat
/// cache this replaced had only the global bound, which made one machine consumer crawling a large
/// tree an eviction event for every other session on the server — the sessions doing ordinary work
/// paid for the crawl. Both caps are enforced against <see cref="Interlocked"/> counters, never
/// <b>Storage is sharded per calling KEY, under a two-level bound.</b> The shard key is
/// <c>ApiKeyId ?? Principal</c>, so <i>sessions sharing one API key share one shard and one
/// <see cref="MaxEntriesPerSession"/> cap</i> — the isolation is across distinct keys, not across
/// sessions. Overflowing a shard clears <i>that shard only</i>, so a crawl by one key cannot evict
/// another key's entries. A second, global <see cref="MaxEntries"/> bound clears every shard, and
/// exists purely so many keys each staying under their own cap cannot add up to unbounded growth.
/// The flat cache this replaced had only the global bound, which made one machine consumer
/// crawling a large tree an eviction event for every other caller on the server — the callers
/// doing ordinary work paid for the crawl. Both caps are enforced against
/// <see cref="Interlocked"/> counters, never
/// <see cref="ConcurrentDictionary{TKey,TValue}.Count"/>, which locks the whole table.
/// </para>
/// </remarks>
@@ -46,7 +49,7 @@ public sealed class SessionAclDecisionCache
{
/// <summary>
/// Process-wide entry ceiling, past which <b>every</b> shard clears. The outer of the two bounds,
/// and a rare fallback: it exists so a large fleet of sessions each staying well under
/// and a rare fallback: it exists so a large fleet of distinct keys each staying well under
/// <see cref="MaxEntriesPerSession"/> still cannot grow the cache without limit.
/// </summary>
/// <remarks>
@@ -59,16 +62,18 @@ public sealed class SessionAclDecisionCache
public const int MaxEntries = 100_000;
/// <summary>
/// Per-session entry ceiling, past which that session's shard alone clears. The inner and normally
/// the only bound reached: it is what keeps one machine consumer crawling a 50k-node tree from
/// evicting the decisions of every other session on the server.
/// Per-<b>key</b> entry ceiling (the shard key is <c>ApiKeyId ?? Principal</c>, so every session
/// using the same API key shares this one budget), past which that shard alone clears. The inner
/// and normally the only bound reached: it is what keeps one machine consumer crawling a 50k-node
/// tree from evicting the decisions taken for every OTHER key on the server. The name predates the
/// shard key and is kept for source compatibility.
/// </summary>
/// <remarks>
/// Clearing beats evicting <i>within a shard</i>: an LRU needs per-entry bookkeeping on the hot path
/// to save a trie walk that is itself cheap, so the eviction policy would cost more than the miss it
/// prevents. The failure mode of clearing is a burst of recomputation, which is exactly the steady
/// state of not having a cache at all — and sharding confines that burst to the session that caused
/// it, which is the part the old flat cache got wrong.
/// state of not having a cache at all — and sharding confines that burst to the key that caused it,
/// which is the part the old flat cache got wrong.
/// </remarks>
public const int MaxEntriesPerSession = 20_000;
@@ -105,7 +110,7 @@ public sealed class SessionAclDecisionCache
/// answer is already memoized, so the allocation the cache exists to avoid is paid on the hit
/// path too. Read and Browse ask this question once per node, which is precisely where paying
/// for a walk that never runs is worth removing. <see cref="GetOrAdd"/> remains, delegating
/// here, for callers that genuinely have nothing cheaper to offer.
/// here, but has no production caller — it is exercised by the cache's own tests.
/// </para>
/// <para>
/// The generation is checked (and a stale cache dropped) here rather than in
@@ -187,8 +192,14 @@ public sealed class SessionAclDecisionCache
/// <summary>
/// Returns the memoized decision for this key, computing and storing it when absent or when the
/// generation has moved on. A convenience over <see cref="TryGet"/> + <see cref="Set"/> for callers
/// that have no cheaper way to express the walk than a closure.
/// generation has moved on. A convenience over <see cref="TryGet"/> + <see cref="Set"/>.
/// <para>
/// <b>Retained for tests; production has no caller.</b> <see cref="DataPlaneAuthorizer"/> uses
/// the split <see cref="TryGet"/>/<see cref="Set"/> pair precisely so a hit pays for no closure,
/// and no other caller has appeared since. Kept because it is the readable single-expression
/// form of the same protocol and is how the cache's tests drive it — not because a production
/// caller is expected back.
/// </para>
/// </summary>
/// <param name="generation">The authorization generation the decision is being taken under.</param>
/// <param name="sessionKey">Stable identity of the calling session — its key id, else its principal.</param>