diff --git a/docs/plans/2026-08-15-site-events-policy-design.md b/docs/plans/2026-08-15-site-events-policy-design.md new file mode 100644 index 00000000..16894765 --- /dev/null +++ b/docs/plans/2026-08-15-site-events-policy-design.md @@ -0,0 +1,210 @@ +# site_events Volume Policy — WP3.2 Stage (a) Design Memo + +**Date:** 2026-08-15 · **Author:** fable (design) → sonnet (stage b implement) +**Parent:** `docs/plans/2026-08-14-arch-review-remediation-plan.md` §5, WP3.2 +**Finding:** #High site_events (policy half; writer mechanics already fixed in WP1.7) + +--- + +## 1. The problem, quantified + +Every script run emits **two Info rows** into `site_events` — "started" and "completed" — +from `ScriptExecutionActor` (`src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptExecutionActor.cs`, +lines ~275 and ~288). `site_events` is a CDC-replicated table (`SiteLocalDbSetup.OnReady`, +`db.RegisterReplicated("site_events")`), so each row does not cost one write — the capture +triggers (`TriggerSqlGenerator` in `~/Desktop/scadaproj/ZB.MOM.WW.LocalDb`) add **one +`__localdb_oplog` row + one `__localdb_row_version` row per INSERT**, the batch ships to the +peer and is applied there, and 30 days later the retention purge DELETEs the row — which the +same triggers capture as **an oplog tombstone + a row_version tombstone**, also shipped. + +Net: one script run ≈ **12 physical row-writes across the pair** plus two replication round +trips (insert-time and purge-time), for two rows nobody looks at unless something went wrong. +A modest site running 10 script executions/second writes ~1.7M `site_events` rows/day plus the +same again in oplog traffic. WP1.7 batched the commits and sliced the purge — the *mechanics* +are fine; this memo fixes the *volume*. + +Every other `site_events` writer is per-occurrence, not per-run: DCL connect/disconnect +(`DataConnectionActor`), alarm state transitions (`AlarmActor`, `NativeAlarmActor`), S&F +parking (`StoreAndForwardService`), script **errors** (timeout / failure / stuck-watchdog / +recursion-limit). Those stay untouched — they are the rows the log exists for. + +--- + +## 2. Decision (a) — per-run events off by default; interval summaries instead + +### Policy + +1. **Per-run Started/Completed Info events are OFF by default.** The Error emissions + (timeout, failure, stuck-watchdog in the CTS callback, recursion-limit in + `ScriptRuntimeContext`) are unchanged and unconditional. +2. **An interval aggregate replaces them:** a new `ScriptRunSummaryRecorder` (SiteRuntime) + accumulates per-`(instanceName, scriptName)` counters — started, completed, failed, + timedOut, plus total/max duration ms — incremented from the same call sites that today + emit the two Info events. A timer flush (hosted service, Site nodes only) emits **one** + site event per interval: `event_type "script"`, severity `Info`, `instance_id` NULL, + source `"ScriptRunSummary"`, message = headline totals (e.g. "1,243 runs: 1,240 completed, + 2 failed, 1 timed out across 17 scripts"), details = per-script JSON breakdown, capped at + the top 50 scripts by run count with an "others" rollup so a pathological deployment + cannot mint oversized rows. **Zero activity ⇒ no row** (the standby therefore emits + nothing naturally — only the active node runs scripts). Counters reset on flush. +3. **Per-script opt-in for debugging** restores the per-run rows for named scripts without a + redeploy (mechanism below). + +At the default interval this is ≤288 summary rows/day instead of 2 rows/run — the CDC tax on +the residual table content (errors + lifecycle + summaries) is negligible, which is what makes +Decision (b) cheap to settle on merit rather than on cost. + +### Config keys (`ScadaBridge:SiteRuntime`, on `SiteRuntimeOptions`) + +| Key | Default | Meaning | +|---|---|---| +| `PerRunScriptEvents` | `false` | Global switch back to legacy per-run Started/Completed rows. | +| `PerRunScriptEventScripts` | `[]` | Per-script opt-in list; entries `"{InstanceName}/{ScriptName}"`, `"{InstanceName}/*"` supported. Effective when the global switch is off. | +| `ScriptRunSummaryIntervalSeconds` | `300` | Summary flush interval; `0` disables summaries. Validated ≥ 0. | + +**Hot toggle, no new plumbing:** the execution path already threads `serviceProvider` +(it resolves `ISiteEventLogger` at line ~135); it additionally resolves +`IOptionsMonitor` and reads `CurrentValue` per run, so with +`reloadOnChange` on the mounted appsettings an operator can flip per-run events for one +script live, debug, and flip back — no restart, no redeploy, no template/schema change. +Fallback to the constructor-passed `SiteRuntimeOptions` when the provider is null (tests). +This keeps stage (b)'s blast radius exactly what the plan's dependency table promises: +**SiteEventLogging + one SiteRuntime call site** — the opt-in deliberately does NOT ride the +script config entity, because that would drag Template Engine, flattened config, Commons +DTOs and the deploy pipeline into a volume-policy fix. + +### What the Site Event Log page loses and what replaces it + +- **Loses:** the two Info rows per run. Operators who used them as a liveness signal get the + `ScriptRunSummary` rows instead (findable via the existing keyword search — no UI change + required), the unchanged Error rows for anything abnormal, and the existing health-report + script-error counters. Call this out in the release note: the page's texture changes. +- **Gains:** per-interval activity summaries with per-script counts and durations — strictly + more diagnostic signal per row than "started"/"completed" pairs. +- Optional follow-up (not required for stage b): surface run counts on the health snapshot + via a new default-interface `ISiteHealthCollector` member. If anyone instead reaches for a + Prometheus counter, remember `ZbTelemetryOptions.Meters` is an allowlist — an unlisted + meter exports nothing, silently. + +**WP3.1 coordination:** WP3.1 deletes `ScriptExecutionActor` and runs the guarded task under +`ScriptActor`. Per the plan's dependency table, **3.1 merges first**; stage (b) lands this +policy at wherever the two emissions live after that move. The policy is about the emission +call site, not the actor, so the design is unaffected by the move. + +--- + +## 3. Decision (b) — keep `site_events` replicated; fix the *reason* on record + +### Verdict: KEEP registered. But the plan's presumed justification is false, and the real one is different. + +The plan's default recommendation said "keep replicated — central event-log queries hit +either node." **That presumption does not survive contact with the code:** + +- The event-log query handler is a **ClusterSingleton** scoped to the site role: + `AkkaHostedService.cs` (~905–917) starts `EventLogHandlerActor` via `SingletonRegistrar` + ("event-log-handler", `role: siteRole`) and registers the **singleton proxy** as the + handler. +- Both central→site transports route through the same table: + `SiteCommandDispatcher.ResolveRoute` sends `EventLogQueryRequest` to `_eventLogHandler` — + the singleton proxy (`SiteCommandDispatcher.cs` ~165). The dispatcher is the single routing + truth for the Akka actor path AND `SiteCommandGrpcService`, so even when central's + NodeA→NodeB channel flip lands the gRPC call on the **standby**, the query is forwarded to + the **active** node's handler, whose `EventLogQueryService` reads the active node's local + SQLite. **No code path ever reads the standby's `site_events` replica while it is standby.** + +So by the plan's own test ("if nothing reads it on the peer, deregister it"), dereg would +follow — except one consumer remains, and it is the one Phase 1 built the replication for: + +- **Post-failover history continuity.** When the active node dies, the singleton restarts on + the survivor and its query service reads the survivor's local copy — which has content + *only because* it was replicated. Without registration, a failover starts an empty event + log at precisely the moment an operator is investigating an incident. That is not + hypothetical: it was the shipped pre-Phase-1 behavior (the stale comment at + `AkkaHostedService.cs` ~905 still describes it), and Phase 1 explicitly fixed it — + `SiteEventLogger`'s doc header and `SiteServiceRegistration.cs` ~79 ("so the pair stops + losing them on failover") record the intent. + +With Decision (a) removing the dominant per-run volume, the residual replication cost is +small, and the failover-continuity benefit lands exactly when the log is most valuable. +**Keep `site_events` in `RegisterReplicated`.** Rig posture note: site-b/site-c run +unreplicated by deliberate choice; there the log stays node-local and failover starts fresh — +that is the documented trade of not configuring a peer, unchanged by this memo. + +Stage (b) must also **fix the stale comment** at `AkkaHostedService.cs` ~905–908 ("The event +log is node-local SQLite and is not replicated") — the singleton rationale it states is still +correct, the replication claim is two phases out of date. The correct rationale to write: +singleton = queries always read the actively-written copy; replication = the standby's copy +exists so the singleton has history after it moves. + +### Purge traffic — coordinate with WP3.3, do not block on it + +Retention/cap purge DELETEs are trigger-captured today: every event pays the CDC tax **twice** +(insert + tombstone), and tombstones linger for `TombstoneRetention` (7 days). WP3.3's menu +includes clock-based local purges that never enter the oplog. When that API lands: + +- switch `EventLogPurgeService` to the oplog-bypassing delete, +- run the **retention** purge on **both** nodes (drop the active-node gate for it): the age + cutoff is deterministic, so both nodes converge independently without shipping tombstones. + (The 1GB **cap** purge stays per-node local hygiene; minor divergence is harmless.) +- Accepted edge: with no tombstones, a lagging peer's late oplog INSERT can resurrect an + already-expired row; the next purge tick removes it. Harmless for an operational log. + +If WP3.3's API is not merged when stage (b) lands, keep the current WP1.7 purge shape and +leave the switch as a recorded follow-up — Decision (a) already removed the bulk of the +tombstone traffic. + +--- + +## 4. Migration / compat + +- **No schema change, no wire change.** `EventLogQueryRequest`/`EventLogQueryResponse`, + `EventLogEntry`, and the opaque continuation token are untouched — central↔site version + skew is a non-issue. +- **Existing rows:** nothing to migrate; historical per-run rows age out via the 30-day + retention window. +- **Pair upgrade:** site pairs already stop/start together (topology guide), so both nodes + adopt the new emission policy in the same instant — no mixed-emission pair exists. If the + WP3.3 local-purge lands later, its first tick on the formerly-gated standby simply purges + rows the active had already deleted; converges in one tick. +- **Config:** absent keys = new defaults (per-run OFF, summaries every 300s). Operators who + want the legacy behavior set `PerRunScriptEvents: true` — byte-for-byte the old rows. +- **Release note:** Site Event Log page loses per-run Started/Completed rows by default; + `ScriptRunSummary` rows replace them; per-script opt-in documented. + +--- + +## 5. Stage (b) test plan + +1. **Default-off:** successful run emits no Info events; timeout/failure/stuck-watchdog + Error paths still emit (site-runtime tests at the post-WP3.1 call site). +2. **Global opt-in:** `PerRunScriptEvents=true` → started+completed emitted with the legacy + message shape. +3. **Per-script opt-in:** exact `"Instance/Script"` match emits; `"Instance/*"` wildcard + emits; non-matching script stays silent. +4. **Hot toggle:** fake `IOptionsMonitor` flipped mid-test changes behavior on the next run + without actor restart. +5. **Summary recorder:** N runs with mixed outcomes across scripts → exactly one flush event + with correct counts and capped details JSON (>50 scripts → top-50 + "others" rollup); + zero-activity interval → no row; counters reset after flush; concurrent increments are + race-free. +6. **Replication stance pinned:** test asserting `site_events` remains in + `SiteLocalDbSetup`'s `RegisterReplicated` set (guards Decision (b) against drive-by + "optimization"). +7. **If the WP3.3 purge switch lands in the same window:** purge deletes produce zero + `__localdb_oplog` rows; both-node retention convergence test. +8. **Live probes (Phase 4 gate):** rig site-a `site_events` growth rate before/after under + the demo script load; failover drill confirms the new active node serves pre-failover + event history (replication continuity, the reason (b) kept it). + +--- + +## 6. Affected documents (Phase 4 collects) + +- `docs/requirements/Component-SiteEventLogging.md` — volume policy, summary events, the + replication rationale (failover continuity, NOT either-node queries), purge/oplog note. +- `docs/requirements/Component-SiteRuntime.md` — per-run emission policy + the three config + keys. **This file currently carries unrelated user WIP — stage (b) must merge around it, + and this memo's commit deliberately excludes it.** +- Code comment fix (stage b, not a doc): `AkkaHostedService.cs` ~905 stale "not replicated" + rationale. +- `Component-HealthMonitoring.md` only if the optional run-count snapshot member is taken.