fix(site-runtime): artifact-apply watchdog + queued-apply failure on stop, boot-skip site event, enable revert, batch Ask slack (review 2026-10-21 lane A L1-L4)
L1: an artifact apply holding the slot past 10 min (ctor-overridable) logs an Error site event naming it without releasing the slot; PostStop answers queued applies Failed. L2: a boot-batch instance whose actor cannot be created emits an Error site event and leaves the deployed set. L3: a failed enable reverts the committed is_enabled (generation-bumped both ends). L4: the routed SetAttributes batch Ask is the 30 s per-write bound plus 10 s mailbox slack.
This commit is contained in:
@@ -162,14 +162,14 @@ flowchart TD
|
||||
|
||||
> **A deploy's config is deserialized once (review 2026-10-21 01/SR-P5).** The compile warm, the synchronous gate, the DCL-connection ensure and the Instance Actor constructor each used to `JsonSerializer.Deserialize<FlattenedConfiguration>` the same ~70 KB JSON — four object graphs per instance, two of them on the singleton's own thread. The warm now parses it once off-thread (`DeployCompileValidator.TryParse`), and the parsed object rides `DeployCompileWarmed` → the gate (`Validate(FlattenedConfiguration, …)`) → the redeploy buffer → `CreateInstanceActor` → a new `InstanceActor` constructor overload that takes the parsed configuration (read-only, so sharing it is safe; the Props no longer retain the JSON string either). A config that does not parse is still the gate's readable `Configuration JSON failed to deserialize` rejection. Paths that start from a stored row (enable, self-heal, boot) parse once inside `CreateInstanceActor`; a stored row that does not parse is handed to the actor as JSON, so it still fails init through the S6 path exactly as before.
|
||||
|
||||
> **Runtime names are validated before anything is applied (review 2026-10-21 01/SR-C1).** The instance unique name becomes the Instance Actor's name, and every script / alarm / native-alarm-source canonical name becomes a child actor name (`script-…`, `alarm-…`, `native-alarm-…`). Akka refuses any of these that is not a legal path element (a space, `#`, `/`, `[`, `]`, a bare `%`, a non-ASCII letter, a leading `$`), and the refusal is a **synchronous** `InvalidActorNameException` from `Context.ActorOf` — on the Deployment Manager singleton's own thread for an instance name, where the default Restart directive used to stop **every** Instance Actor on the site, lose all in-flight deploy bookkeeping, and leave the deployer's Ask to time out (so a retry repeated the outage). The site compile gate (`DeployCompileValidator`) now checks all of those names with Akka's own `ActorPath.IsValidPathElement` and rejects the deploy with `Site deploy validation failed: Invalid runtime name: …` before any reset, persistence or actor work. It deliberately does **not** decide with the Commons mirror `ActorNameRules`: the mirror is currently stricter than Akka (it refuses `"`, `(` and `)`), so gating on it would reject names that deploy today. Behind the gate, `CreateInstanceActor` catches the exception and returns a failure that each caller reports — a Failed deploy, a failed enable, an Error site event for a self-heal start, a skipped instance in the boot batch — so a row stored before names were checked can never escalate to the singleton. **Data connection names** become `DataConnectionActor` names through the DCL's shared sanitiser `ConnectionActorNames.Sanitize` (ASCII letters/digits plus `-_.*+:@&=,!~';()`, everything else → `-`, so the result is always Akka-legal). Because that mapping is not injective (`PLC A` and `PLC-A` both become `PLC-A`), the gate also rejects a config whose connection names collide after sanitising, and the DCL manager fails just the later connection (Error log + `connection` site event) instead of letting `ActorOf` restart it — the guard that also covers collisions between connections from different instances, which a per-instance gate cannot see. The Instance Actor applies the same rule to its children: a refused child name is logged and that one child skipped, exactly like a compile failure, instead of failing the whole instance's `PreStart`.
|
||||
> **Runtime names are validated before anything is applied (review 2026-10-21 01/SR-C1).** The instance unique name becomes the Instance Actor's name, and every script / alarm / native-alarm-source canonical name becomes a child actor name (`script-…`, `alarm-…`, `native-alarm-…`). Akka refuses any of these that is not a legal path element (a space, `#`, `/`, `[`, `]`, a bare `%`, a non-ASCII letter, a leading `$`), and the refusal is a **synchronous** `InvalidActorNameException` from `Context.ActorOf` — on the Deployment Manager singleton's own thread for an instance name, where the default Restart directive used to stop **every** Instance Actor on the site, lose all in-flight deploy bookkeeping, and leave the deployer's Ask to time out (so a retry repeated the outage). The site compile gate (`DeployCompileValidator`) now checks all of those names with Akka's own `ActorPath.IsValidPathElement` and rejects the deploy with `Site deploy validation failed: Invalid runtime name: …` before any reset, persistence or actor work. It deliberately does **not** decide with the Commons mirror `ActorNameRules`: the mirror is currently stricter than Akka (it refuses `"`, `(` and `)`), so gating on it would reject names that deploy today. Behind the gate, `CreateInstanceActor` catches the exception and returns a failure that each caller reports — a Failed deploy, a failed enable, an Error site event for a self-heal start, a skipped instance in the boot batch (Error site event; the name is dropped from the deployed count), and for an enable the already-committed `is_enabled=1` is reverted — so a row stored before names were checked can never escalate to the singleton. **Data connection names** become `DataConnectionActor` names through the DCL's shared sanitiser `ConnectionActorNames.Sanitize` (ASCII letters/digits plus `-_.*+:@&=,!~';()`, everything else → `-`, so the result is always Akka-legal). Because that mapping is not injective (`PLC A` and `PLC-A` both become `PLC-A`), the gate also rejects a config whose connection names collide after sanitising, and the DCL manager fails just the later connection (Error log + `connection` site event) instead of letting `ActorOf` restart it — the guard that also covers collisions between connections from different instances, which a per-instance gate cannot see. The Instance Actor applies the same rule to its children: a refused child name is logged and that one child skipped, exactly like a compile failure, instead of failing the whole instance's `PreStart`.
|
||||
|
||||
### System-Wide Artifact Handling
|
||||
- Receives updated shared scripts, external system definitions, database connection definitions, and data connection definitions from central. (Notification lists and SMTP configuration are central-only and are not deployed to sites — see Component-NotificationService.md.)
|
||||
- Stores all artifacts in local SQLite. After artifact deployment, the site is fully self-contained — all runtime configuration is read from local SQLite with no access to the central configuration database.
|
||||
- Recompiles shared scripts and makes updated code available to all Script Actors.
|
||||
- **Reconciles deletions**: each non-null artifact list is the complete set for its class, so after upserting, any stored row absent from the set is deleted (the artifact was deleted centrally — there is no per-artifact delete command). A removed shared script is also unregistered from the compiled script library, and a removed data connection's live DCL connection actor is stopped. A `null` list means "field not shipped" and reconciles nothing.
|
||||
- **Applies are serialised per site, in arrival order (review 2026-10-21 01/SR-C2).** Because each apply is "upsert my complete set, then delete every stored row not in it" and runs off the actor thread, two overlapping `DeployArtifactsCommand`s (central's per-site retry racing a fresh deploy, two operators clicking *Deploy Artifacts*) used to run concurrently, and the interleaving *B upserts Z → A deletes-except(A's set)* deleted Z — a shared script, external system, data connection or deployed secret central never removed — until the next artifact deploy. The Deployment Manager now keeps one apply in flight and queues the rest; the slot is released by a self-message sent after the apply task completes, i.e. behind that apply's own DCL push, so the final state is always exactly the last-arriving command's set. No revision comparison is made: a genuinely *older* snapshot that arrives last still wins, which is central's ordering to guarantee.
|
||||
- **Applies are serialised per site, in arrival order (review 2026-10-21 01/SR-C2).** Because each apply is "upsert my complete set, then delete every stored row not in it" and runs off the actor thread, two overlapping `DeployArtifactsCommand`s (central's per-site retry racing a fresh deploy, two operators clicking *Deploy Artifacts*) used to run concurrently, and the interleaving *B upserts Z → A deletes-except(A's set)* deleted Z — a shared script, external system, data connection or deployed secret central never removed — until the next artifact deploy. The Deployment Manager now keeps one apply in flight and queues the rest; the slot is released by a self-message sent after the apply task completes, i.e. behind that apply's own DCL push, so the final state is always exactly the last-arriving command's set. No revision comparison is made: a genuinely *older* snapshot that arrives last still wins, which is central's ordering to guarantee. An apply still holding the slot after 10 minutes is reported (Error log + `deployment` site event naming its deployment id and how many deploys are queued behind it) but **not** released, since releasing would let the next apply overlap a still-running one; and when the singleton stops, every still-queued deploy is answered `Success=false` rather than left to time out.
|
||||
|
||||
### Deployed Secrets (2026-09-28)
|
||||
- `HandleDeployArtifacts` additionally accepts the command's `Secrets` list (`SecretArtifact`, ciphertext only) and writes the full shipped set to the `deployed_secrets` table via `SiteStorageService.UpsertDeployedSecretsAsync` — upsert every shipped name, soft-remove (`is_removed = 1`) any stored name absent from the set. A `null` `Secrets` field (pre-feature central) touches nothing and answers with no `SecretsApplied`. A current central always ships a list — **empty** when the site references no secrets — so dropping a site's last `secret:` reference removes the secret from both nodes. Storage detail: *Deployed Secrets (Site SQLite / LocalDb)* below.
|
||||
@@ -237,7 +237,7 @@ flowchart TD
|
||||
- **SetAttribute** (for attributes with data source reference): Sends a write request to the Data Connection Layer. The DCL writes to the physical device. If the write fails (connection down, device rejection, timeout), the error is returned synchronously to the calling script for handling. On success, the existing subscription picks up the confirmed value from the device and sends it back as a value update, which then updates the in-memory value. The in-memory value is **not** optimistically updated.
|
||||
- **SetAttribute** (for static attributes): Updates the in-memory value and **persists the override to local SQLite**. On restart (or failover), the Instance Actor loads persisted overrides on top of the deployed configuration, preserving runtime-modified values. A redeployment of the instance resets all persisted overrides to the new deployed configuration values.
|
||||
|
||||
> **Routed multi-attribute reads and writes are one message each (review 2026-10-21 01/SR-P4).** `Route.To().GetAttributes()` / `SetAttributes()` used to cost one Ask — a temp actor, a mailbox round trip and a 30 s timer — per attribute. The Deployment Manager now sends the Instance Actor a single site-local `InstanceGetAttributesRequest` / `InstanceSetAttributesRequest` (`SiteRuntime/Messages/InstanceAttributeBatch.cs`, never on the wire). Reads are answered in one pass from one consistent point in the mailbox. Writes are applied in order exactly as the equivalent single `SetStaticAttributeCommand`s (static → in-memory + coalesced persist, data-sourced → DCL write, unknown/poison values rejected); data-sourced writes are issued concurrently and the reply waits only for those, each still bounded by its own 30 s DCL Ask (the batch Ask allows 35 s so a device timeout surfaces as that attribute's own failure). Both replies carry the per-attribute `GetAttributeResponse` / `SetStaticAttributeResponse`, so the routed response is unchanged: a failed device write or unknown attribute is still reported by name.
|
||||
> **Routed multi-attribute reads and writes are one message each (review 2026-10-21 01/SR-P4).** `Route.To().GetAttributes()` / `SetAttributes()` used to cost one Ask — a temp actor, a mailbox round trip and a 30 s timer — per attribute. The Deployment Manager now sends the Instance Actor a single site-local `InstanceGetAttributesRequest` / `InstanceSetAttributesRequest` (`SiteRuntime/Messages/InstanceAttributeBatch.cs`, never on the wire). Reads are answered in one pass from one consistent point in the mailbox. Writes are applied in order exactly as the equivalent single `SetStaticAttributeCommand`s (static → in-memory + coalesced persist, data-sourced → DCL write, unknown/poison values rejected); data-sourced writes are issued concurrently and the reply waits only for those, each still bounded by its own 30 s DCL Ask (the batch Ask allows that 30 s plus 10 s of mailbox slack, so a device timeout or a busy instance mailbox surfaces as that attribute's own failure, not as a whole-batch timeout over writes that were applied). Both replies carry the per-attribute `GetAttributeResponse` / `SetStaticAttributeResponse`, so the routed response is unchanged: a failed device write or unknown attribute is still reported by name.
|
||||
|
||||
> **Loaded overrides are republished to the children and the stream (review 2026-10-21 01/SR-U1).** The override load is asynchronous and lands *after* `PreStart` has already created the Script/Alarm Actors, whose trigger snapshots are seeded from the attribute set at that moment — the deploy **defaults**. Static attributes never re-emit a change on their own, so before this fix every trigger on an instance that restarted, failed over, was enabled or was started by the reconcile sweep evaluated an overridden attribute at its default indefinitely (an Expression alarm on `Mode == "Auto"` never raised), and the Debug View live stream showed the default, while `GetAttribute` answered correctly. The load now republishes each override that changed the in-memory value or quality (quality from the load, no `SourceExecutionId`), with waiter evaluation **off** — nothing can legitimately be waiting on a replay. The site stream / Debug View receive it as an ordinary `AttributeValueChanged`; the children receive it as a site-local **`AttributeValueSeed`** (decision 2026-10-22): **Script Actors adopt the value without running** — the snapshot, a Conditional trigger's truth value and an Expression trigger's last result (evaluated, never fired) are brought up to date, so a restart/failover never re-fires ValueChange/Conditional/Expression scripts and the next real change is judged against the real value; a WhileTrue trigger whose condition already holds is seeded true without starting its re-fire timer. **Alarm Actors evaluate the seed like any change**, so an alarm whose condition already holds raises. A static write that lands before the load is newer than the stored row, so the load skips that attribute rather than rolling it back. Overrides for attributes the deployed config no longer defines are still applied (unchanged) but not republished.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user