docs(plans): site_events volume policy design — WP3.2a
This commit is contained in:
Joseph Doherty
@@ -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<SiteRuntimeOptions>` 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.
|
||||
Reference in New Issue
Block a user