From dce52ad08707773d20dbcb8b34016cf554cd0803 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 28 Aug 2026 14:13:02 -0400 Subject: [PATCH] =?UTF-8?q?docs(plans):=20MxGateway=20subscribe-storm=20pr?= =?UTF-8?q?evention=20design=20=E2=80=94=20reviewed=20against=20both=20rep?= =?UTF-8?q?os;=20Lever=203=20re-scoped=20to=20the=20gateway's=20per-sessio?= =?UTF-8?q?n=20command=5Ftimeout=20(no=20per-invoke=20override=20exists),?= =?UTF-8?q?=20da0bfe2b=20attributed=20to=20the=20ScadaBridge=20DCL,=20line?= =?UTF-8?q?=20refs=20refreshed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_019ZxXT5sxZs5XrtdwYzJDPV --- ...teway-subscribe-storm-prevention-design.md | 174 ++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 docs/plans/2026-08-28-mxgateway-subscribe-storm-prevention-design.md diff --git a/docs/plans/2026-08-28-mxgateway-subscribe-storm-prevention-design.md b/docs/plans/2026-08-28-mxgateway-subscribe-storm-prevention-design.md new file mode 100644 index 00000000..bbc5e8a7 --- /dev/null +++ b/docs/plans/2026-08-28-mxgateway-subscribe-storm-prevention-design.md @@ -0,0 +1,174 @@ +# MxGateway subscribe-storm prevention — design + +**Date:** 2026-08-28 +**Status:** Design (not yet implemented) — **reviewed against both repos 2026-08-28** (ScadaBridge `main` @ `a9b23556`, MxAccessGateway @ `74ba889`): no part of this design has landed in either repo; corrections from that review are folded in below (Lever 3 was re-scoped — see §5). +**Scope:** ScadaBridge (`SiteRuntime`, `DataConnectionLayer`) **and** MxAccessGateway (`~/Desktop/MxAccessGateway`: `Contracts` proto, `Worker`, `Server`) +**Motivating incident:** 2026-08-26 QA (vq03/vq04) — Z28061's 244 attributes went Bad/frozen for ~20 min. Root cause was a **transient MxAccessGateway subscribe wedge under deploy/subscribe churn**, not ScadaBridge or Galaxy. No dedicated incident record exists for 2026-08-26 yet — the analysis lives in this doc. Related prior record: `docs/deployment/deployment-records/2026-08-20-wonder-app-vq03-vq04.md` §4, the earlier **ScadaBridge DCL** hotfix `da0bfe2b` on the same connection (MxGateway adapter treating a graceful event-stream end as a disconnect — a ScadaBridge commit, not a gateway one; the 2026-08-26 freeze is a *different* failure mode, see §1). + +--- + +## 1. Problem statement + +Two independent defects compound into a self-sustaining freeze: + +1. **Amplifier (ScadaBridge).** `InstanceActor.SendTagSubscribe` arms a **fixed 5 s** retry timer (`SiteRuntimeOptions.TagSubscribeRetryIntervalMs = 5000`) **on send**, with **no in-flight guard and no backoff**. When a `SubscribeBulk` round-trip takes longer than 5 s — which it does the moment the gateway is under load — a *new* full 244-tag subscribe is issued every 5 s while the previous one is still outstanding. With two instances (Z28061 + Z28062) this stacks ~6 concurrent 244-tag subscribes per 30 s window. + - `src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs:1234` (`SendTagSubscribe`, called from `SubscribeToDcl` :1224 and the retry handler), `:1250-1254` (timer arm — the only "guard" is that re-arming overwrites the previous per-connection timer; nothing checks `_pendingTagSubscribes` before issuing), `:1264` (stale-correlation drop), `:1269` (cancel-on-success), `:314` (`RetryTagSubscribe` handler), `:391-394` (`PostStop` cancels timers). State fields `_tagsByConnection`/`_pendingTagSubscribes`/`_tagSubscribeRetryTimers` at `:185-188`. `SiteRuntimeOptions.cs:120` (`TagSubscribeRetryIntervalMs = 5000`, doc comment explicitly says "fixed interval"); `SiteRuntimeOptionsValidator.cs:77-80` checks only `> 0`. + +2. **Load (cross-repo).** In supervisory mode (`WriteUserId == 0`, how QA runs), one ScadaBridge subscribe = **1 `AddItemBulk` RPC + ~244 per-item `AdviseSupervisory` RPCs** (pipelined ×16). Each `AdviseSupervisory` is a separate STA command on the worker's single-threaded apartment. The gateway's per-command deadline is **30 s** (`MxGateway:Sessions:DefaultCommandTimeoutSeconds`), so under contention these bulk/per-item commands hit the deadline, cascade `DeadlineExceeded`, and no advise is ever established → no `OnDataChange` reaches ScadaBridge → all tags Bad. + - ScadaBridge: `src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/RealMxGatewayClient.cs:108-160` (`SubscribeBulkAsync`, supervisory branch). + - Gateway (canonical proto `src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_gateway.proto`; the copies under `clients/` and `build/` are generated mirrors): `SubscribeBulkCommand` (:325-328, `server_handle=1`, `tag_addresses=2`, no field 3) does AddItem + **plain** `Advise` per tag in one STA pass — `Worker/MxAccess/MxAccessSession.cs:628-652` (`SubscribeBulk`, with `AppendRemoveItemCleanup` rollback), `Worker/MxAccess/MxAccessCommandExecutor.cs:646` (`ExecuteSubscribeBulk`). The only supervisory entry point is the per-item `AdviseSupervisoryCommand` (proto :220-223, oneof 17; `MxAccessSession.AdviseSupervisory` :319-331). Note an `AdviseItemBulkCommand` (proto :312, oneof 29, `repeated int32 item_handles`) already exists for the *plain* kind — there is simply no supervisory counterpart, which is why ScadaBridge fans out per item. The STA really is one command at a time: `Worker/Sta/StaRuntime.cs:58-63` (single STA thread), `StaCommandDispatcher.DrainAsync` :233-268 (awaits each command fully before dequeuing the next). + +**Proof it is not MXAccess/Galaxy:** a fresh `LMXProxy` 32-bit STA probe did AddItem + AdviseSupervisory for every sampled tag in 0–5 ms while both worker processes sat < 5 % CPU. The bottleneck is command-path fan-out and retry stacking, not the OT layer. + +### Goal / non-goals + +- **Goal:** a subscribe or reconnect storm on one instance can no longer wedge the gateway session or freeze data; recovery is prompt and self-correcting. +- **Non-goal:** changing the alarm/event streaming path (`SiteStreamService`, `da0bfe2b`), the write path, or anything about Galaxy/MXAccess itself. +- **Non-goal:** all-or-nothing subscribe semantics; per-tag partial success is preserved. + +--- + +## 2. Approach (chosen: full fix) + +Three levers were considered; the full fix combines the two that address root cause, and folds in the third as a bounded safety knob: + +| Lever | What | Repo | Role | +|---|---|---|---| +| **1 — retry backoff + in-flight guard** | Stop re-sending a subscribe while one is in flight; exponential backoff with jitter on genuine failure | ScadaBridge | Removes the **amplifier** | +| **2 — bulk supervisory-advise RPC** | `SubscribeBulkCommand.supervisory` flag → worker does AddItem + `AdviseSupervisory` in one STA pass; collapses ~244 RPCs → 1 | Cross-repo, **capability-negotiated** | Removes the **load** | +| **3 — session command timeout** | ScadaBridge sets an explicit `command_timeout` at `OpenSession` (the gateway's only existing override — it is **per-session**, not per-command; see §5) | ScadaBridge | Bounded safety margin | + +Levers 1 and 2 are each sufficient to break the feedback loop; together they are defense-in-depth (1 caps concurrency, 2 cuts the per-subscribe cost by ~244×). Lever 3 is a cheap knob that stops treating a merely-slow (but healthy) bulk op as a failure — with the caveat, found in review, that the gateway has no per-command override, so the knob is session-wide. + +**Rejected as standalone:** raising `DefaultCommandTimeoutSeconds` globally (band-aid — lets the queue back up *longer*); an unnegotiated proto bump for Lever 2 (an old gateway would silently do a **plain** advise, breaking writes on a `WriteUserId==0` connection). + +--- + +## 3. Lever 1 — ScadaBridge retry backoff + in-flight guard + +**Where:** `InstanceActor` (`SiteRuntime`). Per-connection subscribe state already lives here (`_tagSubscribeRetryTimers`, `_pendingTagSubscribes`). + +**Change:** +- Track an **in-flight** flag per connection. `SendTagSubscribe` becomes: if a subscribe for this connection is already outstanding **and** not past a hard "lost reply" ceiling, do **not** issue a new one — just re-arm the watchdog. This alone stops the 5 s stacking. +- Replace the fixed 5 s re-arm with **exponential backoff + jitter**: floor `TagSubscribeRetryIntervalMs` (keep 5 s default), doubling to a new `TagSubscribeRetryMaxIntervalMs` (default 60 s), reset to the floor on a `Success` reply. Mirrors the existing DCL pattern for unresolved tags (`DataConnectionOptions.cs:17,24` `TagResolutionRetryInterval` 10 s / `TagResolutionRetryMaxInterval` 5 min, validated floor `> 0` and max `>= floor` at `DataConnectionOptionsValidator.cs:21-22,39-41`; machinery in `DataConnectionActor.cs` — `ScheduleTagResolutionRetry` :2393-2405 with its `IsTimerActive` guard, `ResetTagResolutionBackoff` :2409, `BackOffTagResolution` :2417-2419, pure step `NextTagResolutionInterval` ~:2422) — reuse the shape for consistency. +- **Lost-reply safety:** because a reply can be genuinely lost, the in-flight guard must expire. Keep a `TagSubscribeInFlightCeilingMs` (default = current gateway command timeout + margin, e.g. 45 s) after which a new subscribe is allowed even if the old correlation never answered. This preserves today's "retry fires whether the response fails or is lost" guarantee without the 5 s stacking. + +**New options** (`SiteRuntimeOptions` + validator): +- `TagSubscribeRetryMaxIntervalMs` (default 60000; validate `>= TagSubscribeRetryIntervalMs`). +- `TagSubscribeInFlightCeilingMs` (default 45000; validate `> 0`). + +**Invariants preserved:** a superseded correlation id is still ignored (`HandleSubscribeTagsResponse` :1262-1264 stale-reply drop); a `Success` still cancels the timer (:1269) and resets backoff; `PostStop` still cancels all timers (`:391-394`). + +--- + +## 4. Lever 2 — bulk supervisory-advise RPC (capability-negotiated) + +**Contract (`mxaccess_gateway.proto`), additive only:** +```proto +message SubscribeBulkCommand { + int32 server_handle = 1; + repeated string tag_addresses = 2; + bool supervisory = 3; // NEW — advise supervisory instead of plain advise +} +``` +Field 3 is unused today (verified); additive, field numbers never reused (house rule). Regen is the manual vendored-proto toggle both repos already use. **Alternative considered:** a new `AdviseSupervisoryItemBulkCommand` mirroring the existing plain `AdviseItemBulkCommand` (oneof 29) — rejected because it keeps the two-RPC AddItemBulk+AdviseBulk shape and two STA commands; the flag gives one RPC / one STA pass and reuses `SubscribeBulkReply` unchanged. + +**Capability negotiation (the back-compat guarantee):** the gateway already advertises a capability list on `OpenSessionReply.capabilities` (field 5) — today exactly nine strings: `unary-open-session`, `unary-close-session`, `unary-invoke`, `server-stream-events`, `bulk-subscribe-commands`, `bulk-read-commands`, `bulk-write-commands`, `unary-acknowledge-alarm`, `server-stream-active-alarms` (`Server/Grpc/MxAccessGatewayService.cs:59-67`; `bulk-subscribe-supervisory` does not exist anywhere yet). The reply also carries `gateway_protocol_version` (field 8) — a capability string is still the right gate (feature-keyed, not version-keyed). Add: +``` +reply.Capabilities.Add("bulk-subscribe-supervisory"); +``` +ScadaBridge reads the capability set at session open and only sets `supervisory = true` when `bulk-subscribe-supervisory` is present. Against an older gateway the capability is absent → ScadaBridge keeps today's `AddItemBulk` + per-item `AdviseSupervisory` path. **This is the correctness gate:** a plain advise on a `WriteUserId==0` connection would silently break writes, so we must never send `supervisory=true` blind. +- ScadaBridge does not currently retain the capability list in the DCL adapter — verified: `RealMxGatewayClient.ConnectAsync` (:83-84) does `OpenSessionAsync` then `RegisterAsync` and no `Capabilities` reference exists anywhere in `DataConnectionLayer`. This design adds: capture `OpenSessionReply.capabilities` into the `RealMxGatewayClient` at open, expose a `SupportsSupervisorySubscribeBulk` predicate. + +**Worker (`Worker/MxAccess/…`):** +- `MxAccessSession.SubscribeBulk` gains a `supervisory` parameter; when true it calls the MXAccess `AdviseSupervisory` (and registers `MxAccessAdviceKind.Supervisory`, exactly as the per-item path at :319-331 does) instead of plain `Advise` at :651-652 inside the same AddItem+advise loop. Per-item try/catch and `AppendRemoveItemCleanup` rollback are unchanged (partial success preserved). Long bulk commands hold the STA — keep the existing `StaRuntime.PumpPendingMessages()` (:94) behaviour the plain bulk path relies on. +- `ExecuteSubscribeBulk` passes `subscribeBulkCommand.Supervisory` through (`MxAccessCommandExecutor.cs:646`). Reply shape (`SubscribeBulkReply`, per-item handles) is unchanged. + +**ScadaBridge (`RealMxGatewayClient.SubscribeBulkAsync`, `:108`):** +- Supervisory branch (`UseSupervisoryAdvise` :53, `_writeUserId == 0`): if the gateway advertises the capability, issue **one** `SubscribeBulkAsync(_serverHandle, tagPaths, supervisory: true, ct)` instead of `AddItemBulkAsync` (:128) + the `BulkPipeline.RunAsync` (:136-146) of per-item `EnsureSupervisoryAdvisedAsync` (:142, method :591; parallelism `MxSupervisoryAdviseParallelism` = 16, `DataConnectionOptions.cs:73`). Result mapping (`MapSubscribeResults`, handle bookkeeping, failed-item handle eviction) reuses the existing plain-mode path. +- Fallback branch unchanged. Delete the now-superseded "Cross-repo follow-up" comment (`RealMxGatewayClient.cs:126-127`) since this design fulfills it. +- Plain-advise mode (a configured write user) already used single-RPC `SubscribeBulkAsync` and is untouched. + +**Net effect:** supervisory subscribe drops from `1 + N` RPCs and `N` STA commands to **1 RPC / 1 STA command** for N tags. + +--- + +## 5. Lever 3 — explicit session command timeout (re-scoped in review) + +**Correction from the 2026-08-28 review:** the gateway has **no per-command timeout override**. `MxCommandRequest` (proto :89-93) carries only `session_id`, `client_correlation_id`, `command`, and `MxAccessGrpcRequestValidator.ValidateInvoke` (:38-54) has no timeout handling. The only override is **per session**: `OpenSessionRequest.command_timeout` (field 4, `google.protobuf.Duration`), validated `> 0` (`MxAccessGrpcRequestValidator.cs:15`), resolved by `SessionManager.ResolveCommandTimeout` (:583, called from :528) to `MxGateway:Sessions:DefaultCommandTimeoutSeconds` (default 30, `SessionOptions.cs:8`) when absent, then captured on `GatewaySession.CommandTimeout` (:223,293) and applied to **every** invoke on that session (:1151). The reply echoes the resolved value as `OpenSessionReply.default_command_timeout` (field 6). ScadaBridge's DCL never sets it and never reads it back (verified — no `CommandTimeout` anywhere in `DataConnectionLayer`; only `_readTimeoutMs` for reads). + +Two ways to get a bulk-sized margin; **(a) is chosen**: + +- **(a) Session-scoped, ScadaBridge-only (chosen).** New `DataConnectionOptions.MxCommandTimeout` (default 60 s, validated `> 0`), sent as `OpenSessionRequest.command_timeout` in `RealMxGatewayClient.ConnectAsync`. Trade: it governs **every** command on the session, writes included — a wedged write would take up to 60 s instead of 30 s to fault. Acceptable: ScadaBridge's own `WriteTimeout` still bounds the script-facing write, and the gateway deadline is the backstop, not the primary. Log the echoed `default_command_timeout` at open so a gateway that clamps or ignores the request is visible. +- **(b) Per-invoke override (cross-repo, deferred).** Additive `google.protobuf.Duration command_timeout = 4` on `MxCommandRequest`, honoured in `GatewaySession` invoke resolution, capability-advertised (`per-command-timeout`). Cleaner but a second wire change for a margin that Lever 2 makes mostly moot (a single-pass bulk subscribe completes well under a second). Revisit only if (a)'s write-side widening proves a problem. + +This remains a margin, not a fix — it only stops a merely-slow-but-alive gateway from being treated as failed and re-storming. + +--- + +## 6. Data flow (after) + +``` +InstanceActor.SendTagSubscribe(conn) + └─ in-flight guard: skip if a subscribe for conn is outstanding & within ceiling + └─ else: 1 SubscribeTagsRequest ─▶ DataConnectionActor + └─ RealMxGatewayClient.SubscribeBulkAsync(tagPaths) + ├─ gateway advertises bulk-subscribe-supervisory? + │ yes → 1 RPC SubscribeBulk(supervisory=true) [session CommandTimeout = MxCommandTimeout, set at OpenSession] + │ └─ worker: AddItem + AdviseSupervisory per tag, ONE STA pass + │ no → legacy AddItemBulk + N×AdviseSupervisory (fallback) + └─ per-tag results ─▶ DCL resolves handles ─▶ OnDataChange flows + └─ Success reply → cancel timer, reset backoff to floor + └─ Failure/lost → backoff × jitter, re-arm (bounded by max interval) +``` + +--- + +## 7. Error handling & edge cases + +- **Old gateway / capability absent:** ScadaBridge falls back to today's path; no behavior change, writes stay correct. +- **Partial subscribe failure:** unchanged — per-tag success/failure preserved end to end; failed items evict their handle mapping so a later retry re-adds cleanly. +- **Lost subscribe reply:** in-flight ceiling releases the guard after `TagSubscribeInFlightCeilingMs`; a fresh subscribe then issues. No permanent stall. +- **Gateway restart mid-subscribe (the incident trigger):** connection-level fault path (`DataConnectionActor` reconnect state machine) is unchanged; on reconnect the first subscribe issues immediately (backoff reset), and with the in-flight guard the reconnect storm cannot stack. +- **Reconnect + backoff interaction:** a connection restore must reset backoff to the floor so recovery is prompt (don't inherit a stretched interval from the pre-fault failures). + +--- + +## 8. Testing + +**ScadaBridge (Lever 1):** +- Unit (`InstanceActor` tests): a slow/never-answered subscribe issues exactly **one** in-flight request until the ceiling, not one per interval; backoff doubles on repeated failure and resets on success; a connection restore resets backoff. +- Option validation: `TagSubscribeRetryMaxIntervalMs >= TagSubscribeRetryIntervalMs`; ceiling `> 0`. + +**Cross-repo (Lever 2):** +- Worker unit: `SubscribeBulk(supervisory:true)` calls `AdviseSupervisory` (fake MXAccess session asserts advise kind); partial-failure and cleanup unchanged. +- Gateway: `OpenSessionReply.capabilities` includes `bulk-subscribe-supervisory`. +- ScadaBridge adapter: with capability present → single `SubscribeBulk(supervisory=true)`; absent → legacy fan-out. Handle bookkeeping identical across both paths (round-trip test). +- Proto: additive-only guard (field 3 added, none renumbered). + +**Lever 3:** `RealMxGatewayClient.ConnectAsync` sends `OpenSessionRequest.command_timeout = MxCommandTimeout`; the fake gateway asserts the value and the echoed `default_command_timeout` is logged; option validated `> 0`. + +**Live gate (QA rig, after deploy — not part of this doc's execution):** reproduce the incident shape — redeploy Z28061 several times in quick succession while watching `health site site-a`; confirm `successfullyResolved` tracks to full without a Bad/frozen window, gateway log shows no `AddItemBulk/AdviseSupervisory timed out`, and (with Lever 2 active) the per-subscribe RPC count on the gateway session drops ~244×. + +--- + +## 9. Rollout & sequencing + +Levers are independent and independently shippable; recommended order: + +1. **Lever 1 (ScadaBridge-only).** Highest value / lowest blast radius — breaks the amplifier immediately; no gateway change. Ships and deploys as a ScadaBridge binary swap. +2. **Lever 2 (gateway first, then ScadaBridge).** Deploy the gateway side (new capability + worker flag) before ScadaBridge starts requesting it — capability negotiation makes the order safe either way, but gateway-first means the optimization lights up as soon as ScadaBridge ships. Both are additive/wire-compatible. +3. **Lever 3 (ScadaBridge-only).** Fold in with Lever 1 or Lever 2's ScadaBridge change; pure config surface. + +No EF migrations, no schema, no new ports. MxAccessGateway proto regen uses the existing manual vendored-proto toggle in both repos. + +**Cross-repo index:** update the ScadaBridge entry in `../scadaproj/CLAUDE.md` if the OPC-UA/MxGateway wire relationship summary changes (per repo rule); update `MxAccessGateway`'s own capability list docs for `bulk-subscribe-supervisory`. + +--- + +## 10. Open questions + +- **Backoff defaults:** 5 s floor → 60 s ceiling proposed; confirm the ceiling is acceptable for reconnect-recovery latency (a restore resets to floor, so steady-state recovery is unaffected). +- **Lever 3 value:** 60 s `MxCommandTimeout` assumes Lever 2 lands; without Lever 2 the legacy fan-out is N separate 30 s-bounded commands, so the session timeout barely matters for subscribe — it is the *write-side* widening (§5 trade) that decides whether 60 s is right. Size against the fallback path if shipping Lever 3 before Lever 2. +- **Incident record:** write `docs/deployment/deployment-records/2026-08-26-wonder-app-vq03-vq04-freeze.md` (or fold into the next QA deploy record) so the 2026-08-26 analysis is not only in a design doc. +- **Should the in-flight guard also live in `DataConnectionActor`** (coalescing duplicate concurrent subscribe requests for the same connection across instances), as a second layer? Deferred — `InstanceActor` is where the storm originates and is sufficient; revisit only if multi-instance coalescing proves necessary.