fix(galaxyrepository): cap the per-entry filter memo at 64 signatures; bump 0.2.1 (scadaproj#41)
Both projectors memoized one full result list per filter signature with no limit, and the signature is client-controlled (tag_name_glob '*', '**', ...), so distinct globs retained memory until the next Galaxy deploy. A bounded FIFO ProjectionMemo<T> now backs both memos; an evicted signature is recomputed. - tests: ProjectionMemoBoundTests (500 distinct globs per projector, re-query after eviction) - Directory.Build.props 0.2.0 -> 0.2.1 (NOT packed or published) - code-reviews: GalaxyRepository-001 Resolved; README regenerated Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
Joseph Doherty
@@ -2,6 +2,12 @@
|
||||
|
||||
Shared Galaxy object-hierarchy SQL browse + `galaxy_repository.v1` gRPC service for the **ZB.MOM.WW SCADA family** (consumed by HistorianGateway and mxaccessgw). Build/test from this directory: `dotnet test`.
|
||||
|
||||
## 0.2.1 (2026-11-09, UNPUBLISHED)
|
||||
|
||||
Version bumped in `Directory.Build.props`, not packed or pushed; consumers stay on `0.2.0`.
|
||||
- GalaxyRepository-001 (scadaproj#41): the per-cache-entry filter memos of both projectors are now
|
||||
`ProjectionMemo<T>`, capped at 64 signatures (FIFO; an evicted signature is recomputed, same answer).
|
||||
|
||||
## Index history (migrated from the scadaproj root CLAUDE.md)
|
||||
|
||||
> Moved verbatim out of the scadaproj root `CLAUDE.md` on 2026-09-03 so it loads only when needed. **This file is now the authoritative place for this history — append new entries here and keep the root index row short.**
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
<Nullable>enable</Nullable>
|
||||
<ImplicitUsings>enable</ImplicitUsings>
|
||||
<LangVersion>latest</LangVersion>
|
||||
<Version>0.2.0</Version>
|
||||
<Version>0.2.1</Version>
|
||||
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
|
||||
<GenerateDocumentationFile>true</GenerateDocumentationFile>
|
||||
</PropertyGroup>
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
using System.Collections.Concurrent;
|
||||
using System.Runtime.CompilerServices;
|
||||
using System.Security.Cryptography;
|
||||
using System.Text;
|
||||
@@ -13,13 +12,19 @@ namespace ZB.MOM.WW.GalaxyRepository;
|
||||
/// filtered child list per cache-entry instance so repeated paging is an O(pageSize)
|
||||
/// slice rather than an O(siblings) filter scan per page. The memo is keyed on the
|
||||
/// immutable cache entry, so when the cache publishes a new entry the stale memo
|
||||
/// becomes unreachable and is reclaimed with it.
|
||||
/// becomes unreachable and is reclaimed with it. Each entry's memo is capped at
|
||||
/// <see cref="ProjectionMemo{TValue}.Capacity"/> signatures, because the signature is
|
||||
/// client-controlled (GalaxyRepository-001).
|
||||
/// </summary>
|
||||
public static class GalaxyBrowseProjector
|
||||
{
|
||||
private static readonly ConditionalWeakTable<
|
||||
GalaxyHierarchyCacheEntry,
|
||||
ConcurrentDictionary<string, FilteredChildren>> FilteredChildrenCache = new();
|
||||
ProjectionMemo<FilteredChildren>> FilteredChildrenCache = new();
|
||||
|
||||
/// <summary>Number of filter signatures currently memoized for <paramref name="entry"/> (tests).</summary>
|
||||
internal static int MemoCount(GalaxyHierarchyCacheEntry entry) =>
|
||||
FilteredChildrenCache.TryGetValue(entry, out var memo) ? memo.Count : 0;
|
||||
|
||||
/// <summary>Projects one page of direct children of the resolved parent.</summary>
|
||||
/// <param name="entry">The Galaxy hierarchy cache entry to query.</param>
|
||||
@@ -115,8 +120,8 @@ public static class GalaxyBrowseProjector
|
||||
int parentId,
|
||||
string filterSignature)
|
||||
{
|
||||
ConcurrentDictionary<string, FilteredChildren> memo =
|
||||
FilteredChildrenCache.GetValue(entry, static _ => new ConcurrentDictionary<string, FilteredChildren>(StringComparer.Ordinal));
|
||||
ProjectionMemo<FilteredChildren> memo =
|
||||
FilteredChildrenCache.GetValue(entry, static _ => new ProjectionMemo<FilteredChildren>());
|
||||
|
||||
return memo.GetOrAdd(
|
||||
filterSignature,
|
||||
|
||||
+10
-5
@@ -1,4 +1,3 @@
|
||||
using System.Collections.Concurrent;
|
||||
using System.Runtime.CompilerServices;
|
||||
using System.Security.Cryptography;
|
||||
using System.Text;
|
||||
@@ -23,9 +22,15 @@ public static class GalaxyHierarchyProjector
|
||||
/// With it, the first page builds the filtered list and each subsequent page is an
|
||||
/// O(pageSize) slice. The table is keyed on the immutable cache-entry instance, so
|
||||
/// when the cache publishes a new entry the stale memo becomes unreachable and is
|
||||
/// reclaimed with it — no explicit invalidation needed.
|
||||
/// reclaimed with it — no explicit invalidation needed. Each entry's memo is capped at
|
||||
/// <see cref="ProjectionMemo{TValue}.Capacity"/> signatures, because the signature is
|
||||
/// client-controlled (GalaxyRepository-001).
|
||||
/// </summary>
|
||||
private static readonly ConditionalWeakTable<GalaxyHierarchyCacheEntry, ConcurrentDictionary<string, IReadOnlyList<GalaxyObjectView>>> FilteredViewCache = new();
|
||||
private static readonly ConditionalWeakTable<GalaxyHierarchyCacheEntry, ProjectionMemo<IReadOnlyList<GalaxyObjectView>>> FilteredViewCache = new();
|
||||
|
||||
/// <summary>Number of filter signatures currently memoized for <paramref name="entry"/> (tests).</summary>
|
||||
internal static int MemoCount(GalaxyHierarchyCacheEntry entry) =>
|
||||
FilteredViewCache.TryGetValue(entry, out var memo) ? memo.Count : 0;
|
||||
|
||||
/// <summary>Projects a discovery request against a cache entry and returns all matching objects.</summary>
|
||||
/// <param name="entry">The Galaxy hierarchy cache entry.</param>
|
||||
@@ -111,8 +116,8 @@ public static class GalaxyHierarchyProjector
|
||||
IReadOnlyList<GalaxyObjectView> views = entry.Index.ObjectViews;
|
||||
GalaxyObjectView? root = ResolveRoot(request, entry.Index);
|
||||
|
||||
ConcurrentDictionary<string, IReadOnlyList<GalaxyObjectView>> memo =
|
||||
FilteredViewCache.GetValue(entry, static _ => new ConcurrentDictionary<string, IReadOnlyList<GalaxyObjectView>>(StringComparer.Ordinal));
|
||||
ProjectionMemo<IReadOnlyList<GalaxyObjectView>> memo =
|
||||
FilteredViewCache.GetValue(entry, static _ => new ProjectionMemo<IReadOnlyList<GalaxyObjectView>>());
|
||||
|
||||
return memo.GetOrAdd(
|
||||
filterSignature,
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
namespace ZB.MOM.WW.GalaxyRepository;
|
||||
|
||||
/// <summary>
|
||||
/// A small, bounded, thread-safe memo of filter-signature → projected result, held per
|
||||
/// immutable <see cref="GalaxyHierarchyCacheEntry"/>.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Bounded because the signature is client-controlled: <c>tag_name_glob</c> values such as
|
||||
/// <c>*</c>, <c>**</c>, <c>***</c> are all distinct signatures that each retain a full result
|
||||
/// list, and the per-entry memo is otherwise released only when a new Galaxy deploy publishes
|
||||
/// a new entry (GalaxyRepository-001). Eviction is FIFO: the memo exists to make paging through
|
||||
/// ONE filter O(pageSize), and an evicted signature is simply recomputed — the projection is a
|
||||
/// pure function of the entry and the request, so the answer and its page tokens are unchanged.
|
||||
/// The factory runs outside the lock; two racing first callers may both compute, and the first
|
||||
/// to publish wins (same contract as <c>ConcurrentDictionary.GetOrAdd</c>).
|
||||
/// </remarks>
|
||||
/// <typeparam name="TValue">The memoized projection.</typeparam>
|
||||
internal sealed class ProjectionMemo<TValue>
|
||||
where TValue : class
|
||||
{
|
||||
/// <summary>Maximum number of distinct filter signatures retained per cache entry.</summary>
|
||||
public const int Capacity = 64;
|
||||
|
||||
private readonly object _gate = new();
|
||||
private readonly Dictionary<string, TValue> _items = new(StringComparer.Ordinal);
|
||||
private readonly Queue<string> _insertionOrder = new();
|
||||
|
||||
/// <summary>Number of signatures currently retained.</summary>
|
||||
public int Count
|
||||
{
|
||||
get
|
||||
{
|
||||
lock (_gate)
|
||||
{
|
||||
return _items.Count;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Returns the memoized value for <paramref name="key"/>, computing it when absent.</summary>
|
||||
/// <typeparam name="TState">State passed to <paramref name="factory"/>.</typeparam>
|
||||
/// <param name="key">The filter signature.</param>
|
||||
/// <param name="factory">Computes the value; must be pure.</param>
|
||||
/// <param name="state">State for <paramref name="factory"/>.</param>
|
||||
public TValue GetOrAdd<TState>(string key, Func<string, TState, TValue> factory, TState state)
|
||||
{
|
||||
lock (_gate)
|
||||
{
|
||||
if (_items.TryGetValue(key, out TValue? existing))
|
||||
{
|
||||
return existing;
|
||||
}
|
||||
}
|
||||
|
||||
TValue created = factory(key, state);
|
||||
|
||||
lock (_gate)
|
||||
{
|
||||
if (_items.TryGetValue(key, out TValue? raced))
|
||||
{
|
||||
return raced;
|
||||
}
|
||||
|
||||
_items[key] = created;
|
||||
_insertionOrder.Enqueue(key);
|
||||
while (_items.Count > Capacity)
|
||||
{
|
||||
_items.Remove(_insertionOrder.Dequeue());
|
||||
}
|
||||
|
||||
return created;
|
||||
}
|
||||
}
|
||||
}
|
||||
+77
@@ -0,0 +1,77 @@
|
||||
using ZB.MOM.WW.GalaxyRepository;
|
||||
using ZB.MOM.WW.GalaxyRepository.Grpc;
|
||||
|
||||
namespace ZB.MOM.WW.GalaxyRepository.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// GalaxyRepository-001 (scadaproj#41): the per-entry filter memo was unbounded, so a client
|
||||
/// issuing distinct <c>tag_name_glob</c> values (<c>*</c>, <c>**</c>, ...) retained one full
|
||||
/// result list per glob until the next Galaxy deploy. The memo must stay capped.
|
||||
/// </summary>
|
||||
public sealed class ProjectionMemoBoundTests
|
||||
{
|
||||
private const int DistinctFilters = 500;
|
||||
|
||||
[Fact]
|
||||
public void DiscoverHierarchy_memo_is_bounded_under_distinct_client_globs()
|
||||
{
|
||||
GalaxyHierarchyCacheEntry entry = CreateEntry();
|
||||
|
||||
for (int i = 1; i <= DistinctFilters; i++)
|
||||
{
|
||||
GalaxyHierarchyProjector.Project(entry, new DiscoverHierarchyRequest { TagNameGlob = new string('*', i) }, null, 0, 1);
|
||||
}
|
||||
|
||||
Assert.InRange(GalaxyHierarchyProjector.MemoCount(entry), 1, ProjectionMemo<object>.Capacity);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BrowseChildren_memo_is_bounded_under_distinct_client_globs()
|
||||
{
|
||||
GalaxyHierarchyCacheEntry entry = CreateEntry();
|
||||
|
||||
for (int i = 1; i <= DistinctFilters; i++)
|
||||
{
|
||||
GalaxyBrowseProjector.ProjectChildren(entry, new BrowseChildrenRequest { TagNameGlob = new string('*', i) }, null, 0, 1);
|
||||
}
|
||||
|
||||
Assert.InRange(GalaxyBrowseProjector.MemoCount(entry), 1, ProjectionMemo<object>.Capacity);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Evicted_signature_is_recomputed_with_the_same_answer()
|
||||
{
|
||||
GalaxyHierarchyCacheEntry entry = CreateEntry();
|
||||
DiscoverHierarchyRequest first = new() { TagNameGlob = "Plant.*" };
|
||||
GalaxyHierarchyQueryResult before = GalaxyHierarchyProjector.Project(entry, first, null, 0, 10);
|
||||
|
||||
for (int i = 1; i <= DistinctFilters; i++)
|
||||
{
|
||||
GalaxyHierarchyProjector.Project(entry, new DiscoverHierarchyRequest { TagNameGlob = new string('*', i) }, null, 0, 1);
|
||||
}
|
||||
|
||||
GalaxyHierarchyQueryResult after = GalaxyHierarchyProjector.Project(entry, first, null, 0, 10);
|
||||
Assert.Equal(before.TotalObjectCount, after.TotalObjectCount);
|
||||
Assert.Equal(before.FilterSignature, after.FilterSignature);
|
||||
Assert.Equal(before.Objects.Select(o => o.GobjectId), after.Objects.Select(o => o.GobjectId));
|
||||
}
|
||||
|
||||
private static GalaxyHierarchyCacheEntry CreateEntry()
|
||||
{
|
||||
GalaxyObject[] objects =
|
||||
[
|
||||
new() { GobjectId = 1, ParentGobjectId = 0, IsArea = true, ContainedName = "Plant", BrowseName = "Plant", TagName = "Plant" },
|
||||
new() { GobjectId = 2, ParentGobjectId = 1, ContainedName = "Pump", BrowseName = "Pump", TagName = "Plant.Pump" },
|
||||
new() { GobjectId = 3, ParentGobjectId = 1, ContainedName = "Valve", BrowseName = "Valve", TagName = "Plant.Valve" },
|
||||
];
|
||||
return GalaxyHierarchyCacheEntry.Empty with
|
||||
{
|
||||
Status = GalaxyCacheStatus.Healthy,
|
||||
Sequence = 1,
|
||||
LastSuccessAt = DateTimeOffset.UtcNow,
|
||||
Objects = objects,
|
||||
Index = GalaxyHierarchyIndex.Build(objects),
|
||||
ObjectCount = objects.Length,
|
||||
};
|
||||
}
|
||||
}
|
||||
@@ -10,7 +10,7 @@
|
||||
| Last reviewed | 2026-11-09 |
|
||||
| Reviewer | Claude |
|
||||
| Commit reviewed | `91fb2b4` |
|
||||
| Open findings | 8 |
|
||||
| Open findings | 7 |
|
||||
|
||||
## Summary
|
||||
|
||||
@@ -64,7 +64,7 @@ closed when identity is absent, so the empty-means-unscoped contract is handled
|
||||
| Severity | Medium |
|
||||
| Priority | P2 |
|
||||
| Category | Performance & resource management |
|
||||
| Status | Open |
|
||||
| Status | Resolved |
|
||||
| Location | `ZB.MOM.WW.GalaxyRepository/src/ZB.MOM.WW.GalaxyRepository/GalaxyHierarchyProjector.cs:28` (memo insert `:117`); same pattern in `ZB.MOM.WW.GalaxyRepository/src/ZB.MOM.WW.GalaxyRepository/GalaxyBrowseProjector.cs:20` (insert `:121`) |
|
||||
|
||||
**Description**
|
||||
@@ -108,7 +108,7 @@ globs and asserts the memo size.
|
||||
|
||||
**Resolution**
|
||||
|
||||
_Unresolved._
|
||||
Resolved 2026-11-09 in the commit tagged `(scadaproj#41)` (GalaxyRepository `0.2.1`, unpublished) — new internal `ProjectionMemo<T>` (lock + dictionary + FIFO queue, `Capacity = 64`, factory runs outside the lock) replaces the unbounded `ConcurrentDictionary` in both `GalaxyHierarchyProjector.FilteredViewCache` and `GalaxyBrowseProjector.FilteredChildrenCache`. Eviction is safe because the projection is a pure function of (entry, request): an evicted signature is recomputed with the same result and page-token signature. Regression tests in `ProjectionMemoBoundTests` drive 500 distinct `tag_name_glob` values through each projector and assert the memo stays within the cap (both failed without the fix: 500 retained), plus an eviction-then-requery equality test.
|
||||
|
||||
### GalaxyRepository-002 — Browse-subtree scoping hides the ancestors of an allowed subtree, so scoped lazy browse from the root is empty
|
||||
|
||||
|
||||
@@ -40,7 +40,7 @@ code-reviews/
|
||||
|
||||
## Summary
|
||||
|
||||
10 of 10 libraries reviewed. 46 pending findings across all libraries.
|
||||
10 of 10 libraries reviewed. 45 pending findings across all libraries.
|
||||
|
||||
Coverage: Secrets, LocalDb, GalaxyRepository and Overview first reviewed, and Auth, Health,
|
||||
Theme, Audit, Configuration and Telemetry re-reviewed, on 2026-11-09 (#15). Each finding also
|
||||
@@ -50,9 +50,9 @@ carries a P1/P2/P3 priority (P1 = Critical/High, P2 = Medium, P3 = Low).
|
||||
|----------|---------------|
|
||||
| Critical | 0 |
|
||||
| High | 0 |
|
||||
| Medium | 13 |
|
||||
| Medium | 12 |
|
||||
| Low | 33 |
|
||||
| **Total** | **46** |
|
||||
| **Total** | **45** |
|
||||
|
||||
## Library Status
|
||||
|
||||
@@ -61,7 +61,7 @@ carries a P1/P2/P3 priority (P1 = Critical/High, P2 = Medium, P3 = Low).
|
||||
| [Audit](Audit/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/0/0 | 0 | 5 |
|
||||
| [Auth](Auth/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/1/3 | 4 | 10 |
|
||||
| [Configuration](Configuration/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/0/0 | 0 | 4 |
|
||||
| [GalaxyRepository](GalaxyRepository/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/2/6 | 8 | 8 |
|
||||
| [GalaxyRepository](GalaxyRepository/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/1/6 | 7 | 8 |
|
||||
| [Health](Health/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/0/1 | 1 | 7 |
|
||||
| [LocalDb](LocalDb/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/4/3 | 7 | 7 |
|
||||
| [Overview](Overview/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/0/3 | 3 | 5 |
|
||||
@@ -84,12 +84,11 @@ _None open._
|
||||
|
||||
_None open._
|
||||
|
||||
### Medium (13)
|
||||
### Medium (12)
|
||||
|
||||
| ID | Library | Title |
|
||||
|----|---------|-------|
|
||||
| Auth-007 | [Auth](Auth/findings.md) | User-bind mode never checks that the entry whose groups it returns is the account that bound |
|
||||
| GalaxyRepository-001 | [GalaxyRepository](GalaxyRepository/findings.md) | Filter memo has no size limit: client-chosen filters retain unlimited memory until the next deploy |
|
||||
| GalaxyRepository-002 | [GalaxyRepository](GalaxyRepository/findings.md) | Browse-subtree scoping hides the ancestors of an allowed subtree, so scoped lazy browse from the root is empty |
|
||||
| LocalDb-001 | [LocalDb](LocalDb/findings.md) | Sync endpoint is unauthenticated by default, and `ApiKey` is documented as "expected from the peer" |
|
||||
| LocalDb-002 | [LocalDb](LocalDb/findings.md) | Snapshot streams tables by name and rows by `pk_json` text, so FK children land before parents and are dead-lettered for good |
|
||||
|
||||
Reference in New Issue
Block a user