dohertj2/lmxopcua
1
0
Fork 0
Code Issues 32 Pull Requests Actions Packages Projects Releases Wiki Activity

Review remediation: Lane B — OPC UA address space (fable; review fable) #732

New Issue
Open
opened 2026-10-22 06:05:18 -04:00 by dohertj2 · 0 comments
dohertj2 commented 2026-10-22 06:05:18 -04:00
Owner
Copy Link
Copy Source

From the 2026-10-21 Fable system review. Findings R0x:ID refer to docs/reviews/2026-10-21/0x-*.md (location, evidence, failure scenario, fix recipe). Plan: docs/superpowers/plans/2026-10-22-review-remediation.md.

Lane B — OPC UA address space (fable; review fable)

Owns:

  • src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/**, except HistoryPaging.cs/RawPageLimit.cs (H1) and the HistoryReadEvents projection (B2)
  • Runtime/OpcUa/OpcUaPublishActor.cs

Tests: OpcUaServer.Tests, OpcUaServer.IntegrationTests

  • B1 No phantom nodes (R01:L1, L4, H2):
    • WriteValueLocked is non-creating (unknown key ⇒ skipped, counted, Debug-logged).
    • WriteAlarmCondition unknown id ⇒ logged no-op.
    • Delete the applier's equipment-id RemovedConditionState write.
    • Add a cross-map (realm,id) collision guard in EnsureFolder/EnsureVariable/MaterialiseAlarmCondition.
    • Tests:
      • A publish racing a surgical remove leaves no node.
      • A re-add after that gets the authored DataType, parent and writability (positive control).
  • B2a Stale MonitoredNode2 (R01:L2).
    • On every removal path (rebuild, RemoveVariableNode, RemoveAlarmConditionNode, RemoveEquipmentSubtree): queue BadNodeIdUnknown to the node's items, MonitoredNodes.Remove(nodeId), and drop _liveItems/_eventItems.
    • Revalidation restore re-resolves the node and does not reuse Handle.Node.
    • Integration test (real SDK): subscribe, rebuild, create a new item on the same NodeId. The new item receives values (positive control) and the old one gets BadNodeIdUnknown.
  • B3 Lock-time performance:
    • Rebuild detaches top-level subtrees, not leaves (R01:P1).
    • Per-equipment descendant index for RemoveEquipmentSubtree (P2).
    • LowerAllEventsMonitoring for the surgical removal sites (P3).
    • Notifier wiring de-quadratic (P5).
    • Benchmark test: rebuild a 10k-tag folder and remove 200 equipment from a 50k-node space. Assert completion < 1 s on CI hardware and record the before/after in the commit message.
  • B4 Write-path robustness:
    • OnEquipmentTagWrite observes t.Exception and wraps a synchronous throw (R01:L3, H5).
    • Shelve/condition timestamps use the injected TimeProvider (H4).
    • SDK request quotas + TransportQuotas configurable via OpcUa:Quotas:* with the current defaults (P6).
    • HistoryRead arms: lower the default MaxConcurrentHistoryReadBatches on small nodes, and use an IAsyncNodeManager override if the SDK line exposes HistoryReadAsync; otherwise document it (P4).
  • B5 Delete dead v3 equipment-tag materialisation (R01:U2): the composer's EquipmentTags, applier MaterialiseEquipmentTags, the surgical ChangedEquipmentTags loop, and the classifier TagDeltaIsSurgicalEligible. Keep IsEmpty semantics; the full OpcUaServer suite stays green.
  • B6 Off-thread deploy parse/plan (R02:P5).
    • DeploymentArtifact.Open + ParseComposition + AddressSpacePlanner.Compute run via PipeTo; only sink materialisation runs on the pinned thread.
    • Delete the bytes-less LoadArtifact SQL branch.
    • Test: value batches keep flowing while a 50k-node plan computes.
  • B7 Capability bits refresh (R01:U3). Re-evaluate event-notifier/HistoryWrite bits when HistorianDataSource/Updater is set after the first apply, so a late assignment no longer needs a full rebuild.
  • Documented, no change: R01:U4 (native Confirm/shelve, by design) and R01:L5 (bounded). Note both in the issue.

Lane gate: both suites green. Wave 4 docker-dev gate: a ScadaBridge-style subscription survives a surgical add, and a re-created item after a rebuild receives data (Client.CLI subscribe).

From the 2026-10-21 Fable system review. Findings `R0x:ID` refer to `docs/reviews/2026-10-21/0x-*.md` (location, evidence, failure scenario, fix recipe). Plan: `docs/superpowers/plans/2026-10-22-review-remediation.md`. ### Lane B — OPC UA address space (fable; review fable) **Owns:** - `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/**`, except `HistoryPaging.cs`/`RawPageLimit.cs` (H1) and the HistoryReadEvents projection (B2) - `Runtime/OpcUa/OpcUaPublishActor.cs` **Tests:** `OpcUaServer.Tests`, `OpcUaServer.IntegrationTests` - **B1 No phantom nodes** (`R01:L1`, `L4`, `H2`): - `WriteValueLocked` is non-creating (unknown key ⇒ skipped, counted, Debug-logged). - `WriteAlarmCondition` unknown id ⇒ logged no-op. - Delete the applier's equipment-id `RemovedConditionState` write. - Add a cross-map `(realm,id)` collision guard in `EnsureFolder`/`EnsureVariable`/`MaterialiseAlarmCondition`. - Tests: - A publish racing a surgical remove leaves no node. - A re-add after that gets the authored DataType, parent and writability (positive control). - **B2a Stale MonitoredNode2** (`R01:L2`). - On every removal path (rebuild, `RemoveVariableNode`, `RemoveAlarmConditionNode`, `RemoveEquipmentSubtree`): queue `BadNodeIdUnknown` to the node's items, `MonitoredNodes.Remove(nodeId)`, and drop `_liveItems`/`_eventItems`. - Revalidation restore re-resolves the node and does not reuse `Handle.Node`. - Integration test (real SDK): subscribe, rebuild, create a new item on the same NodeId. The new item receives values (positive control) and the old one gets `BadNodeIdUnknown`. - **B3 Lock-time performance:** - Rebuild detaches top-level subtrees, not leaves (`R01:P1`). - Per-equipment descendant index for `RemoveEquipmentSubtree` (`P2`). - `LowerAllEventsMonitoring` for the surgical removal sites (`P3`). - Notifier wiring de-quadratic (`P5`). - Benchmark test: rebuild a 10k-tag folder and remove 200 equipment from a 50k-node space. Assert completion < 1 s on CI hardware and record the before/after in the commit message. - **B4 Write-path robustness:** - `OnEquipmentTagWrite` observes `t.Exception` and wraps a synchronous throw (`R01:L3`, `H5`). - Shelve/condition timestamps use the injected `TimeProvider` (`H4`). - SDK request quotas + `TransportQuotas` configurable via `OpcUa:Quotas:*` with the current defaults (`P6`). - HistoryRead arms: lower the default `MaxConcurrentHistoryReadBatches` on small nodes, and use an `IAsyncNodeManager` override if the SDK line exposes `HistoryReadAsync`; otherwise document it (`P4`). - **B5 Delete dead v3 equipment-tag materialisation** (`R01:U2`): the composer's `EquipmentTags`, applier `MaterialiseEquipmentTags`, the surgical `ChangedEquipmentTags` loop, and the classifier `TagDeltaIsSurgicalEligible`. Keep `IsEmpty` semantics; the full OpcUaServer suite stays green. - **B6 Off-thread deploy parse/plan** (`R02:P5`). - `DeploymentArtifact.Open` + `ParseComposition` + `AddressSpacePlanner.Compute` run via `PipeTo`; only sink materialisation runs on the pinned thread. - Delete the bytes-less `LoadArtifact` SQL branch. - Test: value batches keep flowing while a 50k-node plan computes. - **B7 Capability bits refresh** (`R01:U3`). Re-evaluate event-notifier/HistoryWrite bits when `HistorianDataSource`/`Updater` is set after the first apply, so a late assignment no longer needs a full rebuild. - Documented, no change: `R01:U4` (native Confirm/shelve, by design) and `R01:L5` (bounded). Note both in the issue. **Lane gate:** both suites green. Wave 4 docker-dev gate: a ScadaBridge-style subscription survives a surgical add, and a re-created item after a rebuild receives data (Client.CLI `subscribe`).
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
auto-managed
queue management
 cross-driver
queue management
 driver/abcip
queue management
 driver/ablegacy
queue management
 driver/focas
queue management
 driver/mqtt
MQTT / Sparkplug B broker-subscribe driver
 driver/opcuaclient
queue management
 driver/s7
queue management
 driver/twincat
queue management
 phase/1
queue management
 phase/2
queue management
 phase/3
queue management
 phase/4
queue management
 phase/5
queue management
 phase/6
queue management
 queue/blocked
queue management
 queue/done
queue management
 queue/failed
queue management
 queue/in-progress
queue management
 queue/queued
queue management
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
dohertj2 (dohertj2)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#732
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?