docs+test(site-runtime): Lever 1 final-review polish — document that the Good reset does not release an in-flight request and that the guard is inert once interval ≥ ceiling; FlappingGood asserts the retry landed during the burst instead of timing Thread.Sleep; NativeAlarmActor (same unguarded amplifier) + cancel-race residue recorded as follow-ups

Claude-Session: https://claude.ai/code/session_019ZxXT5sxZs5XrtdwYzJDPV
This commit is contained in:
Joseph Doherty
2026-08-28 15:10:21 -04:00
parent c52f14afca
commit afb5180ce0
4 changed files with 15 additions and 5 deletions
@@ -174,4 +174,6 @@ No EF migrations, no schema, no new ports. MxAccessGateway proto regen uses the
- **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). - **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. - **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. - **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.
- **`NativeAlarmActor` is the same amplifier, still unguarded (final review of Lever 1, 2026-08-28).** `NativeAlarmActor.SendSubscribe` (`SiteRuntime/Actors/NativeAlarmActor.cs:147-165`) arms a fixed `NativeAlarmRetryIntervalMs × 2` timer on send with no in-flight check and no backoff, and `HandleSubscribeResponse` (`:319-337`) never compares the correlation id — so a stale reply can cancel the timer guarding a newer request. One actor per native alarm source per instance, over the same DCL connections that wedged on 2026-08-26. **Follow-up: port Lever 1's `TagSubscribeState` shape to it** (or a Gitea issue) before the next incident finds it.
- **Cancel-race residue (Lever 1 M5):** `ICancelable.Cancel()` cannot retract a `RetryTagSubscribe` already in the mailbox, so a tick that races a Success issues one spurious full subscribe (self-healing, bounded to one). A `Confirmed` flag on `TagSubscribeState` would close it in ~4 lines; deferred until it is observed.
- **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. - **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.
+1 -1
View File
@@ -197,7 +197,7 @@ flowchart TD
2. Set quality to **uncertain** for all attributes that have a data source reference. Static attributes (no data source reference) have quality **good**. The uncertain quality persists until the first value update arrives from the Data Connection Layer, distinguishing "not yet received" from "known good" or "connection lost." 2. Set quality to **uncertain** for all attributes that have a data source reference. Static attributes (no data source reference) have quality **good**. The uncertain quality persists until the first value update arrives from the Data Connection Layer, distinguishing "not yet received" from "known good" or "connection lost."
3. Register data source references with the Data Connection Layer for subscriptions. 3. Register data source references with the Data Connection Layer for subscriptions.
> **Tag-subscribe retry (S4/UA6, hardened by Lever 1 of the MxGateway subscribe-storm design, 2026-08-28).** The Instance Actor groups its data-sourced tag paths by connection and sends one `SubscribeTagsRequest` per connection, arming a per-connection watchdog *on send* so both a failed reply and a lost reply are retried. **At most one request per connection is outstanding:** a watchdog tick while the in-flight request is younger than `TagSubscribeInFlightCeilingMs` (45 s, sized above the gateway's 30 s command deadline) only re-arms — it never issues a duplicate — so a gateway that is slow to answer is not handed a fresh N-tag subscribe every interval on top of the one it is still processing (the amplifier behind the 2026-08-26 QA freeze). Past the ceiling the reply is presumed lost and a new request goes out; a late reply for the superseded id is ignored. The retry interval is exponential: floor `TagSubscribeRetryIntervalMs` (5 s), ×2 with ±20 % jitter per failed/lost attempt, capped at `TagSubscribeRetryMaxIntervalMs` (60 s); a `Success` reply or the connection returning to `Good` quality resets it to the floor. All three keys are startup-validated (`SiteRuntimeOptionsValidator`). Design: `docs/plans/2026-08-28-mxgateway-subscribe-storm-prevention-design.md` §3; plan: `docs/plans/2026-08-28-tag-subscribe-inflight-guard-backoff.md`. > **Tag-subscribe retry (S4/UA6, hardened by Lever 1 of the MxGateway subscribe-storm design, 2026-08-28).** The Instance Actor groups its data-sourced tag paths by connection and sends one `SubscribeTagsRequest` per connection, arming a per-connection watchdog *on send* so both a failed reply and a lost reply are retried. **At most one request per connection is outstanding:** a watchdog tick while the in-flight request is younger than `TagSubscribeInFlightCeilingMs` (45 s, sized above the gateway's 30 s command deadline) only re-arms — it never issues a duplicate — so a gateway that is slow to answer is not handed a fresh N-tag subscribe every interval on top of the one it is still processing (the amplifier behind the 2026-08-26 QA freeze). Past the ceiling the reply is presumed lost and a new request goes out; a late reply for the superseded id is ignored. The retry interval is exponential: floor `TagSubscribeRetryIntervalMs` (5 s), ×2 with ±20 % jitter per failed/lost attempt, capped at `TagSubscribeRetryMaxIntervalMs` (60 s); a `Success` reply or the connection returning to `Good` quality resets it to the floor — but the `Good` reset does **not** release an outstanding request (it still owns the connection until the ceiling), so it shortens the wait only when nothing is in flight; and once the backed-off interval exceeds the ceiling the guard is inert on ticks and the interval itself is the rate limit (one re-send per 60 s at the defaults). All three keys are startup-validated (`SiteRuntimeOptionsValidator`). Design: `docs/plans/2026-08-28-mxgateway-subscribe-storm-prevention-design.md` §3; plan: `docs/plans/2026-08-28-tag-subscribe-inflight-guard-backoff.md`.
4. Create child Script Actors (one per script defined on the instance). 4. Create child Script Actors (one per script defined on the instance).
5. Create child Alarm Actors (one per alarm defined on the instance). 5. Create child Alarm Actors (one per alarm defined on the instance).
@@ -137,6 +137,15 @@ public class SiteRuntimeOptions
/// interval on top of the one it is still processing. Once older than this the /// interval on top of the one it is still processing. Once older than this the
/// reply is presumed lost and a new request is allowed. Size it above the /// reply is presumed lost and a new request is allowed. Size it above the
/// gateway's per-command deadline (MxGateway default 30 s). Default: 45000ms. /// gateway's per-command deadline (MxGateway default 30 s). Default: 45000ms.
/// <para>
/// Two consequences worth knowing: (1) the guard binds only while the retry interval
/// is shorter than this ceiling — once the backoff has grown past it every tick is
/// already older than the ceiling, so the interval itself becomes the rate limit
/// (steady state at the defaults: one re-send per 60 s); (2) a connection returning to
/// Good resets the backoff but does NOT release an outstanding request — that request
/// still owns the connection until the ceiling, so the Good reset shortens the wait
/// only when nothing is in flight.
/// </para>
/// </summary> /// </summary>
public int TagSubscribeInFlightCeilingMs { get; set; } = 45000; public int TagSubscribeInFlightCeilingMs { get; set; } = 45000;
@@ -1466,7 +1466,6 @@ public class InstanceActorTests : TestKit, IDisposable
instance.Tell(new SubscribeTagsResponse(req.CorrelationId, req.InstanceUniqueName, instance.Tell(new SubscribeTagsResponse(req.CorrelationId, req.InstanceUniqueName,
false, "busy", DateTimeOffset.UtcNow), dclProbe.Ref); false, "busy", DateTimeOffset.UtcNow), dclProbe.Ref);
var sw = System.Diagnostics.Stopwatch.StartNew();
// Twelve Goods over ~1.2 s — a naive re-arm-from-now on each would push the tick to ≥1.5 s. // Twelve Goods over ~1.2 s — a naive re-arm-from-now on each would push the tick to ≥1.5 s.
for (var i = 0; i < 12; i++) for (var i = 0; i < 12; i++)
{ {
@@ -1474,9 +1473,9 @@ public class InstanceActorTests : TestKit, IDisposable
Thread.Sleep(100); Thread.Sleep(100);
} }
// The probe already holds the retry (it landed ~300 ms after the first Good) by the time the burst ends. // The retry must have landed DURING the burst (~300 ms after the first Good), so the probe
dclProbe.ExpectMsg<SubscribeTagsRequest>(TimeSpan.FromSeconds(2)); // already holds it — assert that directly rather than timing the burst's Thread.Sleeps.
Assert.True(sw.Elapsed < TimeSpan.FromMilliseconds(1400), $"retry took {sw.Elapsed} — flapping Good deferred it"); dclProbe.ExpectMsg<SubscribeTagsRequest>(TimeSpan.FromMilliseconds(50));
} }
[Fact] [Fact]