feat(opcuaclient): #676 follow-ups — endpoint rewrite, stall wire test, fast definitive-loss recovery
- Endpoint rewrite: the driver now connects to the configured host AND port (scheme match, advertised path kept) instead of the server-advertised URL — fixes NAT/port-map upstreams and lets an in-test proxy carry the session (SDK SelectEndpoint rule). - TcpStallProxy + OpcUaClientDriverStallWireTests: the in-place blip (#670) is now wire-tested on loopback — one Bad, frozen read stamp, same session, identical value republished Good, a post-recovery change arrives, no source-time regression after the SDK's late backlog flush. - Defect found by that test, fixed: a change published right after a stall rode a Publish response the SDK timed out and never republished, leaving the node Good on the pre-change value indefinitely. New trailing reconcile read (Timeout + KeepAliveInterval after recovery) publishes any Good value newer than the last accepted one, through the replay guard's mark. - IDriverConnectionLossSource (Core.Abstractions): opt-in definitive-loss signal. DriverInstanceActor attaches in PreStart / detaches in PostStop and turns a raise into a disconnect only while Connected. OpcUaClient raises once per session on keep-alive BadSessionIdInvalid/BadSessionClosed/BadSessionNotActivated/ BadServerHalted (measured: the SDK never re-opens a halted channel) or a dropped subscription; ambiguous codes stay with the #527 watchdog. - Replay guard: one Information summary per recovery when the window closes ("dropped N late notifications across M refs"), with a timer so quiet subscriptions log it too. - ReconnectPeriod deleted (options, AdminUI form, docs, tests); stored configs that carry reconnectPeriod still parse in the factory, probe and browser. - Docs: OpcUaClient.md, DriverLifecycle.md, driver-specs.md, CLAUDE.md.
This commit is contained in:
Joseph Doherty
@@ -441,6 +441,31 @@ Two consequences worth knowing:
|
||||
**notices**, so a silent-success termination path — a stream that ends cleanly, a poll loop that exits
|
||||
without throwing — bypasses it entirely.
|
||||
|
||||
**A driver that KNOWS its connection is gone can skip the wait: `IDriverConnectionLossSource` (Gitea #676).** The
|
||||
watchdog has to prove a loss from silence, so it costs ~90 s even when the protocol already said "this connection
|
||||
will never come back". An opt-in `Core.Abstractions` interface lets a driver say so:
|
||||
`event EventHandler<DriverConnectionLossEventArgs> ConnectionLost` (a reason string). `DriverInstanceActor` attaches
|
||||
in `PreStart` and detaches in `PostStop` — the same lifetime as `IRediscoverable`, for the same reason (the `IDriver`
|
||||
outlives the actor on a respawn) — marshals each raise to the actor thread (`Self.Tell`; actor state is never touched
|
||||
on the driver's thread), and acts on it **only while `Connected`**, through the very handler the watchdog's
|
||||
`DisconnectObserved` uses (detach, record a fault, `Reconnecting`, teardown-then-init on the retry). In
|
||||
`Connecting`/`Reconnecting`/`Stubbed` it is ignored, not deferred: a re-initialise is already pending, and acting on a
|
||||
stale raise later could tear down the connection that attempt is building. The two are complementary, not
|
||||
alternatives:
|
||||
|
||||
- **The signal is for DEFINITIVE losses only.** A driver must raise only when no in-place recovery is possible, at most
|
||||
once per connection, re-armed by a re-initialise. An ambiguous failure — a timeout, a stalled socket, "no
|
||||
communication" — must not raise: a stalled link that resumes keeps its session, subscriptions and in-place recovery,
|
||||
and tearing it down would cost all three. Those stay with the watchdog, whose conjunction is the honest test for them.
|
||||
- **Today only `OpcUaClientDriver` implements it:** keep-alive `BadSessionIdInvalid` / `BadSessionClosed` /
|
||||
`BadSessionNotActivated` / `BadServerHalted`, or a subscription the server dropped (see
|
||||
[OpcUaClient.md](drivers/OpcUaClient.md) §"Upstream outage and recovery" for why `BadServerHalted` is on the list —
|
||||
it was measured, the SDK never re-opens a halted channel). A gracefully restarted upstream therefore recovers in
|
||||
about one reconnect interval instead of ~100 s; a killed one still waits for the watchdog.
|
||||
- Pinned by `DriverInstanceActorConnectionLossSignalTests` (Reconnecting within seconds while the watchdog is set to an
|
||||
hour; a falsification control with no raise that stays Connected; a raise mid-connect ignored; detach on stop) and
|
||||
`OpcUaClientConnectionLossSignalTests` (definitive raises once, ambiguous never, re-armed after re-init).
|
||||
|
||||
**The reconnect retry now tears down before re-initialising.** `Reconnecting`'s `RetryConnect` calls
|
||||
`ShutdownAsync` before `InitializeAsync` — the sequence every driver's own `ReinitializeAsync` already
|
||||
uses — because several drivers **overwrite** an `IAsyncDisposable` transport field inside `InitializeAsync`
|
||||
|
||||
@@ -50,7 +50,18 @@ provides ordered **failover** — the driver tries each candidate in turn at eve
|
||||
4840 / 4841). See
|
||||
`src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaClientDriverOptions.cs`
|
||||
for every field (security policy/mode, auth type, session timeout, keep-alive,
|
||||
browse root, node/depth caps). `ReconnectPeriod` is still parsed but no longer used (#670).
|
||||
browse root, node/depth caps). `ReconnectPeriod` was **deleted** (#676 — unused since #670 removed the SDK reconnect
|
||||
handler, and the AdminUI no longer shows it); stored configs that still carry `reconnectPeriod` keep parsing in the
|
||||
factory, the Test-Connect probe and the browse picker (unknown keys are skipped), and the form drops it on the next save.
|
||||
|
||||
### Endpoint address: the configured host and port win (#676)
|
||||
|
||||
GetEndpoints returns each endpoint with the URL the **server** believes it lives at. Behind NAT, a port map, a
|
||||
container port publish or a proxy that address is unreachable from the client — or reachable but bypassing the proxy.
|
||||
The driver therefore rewrites the selected endpoint's **host and port** to the configured URL's whenever the schemes
|
||||
match, keeping the advertised path (the SDK's own `CoreClientUtils.SelectEndpoint` rule, which the driver's
|
||||
policy-aware endpoint selection used to bypass). A different scheme is left alone; an unparseable advertised URL falls
|
||||
back to the configured one. `OpcUaClientDriver.RewriteEndpointUrl`.
|
||||
|
||||
### Session lifetime
|
||||
|
||||
@@ -83,12 +94,38 @@ The session keep-alive (`KeepAliveInterval`, default 5 s) is the liveness signal
|
||||
without moving the mark. Outside that window every notification is
|
||||
accepted whatever its timestamp, so an upstream clock that steps backwards (NTP, a PLC clock reset) never
|
||||
freezes a node. The outage Bad never moves the comparison; a re-initialise resets it.
|
||||
⚠️ **By design, inside the window a notification whose source time EQUALS the last accepted one is dropped** — so a
|
||||
status-only change that keeps the old source timestamp (not what OPC UA servers are supposed to do) is lost during
|
||||
the window only. When the window closes the driver logs ONE Information line — `post-recovery replay window closed —
|
||||
dropped N late notifications across M refs after recovery` — because the per-drop line is Debug and the
|
||||
`stale_notification_dropped` counter is OTel-only, which made the guard unobservable on the docker-dev rig (#676).
|
||||
- **Trailing re-read after an in-place recovery (#676).** Found by the TCP-stall wire test: a change the server
|
||||
published just after the stall ended rode a Publish response that had been in flight across the stall; the SDK
|
||||
timed that request out (`BadTimeout`), discarded the response and never republished the message (the following
|
||||
keep-alives moved the sequence number past it), so the node stayed **Good on the pre-change value indefinitely**.
|
||||
`Timeout + KeepAliveInterval` after the recovery — when every request in flight across the stall has completed or
|
||||
timed out — every subscribed ref is read once more, and any Good value **newer** than the last one accepted for that
|
||||
ref is published (through the replay guard's mark while the window is open, so it can never regress a node). An
|
||||
Information line names how many refs it had to repair.
|
||||
- **The session does not come back** (an upstream restart: the server forgot the session) ⇒ the driver stays
|
||||
`Degraded` with its read stamp frozen, which is exactly the #527 comms-loss watchdog's trip condition. After 90 s
|
||||
`DriverInstanceActor` runs `ShutdownAsync` + `InitializeAsync` (retried every 10 s while the server is down) and
|
||||
re-subscribes data + alarms; the new subscription's initial values come back Good. Alarm last-told state survives
|
||||
the re-initialise (#619), so the post-subscribe `ConditionRefresh` raises no un-ack flicker; the host's reconnect
|
||||
annotation restores condition quality.
|
||||
`Degraded` with its read stamp frozen, which is the #527 comms-loss watchdog's trip condition; `DriverInstanceActor`
|
||||
runs `ShutdownAsync` + `InitializeAsync` (retried every 10 s while the server is down) and re-subscribes data +
|
||||
alarms; the new subscription's initial values come back Good. Alarm last-told state survives the re-initialise
|
||||
(#619), so the post-subscribe `ConditionRefresh` raises no un-ack flicker; the host's reconnect annotation restores
|
||||
condition quality.
|
||||
- **Fast recovery for a DEFINITIVELY lost session (#676).** The driver implements `IDriverConnectionLossSource` and
|
||||
raises `ConnectionLost` — **once per session**, re-armed by a re-initialise — when the loss is certain, so the host
|
||||
re-initialises at once instead of after the watchdog's 90 s. Definitive means: a keep-alive status of
|
||||
`BadSessionIdInvalid` / `BadSessionClosed` / `BadSessionNotActivated` (the server forgot, closed or never activated
|
||||
the session) or `BadServerHalted`, or a tracked subscription declared lost (below). `BadServerHalted` is on the list
|
||||
because it was **measured**: on the restart wire test a gracefully stopped server reports `BadServerHalted` (and the
|
||||
keep-alives alternate it with `BadNoCommunication`), and the SDK never re-opens a channel the server halted — 15 s
|
||||
after the server was back, the same session still failed every request with `BadServerHalted`; `BadSessionIdInvalid`
|
||||
never arrived. Everything else — `BadNoCommunication`, `BadConnectionClosed`, `BadNotConnected`,
|
||||
`BadRequestTimeout`/`BadTimeout` (what a stalled link produces) — is **ambiguous** (the link may resume and keep its
|
||||
session, see the stall test) and stays with the watchdog. Consequence: a gracefully restarted upstream recovers in
|
||||
roughly one reconnect interval (≈10 s) instead of ~100 s; an upstream that vanishes without a halt (killed, cable
|
||||
pulled) still waits for the watchdog.
|
||||
- **The subscription dies but the session lives** (an outage longer than the subscription lifetime —
|
||||
`LifetimeCount` x publishing interval, often well under the session timeout — or a server that drops it) ⇒ keep-alives
|
||||
stay Good, so they cannot be the signal. A `PublishError` of `BadNoSubscription` (while subscriptions are tracked) or
|
||||
@@ -96,7 +133,8 @@ The session keep-alive (`KeepAliveInterval`, default 5 s) is the liveness signal
|
||||
persists for one more subscription keep-alive window + 5 s across good keep-alives, declares it lost: `Degraded`
|
||||
(`"subscription lost: …"`), the outage published once, and — sticky until the session is released — **nothing**
|
||||
advances `LastSuccessfulRead` any more (not good keep-alives, not a sibling subscription's notifications), so the
|
||||
watchdog's re-initialise recreates it. Both detectors are wire-tested independently.
|
||||
watchdog's re-initialise recreates it. Both detectors are wire-tested independently. A dead subscription never comes
|
||||
back on its session, so this also raises `ConnectionLost` (#676) and the re-initialise happens at once.
|
||||
- An in-place recovery is bound to the session whose keep-alive recovered: one overtaken by a re-initialise neither
|
||||
publishes, nor degrades, nor re-arms the outage latch of the new session.
|
||||
|
||||
@@ -126,7 +164,7 @@ initialise (the AdminUI form drops them on the next save). Pinned by
|
||||
|
||||
## Capability surface
|
||||
|
||||
`OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IHostConnectivityProbe, IAlarmSource`
|
||||
`OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IHostConnectivityProbe, IAlarmSource, IDriverConnectionLossSource`
|
||||
(`src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs:31`).
|
||||
|
||||
| Capability | Path | Notes |
|
||||
@@ -253,6 +291,14 @@ the reconnect handler, #670). They are not part of `DriverHealth` or the
|
||||
- **Integration fixture** — exercises the driver against a reference OPC UA
|
||||
server (opc-plc) on the shared docker host; see
|
||||
[OpcUaClient-Test-Fixture.md](OpcUaClient-Test-Fixture.md) for the coverage map.
|
||||
- **Wire tests against the in-process server** — `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/`
|
||||
`OpcUaClientDriver*WireTests` (no Docker). `OpcUaClientDriverStallWireTests` (#676) covers the in-place blip that
|
||||
used to be live-gate-only: the driver connects through `TcpStallProxy`, an in-test TCP forwarder whose pause holds
|
||||
every byte without closing anything (which only works because of the endpoint rewrite above); it asserts one Bad,
|
||||
a frozen read stamp, recovery on the SAME session, the identical value republished Good, a change published right
|
||||
after recovery still arriving, and no source-time regression once the SDK's late backlog flush has arrived.
|
||||
`OpcUaClientDriverReconnectWireTests` covers the server restart (incl. the `BadServerHalted` → `ConnectionLost` signal)
|
||||
and the subscription-lost case.
|
||||
|
||||
## Further reading
|
||||
|
||||
|
||||
@@ -815,7 +815,6 @@ Already used by Client.Shared in the existing codebase.
|
||||
| `Password` | string? | — | For Username auth |
|
||||
| `SessionTimeoutMs` | int | 120000 | Server-negotiated session timeout |
|
||||
| `KeepAliveIntervalMs` | int | 5000 | Session keep-alive interval |
|
||||
| `ReconnectPeriodMs` | int | 5000 | Initial reconnect delay |
|
||||
| `BrowseStrategy` | enum | Full | `Full` (browse all at startup), `Lazy`, `Hybrid` |
|
||||
| `BrowseRoot` | string? | — | Optional root NodeId to mirror (default: ObjectsFolder) |
|
||||
| `AutoAcceptCertificates` | bool | false | Accept untrusted server certificates |
|
||||
|
||||
Reference in New Issue
Block a user