perf(grpc): O(1) unconstrained bulk fast path, direct filtered-command build, cache eviction, capacity hints

This commit is contained in:
Joseph Doherty
2026-08-15 12:24:17 -04:00
parent 88d38bb900
commit 7171892984
6 changed files with 261 additions and 33 deletions
@@ -461,6 +461,14 @@ public sealed class MxAccessGatewayService(
string? correlationId, string? correlationId,
CancellationToken cancellationToken) CancellationToken cancellationToken)
{ {
// An identity with no read constraints allows every tag, so the per-item enforcer call below
// can only answer "allowed" — the whole loop (and the plan it would build) is dead work.
// Returning null is exactly what the denied.Count == 0 exit below returns.
if (!constraintEnforcer.HasReadConstraints(identity))
{
return null;
}
Dictionary<int, SubscribeResult> denied = []; Dictionary<int, SubscribeResult> denied = [];
List<string> allowed = []; List<string> allowed = [];
for (int index = 0; index < tagAddresses.Count; index++) for (int index = 0; index < tagAddresses.Count; index++)
@@ -491,16 +499,23 @@ public sealed class MxAccessGatewayService(
return null; return null;
} }
MxCommand filtered = command.Clone(); // Build the filtered command directly instead of cloning the original and clearing it:
if (filtered.Kind == MxCommandKind.AddItemBulk) // the clone deep-copied every denied address only to drop it. The payload's other fields
// (server_handle) are copied across explicitly. Nothing aliases the request here — these
// bulk payloads carry only strings — and the worker-bound graph is still the unaliased copy
// MapCommand makes.
MxCommand filtered = new() { Kind = command.Kind };
if (command.Kind == MxCommandKind.AddItemBulk)
{ {
filtered.AddItemBulk.TagAddresses.Clear(); AddItemBulkCommand payload = new() { ServerHandle = command.AddItemBulk.ServerHandle };
filtered.AddItemBulk.TagAddresses.Add(allowed); payload.TagAddresses.Add(allowed);
filtered.AddItemBulk = payload;
} }
else else
{ {
filtered.SubscribeBulk.TagAddresses.Clear(); SubscribeBulkCommand payload = new() { ServerHandle = command.SubscribeBulk.ServerHandle };
filtered.SubscribeBulk.TagAddresses.Add(allowed); payload.TagAddresses.Add(allowed);
filtered.SubscribeBulk = payload;
} }
return new SubscribeBulkConstraintPlan(filtered, tagAddresses.Count, denied, allowed.Count > 0); return new SubscribeBulkConstraintPlan(filtered, tagAddresses.Count, denied, allowed.Count > 0);
@@ -517,6 +532,11 @@ public sealed class MxAccessGatewayService(
// Mirrors FilterTagBulkAsync but produces BulkReadResult denial entries // Mirrors FilterTagBulkAsync but produces BulkReadResult denial entries
// so the reply payload merges into BulkReadReply.Results, not // so the reply payload merges into BulkReadReply.Results, not
// BulkSubscribeReply.Results. // BulkSubscribeReply.Results.
if (!constraintEnforcer.HasReadConstraints(identity))
{
return null;
}
Dictionary<int, BulkReadResult> denied = []; Dictionary<int, BulkReadResult> denied = [];
List<string> allowed = []; List<string> allowed = [];
for (int index = 0; index < tagAddresses.Count; index++) for (int index = 0; index < tagAddresses.Count; index++)
@@ -548,9 +568,14 @@ public sealed class MxAccessGatewayService(
return null; return null;
} }
MxCommand filtered = command.Clone(); MxCommand filtered = new() { Kind = command.Kind };
filtered.ReadBulk.TagAddresses.Clear(); ReadBulkCommand payload = new()
filtered.ReadBulk.TagAddresses.Add(allowed); {
ServerHandle = command.ReadBulk.ServerHandle,
TimeoutMs = command.ReadBulk.TimeoutMs,
};
payload.TagAddresses.Add(allowed);
filtered.ReadBulk = payload;
return new ReadBulkConstraintPlan(filtered, tagAddresses.Count, denied, allowed.Count > 0); return new ReadBulkConstraintPlan(filtered, tagAddresses.Count, denied, allowed.Count > 0);
} }
@@ -572,6 +597,11 @@ public sealed class MxAccessGatewayService(
// Parameterising on TEntry + getItemHandle keeps a single filter // Parameterising on TEntry + getItemHandle keeps a single filter
// routine for all four and avoids duplicating CheckWriteHandleAsync // routine for all four and avoids duplicating CheckWriteHandleAsync
// calls. // calls.
if (!constraintEnforcer.HasWriteConstraints(identity))
{
return null;
}
Dictionary<int, BulkWriteResult> denied = []; Dictionary<int, BulkWriteResult> denied = [];
List<TEntry> allowed = []; List<TEntry> allowed = [];
for (int index = 0; index < entries.Count; index++) for (int index = 0; index < entries.Count; index++)
@@ -609,33 +639,70 @@ public sealed class MxAccessGatewayService(
return null; return null;
} }
MxCommand filtered = command.Clone(); return new WriteBulkConstraintPlan(
ReplaceWriteBulkEntries(filtered, allowed); BuildFilteredWriteBulkCommand(command, allowed),
return new WriteBulkConstraintPlan(filtered, entries.Count, denied, allowed.Count > 0); entries.Count,
denied,
allowed.Count > 0);
} }
private static void ReplaceWriteBulkEntries<TEntry>(MxCommand command, IReadOnlyList<TEntry> allowed) /// <summary>
/// Builds the allowed-only bulk-write command. The allowed entries are carried over by
/// reference rather than deep-cloned: the caller only reads this command (TrackCommandReply),
/// and the copy the worker mutates and owns is the one <c>MapCommand</c> clones — the same
/// no-aliasing boundary as before. Cloning the whole command here and clearing it copied
/// every denied entry's payload (including <c>WriteSecured</c> values) for nothing.
/// </summary>
/// <typeparam name="TEntry">The per-family bulk-write entry message type.</typeparam>
/// <param name="command">The original command, read for its kind and payload scalars.</param>
/// <param name="allowed">The entries that survived constraint filtering, in original order.</param>
/// <returns>A command of the same kind carrying only the allowed entries.</returns>
private static MxCommand BuildFilteredWriteBulkCommand<TEntry>(MxCommand command, IReadOnlyList<TEntry> allowed)
where TEntry : class where TEntry : class
{ {
MxCommand filtered = new() { Kind = command.Kind };
switch (command.Kind) switch (command.Kind)
{ {
case MxCommandKind.WriteBulk: case MxCommandKind.WriteBulk:
command.WriteBulk.Entries.Clear(); {
command.WriteBulk.Entries.Add((IEnumerable<WriteBulkEntry>)allowed); WriteBulkCommand payload = new() { ServerHandle = command.WriteBulk.ServerHandle };
break; payload.Entries.Add((IEnumerable<WriteBulkEntry>)allowed);
case MxCommandKind.Write2Bulk: filtered.WriteBulk = payload;
command.Write2Bulk.Entries.Clear();
command.Write2Bulk.Entries.Add((IEnumerable<Write2BulkEntry>)allowed);
break;
case MxCommandKind.WriteSecuredBulk:
command.WriteSecuredBulk.Entries.Clear();
command.WriteSecuredBulk.Entries.Add((IEnumerable<WriteSecuredBulkEntry>)allowed);
break;
case MxCommandKind.WriteSecured2Bulk:
command.WriteSecured2Bulk.Entries.Clear();
command.WriteSecured2Bulk.Entries.Add((IEnumerable<WriteSecured2BulkEntry>)allowed);
break; break;
} }
case MxCommandKind.Write2Bulk:
{
Write2BulkCommand payload = new() { ServerHandle = command.Write2Bulk.ServerHandle };
payload.Entries.Add((IEnumerable<Write2BulkEntry>)allowed);
filtered.Write2Bulk = payload;
break;
}
case MxCommandKind.WriteSecuredBulk:
{
WriteSecuredBulkCommand payload = new() { ServerHandle = command.WriteSecuredBulk.ServerHandle };
payload.Entries.Add((IEnumerable<WriteSecuredBulkEntry>)allowed);
filtered.WriteSecuredBulk = payload;
break;
}
case MxCommandKind.WriteSecured2Bulk:
{
WriteSecured2BulkCommand payload = new() { ServerHandle = command.WriteSecured2Bulk.ServerHandle };
payload.Entries.Add((IEnumerable<WriteSecured2BulkEntry>)allowed);
filtered.WriteSecured2Bulk = payload;
break;
}
default:
// Only the four bulk-write kinds above reach FilterWriteBulkAsync, so this is
// unreachable; keep the previous behaviour (the unmodified command) rather than
// emitting a payload-less one if that ever stops holding.
return command.Clone();
}
return filtered;
} }
private async Task<BulkConstraintPlan?> FilterHandleBulkAsync( private async Task<BulkConstraintPlan?> FilterHandleBulkAsync(
@@ -647,6 +714,11 @@ public sealed class MxAccessGatewayService(
string? correlationId, string? correlationId,
CancellationToken cancellationToken) CancellationToken cancellationToken)
{ {
if (!constraintEnforcer.HasReadConstraints(identity))
{
return null;
}
Dictionary<int, SubscribeResult> denied = []; Dictionary<int, SubscribeResult> denied = [];
List<int> allowed = []; List<int> allowed = [];
for (int index = 0; index < itemHandles.Count; index++) for (int index = 0; index < itemHandles.Count; index++)
@@ -677,9 +749,10 @@ public sealed class MxAccessGatewayService(
return null; return null;
} }
MxCommand filtered = command.Clone(); MxCommand filtered = new() { Kind = command.Kind };
filtered.AdviseItemBulk.ItemHandles.Clear(); AdviseItemBulkCommand payload = new() { ServerHandle = command.AdviseItemBulk.ServerHandle };
filtered.AdviseItemBulk.ItemHandles.Add(allowed); payload.ItemHandles.Add(allowed);
filtered.AdviseItemBulk = payload;
return new SubscribeBulkConstraintPlan(filtered, itemHandles.Count, denied, allowed.Count > 0); return new SubscribeBulkConstraintPlan(filtered, itemHandles.Count, denied, allowed.Count > 0);
} }
@@ -19,10 +19,35 @@ namespace ZB.MOM.WW.MxGateway.Server.Security.Authentication;
/// </remarks> /// </remarks>
public static class GatewayApiKeyIdentityMapper public static class GatewayApiKeyIdentityMapper
{ {
private const int MaxCachedConstraintBlobs = 1024; /// <summary>
/// Maximum number of parsed constraint blobs retained in <see cref="ConstraintCache"/>.
/// Blobs are admin-controlled (one per API key), so the cap is only a memory backstop for a
/// store with an unusually large number of distinct constrained keys.
/// </summary>
internal const int MaxCachedConstraintBlobs = 1024;
/// <summary>
/// Bounded parsed-constraints cache keyed by the raw constraints JSON. The blob is parsed
/// once per authenticated RPC otherwise, so this keeps the JSON parse off the hot path.
/// Beyond <see cref="MaxCachedConstraintBlobs"/> entries the oldest insertion is evicted
/// rather than the cache refusing new entries — a hard stop at the cap would leave every key
/// admitted after it re-parsing its blob on every RPC for the process lifetime. Eviction is
/// approximate (FIFO over insertion order, not true LRU) because only the bound matters.
/// </summary>
private static readonly ConcurrentDictionary<string, ApiKeyConstraints> ConstraintCache = private static readonly ConcurrentDictionary<string, ApiKeyConstraints> ConstraintCache =
new(StringComparer.Ordinal); new(StringComparer.Ordinal);
/// <summary>
/// Insertion-order queue used to evict the oldest cache entry once the cache exceeds
/// <see cref="MaxCachedConstraintBlobs"/>. Keeping it separate leaves
/// <see cref="ConstraintCache"/> reads lock-free; the lock guards only the eviction path.
/// </summary>
private static readonly ConcurrentQueue<string> InsertionOrder = new();
private static readonly object EvictionLock = new();
/// <summary>Current cache size, exposed for tests asserting the cap is honoured.</summary>
internal static int CurrentCacheSize => ConstraintCache.Count;
private static ApiKeyConstraints DeserializeConstraints(string? constraintsJson) private static ApiKeyConstraints DeserializeConstraints(string? constraintsJson)
{ {
if (string.IsNullOrWhiteSpace(constraintsJson)) if (string.IsNullOrWhiteSpace(constraintsJson))
@@ -36,12 +61,36 @@ public static class GatewayApiKeyIdentityMapper
} }
ApiKeyConstraints parsed = ApiKeyConstraintSerializer.Deserialize(constraintsJson); ApiKeyConstraints parsed = ApiKeyConstraintSerializer.Deserialize(constraintsJson);
if (ConstraintCache.Count < MaxCachedConstraintBlobs)
// GetOrAdd returns whichever instance is in the cache after the call, so concurrent parsers
// of the same blob converge on one instance; it also avoids the TryAdd-then-read race where
// the key could be evicted between a failed TryAdd and the read back.
ApiKeyConstraints result = ConstraintCache.GetOrAdd(constraintsJson, parsed);
if (ReferenceEquals(result, parsed))
{ {
ConstraintCache.TryAdd(constraintsJson, parsed); // We were the inserter — track for FIFO eviction and bound the cache.
InsertionOrder.Enqueue(constraintsJson);
EvictIfOverCapacity();
} }
return parsed; return result;
}
private static void EvictIfOverCapacity()
{
if (ConstraintCache.Count <= MaxCachedConstraintBlobs)
{
return;
}
// Serialize eviction so two threads do not race past the cap together.
lock (EvictionLock)
{
while (ConstraintCache.Count > MaxCachedConstraintBlobs && InsertionOrder.TryDequeue(out string? oldest))
{
ConstraintCache.TryRemove(oldest, out _);
}
}
} }
/// <summary> /// <summary>
@@ -16,6 +16,14 @@ public sealed class ConstraintEnforcer(
IGalaxyHierarchyCache cache, IGalaxyHierarchyCache cache,
IAuditWriter auditWriter) : IConstraintEnforcer IAuditWriter auditWriter) : IConstraintEnforcer
{ {
/// <inheritdoc />
public bool HasReadConstraints(ApiKeyIdentity? identity) =>
identity?.EffectiveConstraints.HasReadConstraints ?? false;
/// <inheritdoc />
public bool HasWriteConstraints(ApiKeyIdentity? identity) =>
identity?.EffectiveConstraints.HasWriteConstraints ?? false;
/// <inheritdoc /> /// <inheritdoc />
public Task<ConstraintFailure?> CheckReadTagAsync( public Task<ConstraintFailure?> CheckReadTagAsync(
ApiKeyIdentity? identity, ApiKeyIdentity? identity,
@@ -211,7 +219,25 @@ public sealed class ConstraintEnforcer(
return true; return true;
} }
return subtreeGlobs.Any(glob => GalaxyGlobMatcher.IsMatch(containedPath, glob)) // Plain index loops rather than Any(lambda): this runs once per item of every bulk
|| tagGlobs.Any(glob => GalaxyGlobMatcher.IsMatch(tagAddress, glob)); // read/write, and the closures the lambdas capture (containedPath / tagAddress) allocate a
// display class plus a delegate per call. Same short-circuit order, same result.
for (int i = 0; i < subtreeGlobs.Count; i++)
{
if (GalaxyGlobMatcher.IsMatch(containedPath, subtreeGlobs[i]))
{
return true;
}
}
for (int i = 0; i < tagGlobs.Count; i++)
{
if (GalaxyGlobMatcher.IsMatch(tagAddress, tagGlobs[i]))
{
return true;
}
}
return false;
} }
} }
@@ -5,6 +5,30 @@ namespace ZB.MOM.WW.MxGateway.Server.Security.Authorization;
public interface IConstraintEnforcer public interface IConstraintEnforcer
{ {
/// <summary>
/// Gets a value indicating whether any read constraint applies to an identity at all, so a
/// bulk caller can hoist the question out of its per-item loop.
/// </summary>
/// <param name="identity">The API key identity.</param>
/// <returns><see langword="true"/> when at least one read constraint applies; otherwise <see langword="false"/>.</returns>
/// <remarks>
/// Every per-item <see cref="CheckReadTagAsync"/> / <see cref="CheckReadHandleAsync"/> call for
/// an unconstrained identity allows the item, so skipping the loop removes work without
/// changing a decision. The default implementation answers <see langword="true"/> — an
/// implementation that does not model constraints (test doubles, allow-all enforcers) keeps
/// being consulted per item rather than being silently bypassed.
/// </remarks>
bool HasReadConstraints(ApiKeyIdentity? identity) => true;
/// <summary>
/// Gets a value indicating whether any write constraint applies to an identity at all, the
/// write-side counterpart of <see cref="HasReadConstraints"/>.
/// </summary>
/// <param name="identity">The API key identity.</param>
/// <returns><see langword="true"/> when at least one write constraint applies; otherwise <see langword="false"/>.</returns>
/// <remarks>The same conservative default as <see cref="HasReadConstraints"/> applies.</remarks>
bool HasWriteConstraints(ApiKeyIdentity? identity) => true;
/// <summary>Checks whether a read constraint is satisfied for a tag address.</summary> /// <summary>Checks whether a read constraint is satisfied for a tag address.</summary>
/// <param name="identity">The API key identity.</param> /// <param name="identity">The API key identity.</param>
/// <param name="tagAddress">Tag address to check.</param> /// <param name="tagAddress">Tag address to check.</param>
@@ -120,6 +120,10 @@ internal static class SparseArrayExpander
case MxDataType.Boolean: case MxDataType.Boolean:
{ {
BoolArray values = new(); BoolArray values = new();
// Size the backing store once: the fill below adds exactly `length` elements,
// so without this the RepeatedField doubles its array log2(length) times.
values.Values.Capacity = length;
for (int i = 0; i < length; i++) for (int i = 0; i < length; i++)
{ {
values.Values.Add(false); values.Values.Add(false);
@@ -137,6 +141,7 @@ internal static class SparseArrayExpander
case MxDataType.Integer when UsesInt64(elements): case MxDataType.Integer when UsesInt64(elements):
{ {
Int64Array values = new(); Int64Array values = new();
values.Values.Capacity = length;
for (int i = 0; i < length; i++) for (int i = 0; i < length; i++)
{ {
values.Values.Add(0L); values.Values.Add(0L);
@@ -154,6 +159,7 @@ internal static class SparseArrayExpander
case MxDataType.Integer: case MxDataType.Integer:
{ {
Int32Array values = new(); Int32Array values = new();
values.Values.Capacity = length;
for (int i = 0; i < length; i++) for (int i = 0; i < length; i++)
{ {
values.Values.Add(0); values.Values.Add(0);
@@ -171,6 +177,7 @@ internal static class SparseArrayExpander
case MxDataType.Float: case MxDataType.Float:
{ {
FloatArray values = new(); FloatArray values = new();
values.Values.Capacity = length;
for (int i = 0; i < length; i++) for (int i = 0; i < length; i++)
{ {
values.Values.Add(0f); values.Values.Add(0f);
@@ -188,6 +195,7 @@ internal static class SparseArrayExpander
case MxDataType.Double: case MxDataType.Double:
{ {
DoubleArray values = new(); DoubleArray values = new();
values.Values.Capacity = length;
for (int i = 0; i < length; i++) for (int i = 0; i < length; i++)
{ {
values.Values.Add(0d); values.Values.Add(0d);
@@ -205,6 +213,7 @@ internal static class SparseArrayExpander
case MxDataType.String: case MxDataType.String:
{ {
StringArray values = new(); StringArray values = new();
values.Values.Capacity = length;
for (int i = 0; i < length; i++) for (int i = 0; i < length; i++)
{ {
values.Values.Add(string.Empty); values.Values.Add(string.Empty);
@@ -222,6 +231,7 @@ internal static class SparseArrayExpander
case MxDataType.Time: case MxDataType.Time:
{ {
TimestampArray values = new(); TimestampArray values = new();
values.Values.Capacity = length;
for (int i = 0; i < length; i++) for (int i = 0; i < length; i++)
{ {
values.Values.Add(new Timestamp { Seconds = 0, Nanos = 0 }); values.Values.Add(new Timestamp { Seconds = 0, Nanos = 0 });
@@ -8,10 +8,12 @@ using LibApiKeyIdentity = ZB.MOM.WW.Auth.Abstractions.ApiKeys.ApiKeyIdentity;
namespace ZB.MOM.WW.MxGateway.Tests.Security.Authentication; namespace ZB.MOM.WW.MxGateway.Tests.Security.Authentication;
/// <summary> /// <summary>
/// Hot-path decorators. Covers both mechanisms: <see cref="CachingApiKeyVerifier"/> /// Hot-path decorators. Covers all three mechanisms: <see cref="CachingApiKeyVerifier"/>
/// (read/verification coalescing plus revoke/rotate invalidation) and /// (read/verification coalescing plus revoke/rotate invalidation),
/// <see cref="CoalescingMarkApiKeyStore"/> (the <c>last_used</c> write coalescing that keeps the /// <see cref="CoalescingMarkApiKeyStore"/> (the <c>last_used</c> write coalescing that keeps the
/// per-RPC database write off the throughput ceiling). /// per-RPC database write off the throughput ceiling), and the constraint-blob cache inside
/// <see cref="GatewayApiKeyIdentityMapper"/> (which keeps the per-RPC constraints JSON parse off
/// the authenticated path).
/// </summary> /// </summary>
public sealed class CachingApiKeyVerifierTests public sealed class CachingApiKeyVerifierTests
{ {
@@ -245,6 +247,50 @@ public sealed class CachingApiKeyVerifierTests
Assert.Equal(keyId, CachingApiKeyVerifier.TryParseKeyId($"Bearer mxgw_{keyId}_secret")); Assert.Equal(keyId, CachingApiKeyVerifier.TryParseKeyId($"Bearer mxgw_{keyId}_secret"));
} }
/// <summary>
/// The mapper's constraint-blob cache is bounded by eviction, not by a hard stop at the cap:
/// once it is full the oldest entry is dropped so a newly-seen blob is still cached. A cache
/// that merely stopped accepting entries would re-parse every blob beyond the cap on every
/// single RPC, forever. Asserted behaviourally through instance identity — a cached blob maps
/// to the same <see cref="ApiKeyConstraints"/> instance, a re-parsed one does not.
/// </summary>
[Fact]
public void ToGatewayIdentity_ConstraintCacheOverCapacity_EvictsOldestAndKeepsCaching()
{
string firstJson = ConstraintsJson("Area_FifoProbe");
ApiKeyConstraints first = MapConstraints(firstJson);
Assert.Same(first, MapConstraints(firstJson));
// Push strictly more than the cap through the cache after the probe blob, so FIFO eviction
// is guaranteed to have reached it however full the (process-wide) cache already was.
for (int i = 0; i < GatewayApiKeyIdentityMapper.MaxCachedConstraintBlobs + 8; i++)
{
MapConstraints(ConstraintsJson($"Area_FifoFlood_{i}"));
}
Assert.True(
GatewayApiKeyIdentityMapper.CurrentCacheSize <= GatewayApiKeyIdentityMapper.MaxCachedConstraintBlobs,
$"cache grew to {GatewayApiKeyIdentityMapper.CurrentCacheSize} entries, past the {GatewayApiKeyIdentityMapper.MaxCachedConstraintBlobs} cap");
// Evicted, so the probe blob is parsed afresh...
ApiKeyConstraints reparsed = MapConstraints(firstJson);
Assert.NotSame(first, reparsed);
Assert.Equal(first.ReadSubtrees, reparsed.ReadSubtrees);
// ...and re-cached, rather than re-parsed on every later call.
Assert.Same(reparsed, MapConstraints(firstJson));
}
private static ApiKeyConstraints MapConstraints(string constraintsJson) =>
GatewayApiKeyIdentityMapper.ToGatewayIdentity(new LibApiKeyIdentity(
KeyId: "operator01",
DisplayName: "Operator Key",
Scopes: new HashSet<string>(StringComparer.Ordinal),
Constraints: constraintsJson)).EffectiveConstraints;
private static string ConstraintsJson(string readSubtree) =>
ApiKeyConstraintSerializer.Serialize(ApiKeyConstraints.Empty with { ReadSubtrees = [readSubtree] })!;
private static MemoryCache NewCache() => new(new MemoryCacheOptions()); private static MemoryCache NewCache() => new(new MemoryCacheOptions());
private static ApiKeyVerification Success(string keyId) => new( private static ApiKeyVerification Success(string keyId) => new(