feat(drivers): periodic reconcile of desired vs actual driver subscriptions #488

Closed
opened 2026-07-21 03:36:41 -04:00 by dohertj2 · 1 comment
Owner

Raised while reviewing the #485 subscription guard: is there a way to periodically check for actual subscriptions, to defend against races or a subscription dying silently?

What exists today

  • DriverInstanceActor holds _desiredRefs and re-asserts them on every Connected entry (ResubscribeDesired), so a reconnect self-heals.
  • A periodic 30s health-poll timer already exists — but it only calls IDriver.GetHealth() and publishes to the AdminUI. It never touches subscriptions.
  • ISubscriptionHandle is opaque (just a DiagnosticId string), so the Core currently cannot ask a driver what it is actually subscribed to.

The gap

Nothing detects a subscription that dies while the driver remains Connected — a server-side subscription drop, a session reset the driver does not surface as a disconnect, or a handle lost to an error path. The desired set is only re-applied on a connect transition or a deploy.

Two options

A — cheap, no interface change. A periodic invariant check in DriverInstanceActor: while Connected, if _desiredRefs.Count > 0 but _subscriptionHandle is null, self-tell Subscribe. This is a state-machine consistency check, not a true probe, but it needs no driver support and could ride the existing health-poll tick.

B — real reconcile, needs an interface change. Add a member to ISubscribable (e.g. Task<IReadOnlyList<string>> GetSubscribedRefsAsync()), or liveness on ISubscriptionHandle, then periodically diff desired vs actual and re-subscribe the difference. Requires implementing it across all 7 drivers (Modbus, S7, AB CIP/Legacy, TwinCAT, FOCAS, OpcUaClient) plus Galaxy, each with different subscription mechanics (polled vs native).

Suggest A first — it is small and strictly additive — and only take B if a real occurrence shows the handle is present but dead.

Explicitly NOT this issue

This does not address deploy-time drift, which was #486 (a node claiming a revision it never applied). Re-asserting a desired set that is already correct would not have helped there. Keep the two separate.

Raised while reviewing the #485 subscription guard: is there a way to periodically check for actual subscriptions, to defend against races or a subscription dying silently? ## What exists today - `DriverInstanceActor` holds `_desiredRefs` and re-asserts them on every **Connected** entry (`ResubscribeDesired`), so a reconnect self-heals. - A periodic 30s `health-poll` timer already exists — but it only calls `IDriver.GetHealth()` and publishes to the AdminUI. It never touches subscriptions. - `ISubscriptionHandle` is **opaque** (just a `DiagnosticId` string), so the Core currently cannot ask a driver what it is actually subscribed to. ## The gap Nothing detects a subscription that dies while the driver remains Connected — a server-side subscription drop, a session reset the driver does not surface as a disconnect, or a handle lost to an error path. The desired set is only re-applied on a connect transition or a deploy. ## Two options **A — cheap, no interface change.** A periodic invariant check in `DriverInstanceActor`: while Connected, if `_desiredRefs.Count > 0` but `_subscriptionHandle is null`, self-tell `Subscribe`. This is a state-machine consistency check, not a true probe, but it needs no driver support and could ride the existing `health-poll` tick. **B — real reconcile, needs an interface change.** Add a member to `ISubscribable` (e.g. `Task<IReadOnlyList<string>> GetSubscribedRefsAsync()`), or liveness on `ISubscriptionHandle`, then periodically diff desired vs actual and re-subscribe the difference. Requires implementing it across all 7 drivers (Modbus, S7, AB CIP/Legacy, TwinCAT, FOCAS, OpcUaClient) plus Galaxy, each with different subscription mechanics (polled vs native). Suggest A first — it is small and strictly additive — and only take B if a real occurrence shows the handle is present but dead. ## Explicitly NOT this issue This does not address deploy-time drift, which was #486 (a node claiming a revision it never applied). Re-asserting a desired set that is already correct would not have helped there. Keep the two separate.
Author
Owner

Implemented on fix/issues-503-501-493-499-488, commit 4b9bcddb — option A, as suggested.

ReconcileSubscription rides the existing 30 s health-poll tick: while Connected, a driver holding a desired ref set but no live handle is re-subscribed.

One addition beyond the sketch: gated on ISubscribable. A driver that cannot subscribe at all can still be handed a desired set by a deploy, and without the guard it would be told Subscribe every tick and fail every tick, forever — turning a safety net into a log-spam generator.

Staying honest about what it is: a state-machine consistency check, not a probe. It can only see that this actor holds no handle, never that a handle is present but dead server-side, because ISubscriptionHandle is opaque. That remains option B, deferred exactly as suggested until a real occurrence shows the handle outliving the subscription.

A redundant Subscribe is possible and is accepted. A tick queued ahead of an already-queued Subscribe observes the same no-handle state and tells a second one. Harmless: HandleSubscribeAsync is a ReceiveAsync (so no tick is processed mid-subscribe) and it drops any prior subscription before establishing the new one, so it costs one extra round-trip and converges. Suppressing it would need a pending-subscribe flag threaded through every self-tell site — more state than the race is worth.

healthPollInterval became an optional Props parameter, mirroring the existing rediscoverInterval seam, so a test can drive the reconcile without waiting 30 s.

Three tests. The reconcile is proven by a stub whose first subscribe throws: nothing else would ever retry it — no reconnect, no deploy — so a second attempt can only come from the reconcile. Disabling ReconcileSubscription turns it red (verified). The two guard tests are absence assertions bounded by a positive control, not a sleep: a counting IDriverHealthPublisher makes ticks processed observable, so "still one subscribe after ≥4 ticks" means the reconcile ran and declined, not that the timer had not fired. Runtime.Tests 507/507.

Not deploy-time drift (#486) — kept separate as the issue asks.

Implemented on `fix/issues-503-501-493-499-488`, commit `4b9bcddb` — **option A**, as suggested. `ReconcileSubscription` rides the existing 30 s `health-poll` tick: while Connected, a driver holding a desired ref set but **no live handle** is re-subscribed. **One addition beyond the sketch: gated on `ISubscribable`.** A driver that cannot subscribe at all can still be handed a desired set by a deploy, and without the guard it would be told `Subscribe` every tick and fail every tick, forever — turning a safety net into a log-spam generator. **Staying honest about what it is:** a state-machine consistency check, **not** a probe. It can only see that this actor holds no handle, never that a handle is present but dead server-side, because `ISubscriptionHandle` is opaque. That remains option B, deferred exactly as suggested until a real occurrence shows the handle outliving the subscription. **A redundant `Subscribe` is possible and is accepted.** A tick queued ahead of an already-queued `Subscribe` observes the same no-handle state and tells a second one. Harmless: `HandleSubscribeAsync` is a `ReceiveAsync` (so no tick is processed mid-subscribe) and it drops any prior subscription before establishing the new one, so it costs one extra round-trip and converges. Suppressing it would need a pending-subscribe flag threaded through every self-tell site — more state than the race is worth. `healthPollInterval` became an optional `Props` parameter, mirroring the existing `rediscoverInterval` seam, so a test can drive the reconcile without waiting 30 s. **Three tests.** The reconcile is proven by a stub whose *first* subscribe throws: nothing else would ever retry it — no reconnect, no deploy — so a second attempt can only come from the reconcile. Disabling `ReconcileSubscription` turns it red (verified). The two guard tests are absence assertions bounded by a **positive control**, not a sleep: a counting `IDriverHealthPublisher` makes ticks *processed* observable, so "still one subscribe after ≥4 ticks" means the reconcile ran and declined, not that the timer had not fired. Runtime.Tests 507/507. Not deploy-time drift (#486) — kept separate as the issue asks.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#488