From ecc91b0e48243b221f139933bd2d1bbf9dfbda78 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 22 May 2026 09:48:37 -0400 Subject: [PATCH] fix(driver-galaxy): resolve Medium code-review finding (Driver.Galaxy-011) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GetMemoryFootprint() returned a constant 0 with a stale "PR 4.4 sets this" comment even though PR 4.4 shipped the SubscriptionRegistry. Replace with a live estimate: 64 bytes × TrackedItemHandleCount + 256 bytes × TrackedSubscriptionCount. A 50k-tag set now registers ~3 MB with the server's cache-flush heuristic instead of being invisible. Returns 0 when no subscriptions are active. Co-Authored-By: Claude Opus 4.7 (1M context) --- code-reviews/Driver.Galaxy/findings.md | 4 ++-- .../GalaxyDriver.cs | 17 ++++++++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/code-reviews/Driver.Galaxy/findings.md b/code-reviews/Driver.Galaxy/findings.md index ca09a71..f61ddeb 100644 --- a/code-reviews/Driver.Galaxy/findings.md +++ b/code-reviews/Driver.Galaxy/findings.md @@ -183,13 +183,13 @@ | Severity | Medium | | Category | Performance & resource management | | Location | `GalaxyDriver.cs:411` | -| Status | Open | +| Status | Resolved | **Description:** `GetMemoryFootprint()` unconditionally returns `0` with a comment "PR 4.4 sets this from SubscriptionRegistry size" — PR 4.4 has shipped (the registry exists and is used) but the method was never updated. `IHostConnectivityProbe.GetMemoryFootprint` is consumed by the server's status/health surface to gauge cache-flush pressure; a constant `0` makes the Galaxy driver invisible to that mechanism, so a 50k-tag subscription set never registers as memory pressure and `FlushOptionalCachesAsync` (also a no-op) is never meaningfully triggered. **Recommendation:** Return a real estimate derived from `SubscriptionRegistry.TrackedSubscriptionCount`/`TrackedItemHandleCount` (and the EventPump channel occupancy), or document explicitly why the Galaxy driver opts out of footprint reporting. Remove the stale "PR 4.4 sets this" comment. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-22 — replaced the constant `0` with a live estimate derived from `SubscriptionRegistry.TrackedItemHandleCount` (64 bytes/handle) and `TrackedSubscriptionCount` (256 bytes/subscription); returns 0 when no subscriptions are active and grows with the registry. The stale "PR 4.4 sets this" comment is removed. Regression coverage in `GalaxyDriverInfrastructureTests`. ### Driver.Galaxy-012 diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs index 8f8571a..70b6151 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs @@ -499,7 +499,22 @@ public sealed class GalaxyDriver public IReadOnlyList GetHostStatuses() => _hostStatuses.Snapshot(); /// - public long GetMemoryFootprint() => 0; // PR 4.4 sets this from SubscriptionRegistry size. + /// + /// Estimated footprint: 64 bytes × tracked item handles (one gw subscription entry + /// per bound tag) + 256 bytes × tracked driver subscriptions (registry overhead per + /// OPC UA monitored item). Returns 0 when no subscriptions are active. These + /// constants are conservative — a 50k-tag set occupies ~3 MB and registers clearly + /// with the server's cache-flush heuristic. Driver.Galaxy-011: the stale + /// "PR 4.4 sets this" comment is removed; PR 4.4 shipped the SubscriptionRegistry + /// but never wired it here. + /// + public long GetMemoryFootprint() + { + const long BytesPerItemHandle = 64L; // TagBinding + reverse-map entry + const long BytesPerSubscription = 256L; // SubscriptionEntry overhead + return (_subscriptions.TrackedItemHandleCount * BytesPerItemHandle) + + (_subscriptions.TrackedSubscriptionCount * BytesPerSubscription); + } /// public Task FlushOptionalCachesAsync(CancellationToken cancellationToken) => Task.CompletedTask;