From 8667ce58aa8cbcc058b51d15b123fed753bc1d61 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 13 Nov 2026 03:03:56 -0500 Subject: [PATCH] =?UTF-8?q?docs(717):=20adopt=20the=20T0-proven=20first-de?= =?UTF-8?q?livery=20mechanism=20in=20spec=20=C2=A74.6/=C2=A710=20and=20pla?= =?UTF-8?q?n=20T2/T7/T13?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The decision record is keyed by (SubscriptionId, SequenceNumber); the ordinary-tag marker rides MonitoredItemNotification.Message, validated by ClientHandle and DataValue. The SDK's cached copy is never mutated. None deletes the SDK's orphan i=11314. T13 gains transfer and durable-restore legs. Claude-Session: https://claude.ai/code/session_01GT67ACmpMR7b9LGRPLijMi --- .../2026-11-11-warm-redundancy-717-design.md | 82 +++++++++++++------ .../2026-11-11-warm-redundancy-717-plan.md | 49 ++++++----- 2 files changed, 84 insertions(+), 47 deletions(-) diff --git a/docs/plans/2026-11-11-warm-redundancy-717-design.md b/docs/plans/2026-11-11-warm-redundancy-717-design.md index 3c4db1847..e98e8b020 100644 --- a/docs/plans/2026-11-11-warm-redundancy-717-design.md +++ b/docs/plans/2026-11-11-warm-redundancy-717-design.md @@ -2,7 +2,8 @@ Status: **APPROVED by the owner 2026-11-13** (#717), together with the companion plan (`2026-11-11-warm-redundancy-717-plan.md`). The §9 questions are answered; the answers are recorded in §9 and -applied throughout. Parent gate: #615 (G8 CTT, G9 production-client cutover). These stay open and separate from +applied throughout. §4.6 was amended the same day after T0: SDK 2.0.1 broke the object-keyed hooks, and the owner +adopted the T0-proven alternative (§10). Parent gate: #615 (G8 CTT, G9 production-client cutover). These stay open and separate from everything here. ## 1. Owner decisions this spec builds on (not re-opened) @@ -32,15 +33,14 @@ everything here. | Shape | `OpcUa:Redundancy:Mode` | `RedundancySupport` (i=3709) | `ServerRedundancy` type | `ServerUriArray` (i=11314) | `ServerArray` (i=2254) | Standby behaviour | |---|---|---|---|---|---|---| -| Today / not opted in | `Unset` (default) | SDK default (unchanged) | `ServerRedundancyType` (2034) | absent | self + peers (unchanged) | none; Secondary serves Good at 240 (unchanged) | -| Standalone | `None` | `None` (0) | `ServerRedundancyType` (2034) | absent | self only | never; always serves | +| Today / not opted in | `Unset` (default) | SDK default (unchanged) | `ServerRedundancyType` (2034) | not browsable; the SDK's orphan reads Good `[]` by NodeId (unchanged) | self + peers (unchanged) | none; Secondary serves Good at 240 (unchanged) | +| Standalone | `None` | `None` (0) | `ServerRedundancyType` (2034) | absent (the SDK's orphan `i=11314` is deleted, T0a) | self only | never; always serves | | Pair | `Warm` | `Warm` (2) | `NonTransparentRedundancyType` (2039) | `[effectiveSelfUri, peer…]`, read-only String[], self first (decision #155, `docs/v2/plan.md:930`) | unchanged | §4 | -- The metadata is **startup-static**. The graph is built before node-manager registration through - `StandardServer.CreateMainNodeManagerFactory` + `ConfigurationNodeManager.AddBehaviourToPredefinedNode`. That - route is spike-proven on SDK 1.5.378.106 (#717 c7791) and becomes `AddBehaviourToPredefinedNodeAsync` on 2.0.1. - The passive predefined `i=11314` the SDK visits later must resolve to the same constructed instance, or the wire - value is null (c7791). Changing mode or peers requires a restart. +- The metadata is **startup-static**. On 2.0.1 it is built in `OnNodeManagerStartedAsync` through + `IPredefinedNodeSubtypeReplacer.ReplacePredefinedInstanceSubtypeAsync` on the diagnostics node manager, which keeps + the well-known child NodeId `i=11314` (T0a; the 1.5 factory route of c7791 is not needed). Changing mode or peers + requires a restart. - `effectiveSelfUri` is the **effective** `ApplicationConfiguration.ApplicationUri` after any XML load, never the options default (c7787). - `Hot` is **not advertisable**: continuous acquisition alone does not meet Part 4 §6.6.2.4 Hot (c7794). @@ -150,6 +150,8 @@ creation from the node instance and the `AttributeId`, never from namespace or C covers the Server-object, folder and direct paths, ACL on or off, by construction. Overflow, filtered retain and the #838 deferral queue never see a suppressed event. The `RingEventQueueFactory` queue is **not** used as the gate (D6). +- **Not affected by T0.** The stamp/deliver split is product code before any SDK object exists, so the 2.0.1 + clone/pool behaviour (§4.6) does not touch it. - **Boundary.** The decision is made at report time. An event whose report ran while Active is delivered even if delivery finishes after demotion: deferred (#838) or already in an item queue. This matches the spike's observed admission boundary (c7805). Republish of already-built event messages is unchanged. @@ -166,24 +168,44 @@ creation from the node instance and the `AttributeId`, never from namespace or C ### 4.6 Strict first delivery (D4) The item-level projection (§4.4) stops new Good samples. It cannot reach samples already queued in an item, nor -`NotificationMessage`s the subscription has prebuilt or cached (c7846, c7850). For those, the design uses the -mechanism that passed the provenance spike (c7851, 66 assertions) and the delivery-history spike (c7852, -53 assertions): +`NotificationMessage`s the subscription has prebuilt or cached (c7846, c7850). The 1.5 spikes (c7851, c7852) proved +an object-keyed mechanism; **SDK 2.0.1 breaks it** (T0, `docs/plans/evidence/2026-11-13-717-sdk2/RESULTS.md`): the +`SentMessageQueue` hands every Publish and Republish a fresh `CoreUtils.Clone` of the cached message, and notification +payloads are pooled and reused across items. The owner adopted the T0-proven alternative on 2026-11-13 (#717). It keeps +the same structure (a custom item marks at build time; an override decides at first delivery) and changes both keys: -1. **Provenance.** Ordinary-tag data items are created through a custom sampled item. Its per-value `Publish` - stamps each newly created `MonitoredItemNotification` with an immutable `(NodeId, AttributeId, Category)`. The - stamp survives handle modification, item deletion and reuse. -2. **Delivery history.** `OtOpcUaSdkServer` overrides `PublishAsync` and `RepublishAsync`, calls base first, and - records **one first-delivery choice per cached message object**. The association is weak and keyed by object; - cached SDK objects are never mutated. - - If the node is Active at first delivery, the original message is the choice. - - If the node is Standby, a detached clone is the choice, with every `OrdinaryTag` notification replaced by - `BadOutOfService`/null. - - A Republish returns the recorded choice. First delivery *through* Republish is decided the same way. - - A short lock orders the choice against the role flip. No lock spans an await. -3. **Out of scope of the rewrite**, by D4: messages already in flight before the demotion step, and Republish of +1. **Ordinary-tag marker on an SDK-copied, never-encoded field.** Ordinary-tag `Value` items are created through a + custom sampled item, installed by re-implementing `IMonitoredItemManager.CreateMonitoredItem` on a + `MonitoredNodeMonitoredItemManager` subclass (the 2.0.1 `MonitoredItemFactory` hook exists only on + `AsyncCustomNodeManager`). Its per-value `Publish` runs the base first (so the timestamp filter has been applied) + and then sets `MonitoredItemNotification.Message` (a public helper reference the binary encoder never writes and + the SDK's `MemberwiseClone` copies) to an immutable marker holding the notification's `ClientHandle` and final + `DataValue`. At delivery a notification counts as ordinary-tag **only** if its marker matches its own + `ClientHandle` and `DataValue`; a stale marker left on a pooled object (`Initialize()` does not clear the helper) + fails the match and the notification is treated as unknown-origin. A false match needs the same handle and an + identical value, status and both timestamps on a recycled object. That is improbable, not impossible; it is + accepted, and the residual case can only make a Good value read projected, never the reverse. +2. **Decision record keyed by `(SubscriptionId, SequenceNumber)`, never by object.** `OtOpcUaSdkServer` overrides + `PublishAsync` and `RepublishAsync`, calls base first, and on the first delivery of a sequence number records + Active or Standby. Every later delivery of that number (Republish included) applies the recorded decision to the + fresh clone the SDK returned. + - Active: the clone is returned as is. + - Standby: the clone is projected: every marked ordinary-tag notification whose status is not Bad is replaced by + `BadOutOfService`/null (server timestamp kept). The rewrite allocates new `DataChangeNotification` / + `MonitoredItemNotification` objects; the SDK's cached copy is **never** mutated. + - A short lock orders the record against the role flip. No lock spans an await. + - The record is pruned against the `AvailableSequenceNumbers` each Publish returns and bounded per subscription + and in total, so dead subscriptions cannot grow it. +3. **Out of scope of the rewrite**, by D4: responses already in flight before the demotion step, and Republish of messages first delivered while Active. Unknown-origin notifications are not rewritten; there is no implicit policy (c7853). +4. **2.0.1 facts the matrix follows.** A Republish of a prebuilt but never-sent sequence number is refused + `BadMessageNotAvailable` by the SDK, so "first delivery through Republish" cannot occur for queued messages. + TransferSubscriptions moves items (and their markers) with the subscription; a decision record recorded under the + old session stays keyed by the same subscription id. Durable subscription restore is exercised in T13. + +A regression test pins the SDK behaviour this relies on: the helper field survives the per-delivery clone, and it is +never encoded onto the wire. An SDK upgrade that changes either fails it loudly. ### 4.7 Transition sequencing @@ -318,8 +340,7 @@ Existence is not behaviour. Plan T0 re-proves, on 2.0.1 over the wire: - (b) that **every** value read path reaches the per-tag projection: Read, initial sample, change fan-out, ResendData, size-1 queue and AggregateFilter. In particular, the new async read callbacks must not bypass a sync `OnSimpleReadValue`; if they do, the hook moves to a `ReadValueAttribute` override on `EquipmentVariableState`; -- (c) provenance through the 2.0.1 creation hook, which replaces the 1.5 manager-subclass route, including - transfer/restore; +- (c) provenance through the 2.0.1 creation hook, including transfer/restore; - (d) the Publish/Republish choice with the new signatures; - (e) suppression and promotion refresh through the product's #854/#855 per-item paths, which did not exist when the 1.5 event spike ran. @@ -327,5 +348,12 @@ Existence is not behaviour. Plan T0 re-proves, on 2.0.1 over the wire: **Result (2026-11-13):** (a) and (b) are re-proven on 2.0.1. The metadata graph uses `IPredefinedNodeSubtypeReplacer` in `OnNodeManagerStartedAsync`, which is simpler than the factory route. (c) and (d) **fail as written in §4.6**: 2.0.1 clones the cached message on every delivery and pools notification payloads, so neither object-keyed -provenance nor an object-keyed choice can work. An alternative mechanism passed the spike and awaits the owner's -decision. See `docs/plans/evidence/2026-11-13-717-sdk2/RESULTS.md`. +provenance nor an object-keyed choice can work. An alternative mechanism passed the spike; **the owner adopted it on +2026-11-13** and §4.6 now describes it: a decision record keyed by `(SubscriptionId, SequenceNumber)` and a +`(ClientHandle, DataValue)`-validated marker on `MonitoredItemNotification.Message`. The custom item is installed by +re-implementing `IMonitoredItemManager` on a `MonitoredNodeMonitoredItemManager` subclass. The T0 record is kept as +evidence: `docs/plans/evidence/2026-11-13-717-sdk2/RESULTS.md`. + +The T0 finding on the metadata graph also applies: in `None`/`Unset` the predefined `i=11314` is an orphan, readable by +NodeId though not browsable. §3's "absent" is implemented for `None` by deleting that orphan at startup; `Unset` keeps +the SDK's graph untouched (today's behaviour) and `docs/Redundancy.md` says so. diff --git a/docs/plans/2026-11-11-warm-redundancy-717-plan.md b/docs/plans/2026-11-11-warm-redundancy-717-plan.md index e9f39118d..4b45b74cf 100644 --- a/docs/plans/2026-11-11-warm-redundancy-717-plan.md +++ b/docs/plans/2026-11-11-warm-redundancy-717-plan.md @@ -45,8 +45,8 @@ D4 itself stays an owner decision. **T0 run 2026-11-13: (a) and (b) pass; (c) and (d) are no-go as written.** SDK 2.0.1 hands every Publish and Republish a fresh clone of the cached message and pools the notification payloads, so provenance and the first-delivery choice cannot be keyed by object. An alternative, keyed by `(SubscriptionId, SequenceNumber)` with a clone-surviving -provenance token, passed the same spike. It awaits the owner's go/no-go, and T1 onward are stopped until then. -Evidence: `docs/plans/evidence/2026-11-13-717-sdk2/RESULTS.md`. +provenance token, passed the same spike. **The owner adopted it on 2026-11-13**; spec §4.6 and T2/T7 below describe +it. Evidence: `docs/plans/evidence/2026-11-13-717-sdk2/RESULTS.md`. ## T1: Configuration, validation, effective self URI @@ -69,10 +69,11 @@ Evidence: `docs/plans/evidence/2026-11-13-717-sdk2/RESULTS.md`. ## T2: Standard redundancy metadata graph **Files:** -- `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaSdkServer.cs`: next to `CreateMasterNodeManager` (`:341`), add a - `CreateMainNodeManagerFactory` override that installs the T0a graph per mode. +- `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaSdkServer.cs`: in the existing `OnNodeManagerStartedAsync` + override, install the T0a graph per mode through `IPredefinedNodeSubtypeReplacer.ReplacePredefinedInstanceSubtypeAsync` + (no `CreateMainNodeManagerFactory` override is needed on 2.0.1). - Under `Warm`, the initial `Server.ServiceLevel` is set to **1** in the same pass (spec §4.3). `Unset` must not - touch the graph. + touch the graph. Under `None` the SDK's orphan `i=11314` (T0a finding) is deleted so it is absent by NodeId too. **Tests (wire):** - Per mode, check the `i=3709` value, the `i=2296` type definition, the presence or absence of `i=11314`, its @@ -152,25 +153,31 @@ Evidence: `docs/plans/evidence/2026-11-13-717-sdk2/RESULTS.md`. - Acquisition continues: flip to Active and the latest value is read without any new driver publish. - ACL on: a denied node is still `BadUserAccessDenied` and the projection never leaks existence. -## T7: Provenance and first-delivery choice (spec §4.6, D4) +## T7: Ordinary-tag marker and first-delivery decision record (spec §4.6, D4) + +The T0-adopted alternative (owner, 2026-11-13). The 1.5 object-keyed provenance and choice table are not used. **Files:** -- `OtOpcUaSdkServer.cs`: install the custom sampled item (T0c route) for ordinary-tag `Value` items only. -- `OtOpcUaSdkServer.cs`: add the `PublishAsync`/`RepublishAsync` overrides and a new `FirstDeliveryLedger`, a - `ConditionalWeakTable` keyed by the SDK message object. -- Wire into `OtOpcUaNodeManager.CreateMonitoredItems` (`:5295`) / `OnMonitoredItemCreated` (`:5442`) without - disturbing the ACL filtering or `_liveItems`. +- `OtOpcUaNodeManager`: replace `m_monitoredItemManager` with a `MonitoredNodeMonitoredItemManager` subclass that + re-implements `IMonitoredItemManager.CreateMonitoredItem` and creates a custom sampled item for ordinary-tag + (`EquipmentVariableState`) `Value` items only. Its `Publish` runs base first and then sets + `MonitoredItemNotification.Message` to an `OrdinaryTagMarker(ClientHandle, DataValue)`. +- `OtOpcUaSdkServer.cs`: `PublishAsync`/`RepublishAsync` overrides and a new `FirstDeliveryRecord` keyed by + `(SubscriptionId, SequenceNumber)`, pruned by `AvailableSequenceNumbers` and bounded. +- Wire into `OtOpcUaNodeManager.CreateMonitoredItems` / `OnMonitoredItemCreated` without disturbing ACL filtering + or `_liveItems`. -**Tests (wire):** port the spikes' assertion matrix to product code. -- Queued Good samples, then demotion, then the first Publish carries `BadOutOfService`. -- Prebuilt sequences, then demotion: the first delivery is projected and a later Republish returns the same - projected payload. -- Good delivered while Active, then demotion, then Republish returns the original Good. -- A first delivery through Republish. +**Tests (wire):** +- Queued Good samples, a ClientHandle change, then demotion: the first Publish carries `BadOutOfService`, and a + Republish returns the same projected payload. +- Prebuilt sequences (`maxNotificationsPerPublish=1`), then demotion: the next Publish is projected, and Republish + returns the recorded projected choice. A Republish of a prebuilt, never-sent number is `BadMessageNotAvailable`. +- Good delivered while Active, then demotion: Republish returns the original Good; after an ack it is + `BadMessageNotAvailable`. - A mixed subscription (ordinary tag + `i=2267` + condition-child item): only ordinary-tag items are rewritten. -- Handle modified, deleted or reused between queue and publish. -- The cached SDK object is unchanged (sacrificial-clone check). -- A bounded clone matrix (arrays, ExtensionObject, DiagnosticInfo, per c7890). +- The marker is validated: a notification whose marker names another handle or value is not rewritten (unit). +- **SDK-field regression pin:** the helper survives `CoreUtils.Clone` and is not encoded by the binary encoder. +- The record is bounded, and pruned against `AvailableSequenceNumbers`. - Concurrent Publish and role flip, by a stress loop with a deterministic barrier, not sleeps. ## T8: Promotion and demotion of live data items @@ -269,6 +276,8 @@ everything yourself. Use `otopcua-cli` from `src/Client/ZB.MOM.WW.OtOpcUa.Client | L8 standalone | site-b-2 (or a one-node overlay) with `Mode=None` | `RedundancySupport=None`, no `i=11314`, data Good | | L9 Unset control | Another node left on `Unset` | Byte-identical to pre-change: SL 240/250, SDK-default `i=3709` | | L10 writes/history | Write and HistoryRead on the Standby | The write is refused `BadInvalidState` (owner answer Q1); HistoryRead is Good. | +| L11 transfer | A UserName session's subscription on the Standby is taken over by a second session of the same identity (TransferSubscriptions), then the node is demoted/promoted | The transferred items keep their marker; a demotion still projects; a promotion delivers Good. | +| L12 durable restore | A durable subscription, if the rig's SDK configuration enables them, across a node restart | The restored items are created through the custom item (projected on the Standby). If durable subscriptions are not enabled on the rig, record that and leave the leg open. | Record evidence in `docs/plans/2026-11-xx-warm-redundancy-717-live-gate.md`. Do **not** mark G8/G9 (#615); they need CTT and a production client (Ignition 8.3 / ScadaBridge after T11).