diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/SessionAclDecisionCache.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/SessionAclDecisionCache.cs
index 545b481c..dd1c6fb8 100644
--- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/SessionAclDecisionCache.cs
+++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/SessionAclDecisionCache.cs
@@ -31,30 +31,68 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Security;
/// nodes a changed grant covers, which is the trie walk run backwards; dropping everything on a
/// deploy costs one repopulation and cannot be wrong. Deployments are rare relative to reads.
///
+///
+/// Storage is sharded per session, under a two-level bound. Each session key owns its own
+/// dictionary, capped at ; overflowing it clears that shard
+/// only. A second, global 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 counters, never
+/// , which locks the whole table.
+///
///
public sealed class SessionAclDecisionCache
{
///
- /// Entry ceiling, past which the cache clears rather than growing. Large enough that a realistic
- /// address space browsed by a handful of machine consumers never reaches it, small enough that a
- /// pathological client cannot use it to exhaust memory.
+ /// Process-wide entry ceiling, past which every shard clears. The outer of the two bounds,
+ /// and a rare fallback: it exists so a large fleet of sessions each staying well under
+ /// still cannot grow the cache without limit.
///
///
- /// Clearing beats evicting here: 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.
+ /// Enforced against an counter rather than
+ /// , which takes every lock in the table — a
+ /// price the store path would pay on every miss to answer a question that only matters at the
+ /// ceiling. The counter is approximate under concurrency by design, and is reset by every clear so
+ /// drift cannot ratchet permanently.
///
public const int MaxEntries = 100_000;
- private readonly ConcurrentDictionary _entries = new();
+ ///
+ /// 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.
+ ///
+ ///
+ /// Clearing beats evicting within a shard: 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.
+ ///
+ public const int MaxEntriesPerSession = 20_000;
+
+ private readonly ConcurrentDictionary _shards = new(StringComparer.Ordinal);
private long _generation = -1;
+ private int _globalCount;
/// The authorization generation the cached decisions were taken under.
public long Generation => Interlocked.Read(ref _generation);
- /// Number of decisions currently memoized. Diagnostics and tests.
- public int Count => _entries.Count;
+ ///
+ /// Exact number of decisions currently memoized, across every shard. Diagnostics and tests only —
+ /// it enumerates the shards and takes each one's locks, so it must stay off the store path (which
+ /// uses the approximate counters above instead).
+ ///
+ public int Count
+ {
+ get
+ {
+ var total = 0;
+ foreach (var shard in _shards.Values) total += shard.Entries.Count;
+ return total;
+ }
+ }
///
/// Returns the memoized decision for this key, computing and storing it when absent or when the
@@ -79,8 +117,9 @@ public sealed class SessionAclDecisionCache
InvalidateIfStale(generation);
- var key = new DecisionKey(sessionKey, operation, realm, nodeIdentifier);
- if (_entries.TryGetValue(key, out var cached)) return cached;
+ var shard = _shards.GetOrAdd(sessionKey, static _ => new Shard());
+ var key = new ShardKey(operation, realm, nodeIdentifier);
+ if (shard.Entries.TryGetValue(key, out var cached)) return cached;
var decided = compute();
@@ -89,13 +128,32 @@ public sealed class SessionAclDecisionCache
// trie never made. Dropping it costs one recomputation on the next call.
if (Generation != generation) return decided;
- if (_entries.Count >= MaxEntries) _entries.Clear();
- _entries[key] = decided;
+ if (Volatile.Read(ref _globalCount) >= MaxEntries)
+ {
+ // The outer bound: no single session is over its own ceiling, so there is nothing narrower to
+ // drop. Re-acquire the shard — ClearAll drops the shard objects themselves.
+ ClearAll();
+ shard = _shards.GetOrAdd(sessionKey, static _ => new Shard());
+ }
+ else if (Volatile.Read(ref shard.ApproxCount) >= MaxEntriesPerSession)
+ {
+ shard.Entries.Clear();
+ Interlocked.Add(ref _globalCount, -Interlocked.Exchange(ref shard.ApproxCount, 0));
+ }
+
+ // TryAdd, not an indexer assignment: only a genuine insert may move the counters, or a racing pair
+ // of walks over the same key would inflate them and pull the ceilings in for good.
+ if (shard.Entries.TryAdd(key, decided))
+ {
+ Interlocked.Increment(ref shard.ApproxCount);
+ Interlocked.Increment(ref _globalCount);
+ }
+
return decided;
}
/// Drops every memoized decision. Called when the authorization generation advances.
- public void Clear() => _entries.Clear();
+ public void Clear() => ClearAll();
private void InvalidateIfStale(long generation)
{
@@ -104,9 +162,31 @@ public sealed class SessionAclDecisionCache
// Only the thread that actually moves the stamp clears, so a burst of concurrent requests arriving
// just after a deploy repopulates once rather than each clearing the others' fresh entries.
- if (Interlocked.CompareExchange(ref _generation, generation, seen) == seen) _entries.Clear();
+ if (Interlocked.CompareExchange(ref _generation, generation, seen) == seen) ClearAll();
}
- private readonly record struct DecisionKey(
- string SessionKey, OpcUaOperation Operation, AddressSpaceRealm Realm, string NodeIdentifier);
+ ///
+ /// Drops every shard and resets the global counter. Dropping the shard objects rather than emptying
+ /// them also sheds the shards of sessions that have since closed. A walk racing this holds a shard
+ /// that is no longer in the table; its entry is simply lost, which is the same outcome the clear
+ /// itself produces.
+ ///
+ private void ClearAll()
+ {
+ _shards.Clear();
+ Interlocked.Exchange(ref _globalCount, 0);
+ }
+
+ /// One session's decisions, plus the approximate entry count its own ceiling is enforced on.
+ private sealed class Shard
+ {
+ public readonly ConcurrentDictionary Entries = new();
+
+ /// Approximate by design — read and written with , never derived
+ /// from on the store path.
+ public int ApproxCount;
+ }
+
+ private readonly record struct ShardKey(
+ OpcUaOperation Operation, AddressSpaceRealm Realm, string NodeIdentifier);
}
diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SessionAclDecisionCacheTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SessionAclDecisionCacheTests.cs
index 1b6eaba1..7e595806 100644
--- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SessionAclDecisionCacheTests.cs
+++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SessionAclDecisionCacheTests.cs
@@ -111,20 +111,58 @@ public sealed class SessionAclDecisionCacheTests
}
///
- /// The cache is bounded. A client walking a huge address space must not be able to grow it without
- /// limit; clearing at the ceiling degrades to recomputation, which is the no-cache steady state.
+ /// The global bound still holds when no single session is anywhere near its own ceiling. Spreading
+ /// the entries over many sessions is the case the per-session cap cannot catch, so the global
+ /// counter is the only thing standing between a large fleet and unbounded growth.
///
[Fact]
- public void The_cache_is_bounded()
+ public void The_global_bound_still_holds()
{
var cache = new SessionAclDecisionCache();
- for (var i = 0; i <= SessionAclDecisionCache.MaxEntries + 10; i++)
- Ask(cache, generation: 1, node: $"Plant/dev1/Tag{i}", compute: () => true);
+ // Well under MaxEntriesPerSession, so every clear observed here is the global one.
+ const int perSession = 5_000;
+ var total = SessionAclDecisionCache.MaxEntries + perSession;
+ for (var i = 0; i < total; i++)
+ Ask(
+ cache, generation: 1, session: $"key-{i / perSession}", node: $"Plant/dev1/Tag{i}",
+ compute: () => true);
+
+ // The global counter is approximate under concurrency, so the contract allows one shard's worth of
+ // slack; single-threaded it is exact, which is the stronger fact pinned here.
cache.Count.ShouldBeLessThanOrEqualTo(SessionAclDecisionCache.MaxEntries);
}
+ ///
+ /// One session filling its own shard evicts only itself. This is the whole point of sharding: a
+ /// machine consumer crawling a 50k-node tree used to blow away every other session's decisions on
+ /// its way past the ceiling, so the sessions doing ordinary work paid for the crawl.
+ ///
+ [Fact]
+ public void A_full_session_shard_clears_only_that_session()
+ {
+ var cache = new SessionAclDecisionCache();
+ const string crawler = "crawler-key";
+ const string neighbour = "neighbour-key";
+ var neighbourCalls = 0;
+
+ Ask(cache, generation: 1, session: neighbour, compute: () => { neighbourCalls++; return true; })
+ .ShouldBeTrue();
+ neighbourCalls.ShouldBe(1);
+
+ for (var i = 0; i <= SessionAclDecisionCache.MaxEntriesPerSession + 10; i++)
+ Ask(cache, generation: 1, session: crawler, node: $"Plant/dev1/Tag{i}", compute: () => true);
+
+ // The crawler's own shard was cleared and restarted rather than growing without limit.
+ cache.Count.ShouldBeLessThanOrEqualTo(SessionAclDecisionCache.MaxEntriesPerSession);
+
+ // The neighbour's single decision survived the crawl — its walk is not re-run.
+ Ask(cache, generation: 1, session: neighbour, compute: () => { neighbourCalls++; return true; })
+ .ShouldBeTrue();
+ neighbourCalls.ShouldBe(1);
+ }
+
///
/// A decision taken while a deployment lands mid-walk is returned but NOT stored, so the new
/// generation never inherits a verdict its own trie did not make.