From 354d74d370c02a3b1221ee285b557470138761a8 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 12:21:33 -0400 Subject: [PATCH] docs: ServerHistorian HistoryRead knobs + Security:Ldap resilience keys; R2-08 status (03/S2, 03/S3) --- CLAUDE.md | 3 +++ .../R2-08-ldap-historyread-async-plan.md | 24 +++++++++++++++++++ ...-ldap-historyread-async-plan.md.tasks.json | 2 +- archreview/plans/STATUS.md | 1 + docs/Historian.md | 8 ++++++- 5 files changed, 36 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index eb60060c..1aec5946 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -265,6 +265,9 @@ authorization uses the standard `AccessLevels.HistoryRead` bit set at materializ | `CaCertificatePath` | `null` | PEM CA file pinning the gateway TLS chain; null/empty uses the OS trust store | | `CallTimeout` | `00:00:30` | Per-call deadline for each unary gateway read | | `MaxTieClusterOverfetch` | `65536` | Bounded over-fetch the HistoryRead-Raw paging uses to page within an oversized same-timestamp tie cluster (retained from the prior backend) | +| `HistoryReadBatchParallelism` | `4` | Max nodes served concurrently within one HistoryRead batch (03/S3); `⌈N/P⌉ × CallTimeout` worst case instead of sequential | +| `MaxConcurrentHistoryReadBatches` | `16` | Process-wide concurrent HistoryRead batch cap (03/S3); flood degrades to `BadTooManyOperations`. Total in-flight reads capped at 16 × 4 = 64 | +| `HistoryReadDeadline` | `00:01:00` | Per-request HistoryRead wall-clock deadline (03/S3); a node still in flight surfaces `BadTimeout`. Non-positive = unbounded | ### Alarm-history path (`AlarmHistorian` section) diff --git a/archreview/plans/R2-08-ldap-historyread-async-plan.md b/archreview/plans/R2-08-ldap-historyread-async-plan.md index 9c1b477a..84dfd255 100644 --- a/archreview/plans/R2-08-ldap-historyread-async-plan.md +++ b/archreview/plans/R2-08-ldap-historyread-async-plan.md @@ -674,3 +674,27 @@ Commit: `docs: ServerHistorian HistoryRead knobs + Security:Ldap resilience keys the usual test-run + review cadence. Risk concentrated in T11 (parallel fan-out — mitigated by the thread-safety audit + the paging regression suite) and T4/T5 (auth-path semantics — mitigated by the fail-closed default posture and behavior-neutral defaults). + +## Execution deviations (R2-08) + +- **T1 RED shape.** Written against the *current* 3-arg ctor with a `Stopwatch` timing assertion (the + plan's explicitly-allowed alternative), not the intended-ctor compile-fail — so every commit compiles. + T4 then re-pointed it at the new `Options.Create(...)` ctor to turn it green. +- **Async parking fakes.** The concurrency-cap repro (`GatedFakeLdap`) and the limiter repro + (`GatedRawFake`) had to park **asynchronously** (await a `TaskCompletionSource`), not block a thread + synchronously — a synchronous block before the first `await` deadlocks the caller (the core task never + yields its `Task`). Documented inline in both fakes. +- **T6 InternalsVisibleTo.** Driving the real `internal static OpcUaApplicationHost.HandleImpersonation` + from `Host.IntegrationTests` required adding an `InternalsVisibleTo` for that test assembly to the + OpcUaServer csproj (the plan called for the internal drive but not the grant). Additive + reversible. +- **T13 fake ignores the deadline token.** `GatedRawFake.ReadRawAsync` deliberately does **not** observe + the per-request deadline `ct`: with a small `HistoryReadDeadline`, the parked first batch would + otherwise cancel its own read and free the limiter permit before the second batch checks (race). By + parking on the release gate only, the first batch holds the permit deterministically, so the second + batch reliably hits the saturated limiter → `BadTooManyOperations`. +- **T14 sweep scope.** Per the executor's memory constraint (heavy `*.IntegrationTests` / Playwright + suites leak ~16 GB and the LDAP integration harness spins an ephemeral openldap on :3894), the + whole-solution `dotnet test` was **not** run. Instead: a full-solution `dotnet build` (clean) + the + impacted **filtered** unit suites (Host.IntegrationTests LDAP-classes only, Security.Tests, + Runtime.Tests options, OpcUaServer.Tests HistoryRead). The whole-solution + live legs are deferred to a + serial heavy pass / VPN run. diff --git a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json index 21955c78..0c434070 100644 --- a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json +++ b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json @@ -14,7 +14,7 @@ { "id": "R2-08-T11", "subject": "RunBounded fan-out in all four HistoryRead arms, single bridge per arm (turns T7 green) + upper-bound + isolation tests", "status": "completed", "blockedBy": ["R2-08-T9", "R2-08-T10"] }, { "id": "R2-08-T12", "subject": "Per-request HistoryRead deadline CTS — hung backend surfaces BadTimeout per node", "status": "completed", "blockedBy": ["R2-08-T11"] }, { "id": "R2-08-T13", "subject": "Process-wide concurrent-batch limiter — saturation degrades to BadTooManyOperations (lazy gate, bounded wait)", "status": "completed", "blockedBy": ["R2-08-T12"] }, - { "id": "R2-08-T14", "subject": "Docs (Historian.md + CLAUDE.md ServerHistorian table + security.md) + full touched-suite and whole-solution regression sweep + STATUS.md entry", "status": "pending", "blockedBy": ["R2-08-T5", "R2-08-T6", "R2-08-T13"] } + { "id": "R2-08-T14", "subject": "Docs (Historian.md + CLAUDE.md ServerHistorian table + security.md) + full touched-suite and whole-solution regression sweep + STATUS.md entry", "status": "completed", "blockedBy": ["R2-08-T5", "R2-08-T6", "R2-08-T13"] } ], "lastUpdated": "2026-07-12" } diff --git a/archreview/plans/STATUS.md b/archreview/plans/STATUS.md index 5d12857e..45e9f4cf 100644 --- a/archreview/plans/STATUS.md +++ b/archreview/plans/STATUS.md @@ -176,6 +176,7 @@ what the unit leg cannot; (c) migrate the 3 xunit v2 holdouts once Akka ships an | **R2-02** | 01/S-6 (High, rank 2), 01/U-6, 01/S-8=03/S12 (rank 4), 04/C-7, 01/S-7=03/S13 | `r2/02-resilience-config-hardening` (off `1676c8f4`) | `4be13429`…`dd82e740` (17 task commits) | **Code-complete, offline-verified.** **S-6:** parser clamps timeout/retry/breaker overrides (timeout≤0→tier default, breaker==1→2, caps) with diagnostics + belt-and-braces `Build` clamp so pipeline construction can never throw Polly `ValidationException` (new `ResiliencePolicyRanges` in Core.Abstractions); brick-repro + `Build_NeverThrows_ForHostileDirectOptions` + factory-seam wiring proof; AdminUI `Validate()` range warnings. **S-8:** parser forces Write/AlarmAck retry→0 (spec invariant) **and** dispatch flips to `isIdempotent:false` (RED-first on the Runtime wiring test); non-idempotent arm caches the no-retry snapshot once/invoker (01/P-4) + tracker in-flight accounting; `WriteIdempotentAttribute` false claim corrected; retry cells disabled. **U-6:** bulkhead knob deleted end-to-end (migration-free; stored keys ignored) + `Options_properties_are_exactly_the_pipeline_wired_set` inertness guard. **C-7:** `ResilienceFormModel` JsonObject-bag round-trip preserves unknown keys; malformed JSON → `ParseFailed` lock + verbatim `ToJson` + discard button. **S-7:** per-`Create` `Interlocked` options-generation in the pipeline-cache key kills the respawn stale-pipeline race. Core resilience 121/121, AdminUI form 10/10, Analyzers 32/32, Runtime 364/364; 0 production OTOPCUA0001. **Deferred (T15/T18):** docker-dev `:9200` `/run` (warning banner, input lockout, unknown-key survival) + whole-solution sweep. | | **R2-10** | 03/U10=04/U-5 (Medium/Low, action #10) | `r2/10-resilience-observability` (off `46fedda3`) | `205d3980`…`` (11 task commits) | **Code-complete, offline-verified.** The resilience status tracker now has a production reader, mirroring the driver-health flow. Core truthfulness: `RecordBreakerClose` + derived `IsBreakerOpen` (breaker-open state derivable), `OnClosed` records the close, and `RecordSuccess` wired onto the invoker success paths (`ConsecutiveFailures` is a true consecutive counter, not stuck-forever). Transport: `DriverResilienceStatusChanged` Commons contract → periodic 5 s full-snapshot publisher `DriverResilienceStatusPublisherService` (driver nodes, registered in `AddOtOpcUaDriverFactories` with wiring guard `ResilienceStatusReaderRegistrationTests` — closes the built-but-never-wired class) → `driver-resilience-status` DPS topic → per-admin-node `DriverResilienceStatusBridge` → `IDriverResilienceStatusStore` → resilience chips (breaker-OPEN / consecutive-failures / in-flight / staleness) on `DriverStatusPanel.razor`, read in-process (self-HubConnection ban honored). Core.Tests resilience 31/31 (incl. deterministic breaker-close via a controllable TimeProvider), Host.IntegrationTests publisher+guard 6/6, AdminUI.Tests store 5/5; full-solution build clean. **Deferred (T10):** docker-dev live-`/run` — S7 dead-endpoint breaker-open observed on BOTH centrals + recovery-clear + staleness-dim (also the first live sighting of the #10 `circuit-breaker OPENED` log line, closing FOLLOWUP-10's behavioral gate). | +| **R2-08** | 03/S2 (High, action #8), 03/S3 (High) | `r2/08-ldap-historyread-async` (off `1a698cbb`) | `ce6d9e21`…`` (14 task commits) | **Code-complete, offline-verified.** **S2 (LDAP async/timeout):** the SDK raises the impersonation callback synchronously under a `SessionManager`-wide `m_eventLock` (verified vs. 1.5.378 — a hung bind serializes ALL activations), so the fix is entirely at the `LdapOpcUaUserAuthenticator` boundary: five new `Security:Ldap` keys (`ConnectionTimeoutMs`=10000 projected into the shared lib, `AuthTimeoutMs`=15000 hard whole-flow `WaitAsync` bound → fail-closed "timed out", `MaxConcurrentAuthentications`=8 `SemaphoreSlim` with release-on-core-completion so orphaned binds still count, `OutageFailureThreshold`=3/`OutageCooldownSeconds`=15 directory-outage circuit → fast-deny "unavailable", half-open probe) + `LdapAuthResult.IsSystemFailure` seam (mapped from `ServiceAccountBindFailed`) distinguishing outage from credential-deny + `LdapOptionsValidator` rules + `HandleImpersonation` contract doc + `docs/security.md`. Stall-repro RED→GREEN; offline outage-sim through the real `HandleImpersonation` against an unroutable host. **S3 (HistoryRead fan-out):** all four HistoryRead arms async-ified (`ServeNodeAsync`/`ServeRawPagedAsync`/`ServeEventsAsync`, OCE→`BadTimeout`) then bounded per-node fan-out (`RunBounded` semaphore + single arm-boundary bridge) + per-request deadline CTS (`HistoryReadDeadline`=60s) + process-wide lazy batch limiter (`MaxConcurrentHistoryReadBatches`=16 → flood degrades to `BadTooManyOperations`); 16×4=64 in-flight cap; three new `ServerHistorian` keys + node-manager props + hosted-service wiring + `docs/Historian.md`/CLAUDE.md tables. Sequential-serving RED→GREEN. Host.IntegrationTests (LDAP resilience) 13/13, Security.Tests 11/11, Runtime.Tests options 11/11, OpcUaServer.Tests HistoryRead 30/30. **Deferred (live):** S2 docker-dev unreachable-LDAP + GLAuth outage/recovery cycle; S3 `Category=LiveIntegration` gateway soak. | | **R2-11** | 01/C-1, 01/P-1 + 05/CONV-2, UNDER-1, UNDER-6 | `r2/11-tagconfig-consolidation` (off `46fedda3`) | first `2e14fe1f` … (24 task commits) | **Code-complete, offline-verified.** **Part 1 (C-1/P-1):** single `TagConfigIntent.Parse` + `DeviceConfigIntent` in Commons replace the four byte-parity TagConfig parse copies (composer/artifact/walker/DraftValidator) — composer now parses once per tag (P-1); a permanent `Runtime.Tests/TagConfigCorpusParityTests` (30-blob corpus) guards compose≡decode; `grep "MUST parse identically" src/` = 0. **Part 2 (CONV-2):** shared strict-capable `TagConfigJson` readers in Core.Abstractions replace the six copy-pasted `ReadEnum`s; all six driver parsers gain `Inspect()` warnings + honour the `writable` key (FOCAS **forced read-only**, 05/UNDER-1); FOCAS equipment tags run the capability-matrix pre-flight (05/UNDER-6); Modbus probe parses the factory DTO shape (`timeoutMs` no longer dropped). New `EquipmentTagConfigInspector` in ControlPlane + `AdminOperationsActor` deploy-gate wiring with `Deployment:TagConfigValidationMode` (**Warn default | Error opt-in**), proven through the actor (F10b consuming test). AdminUI: `writable` checkbox in the six typed editors. **Phase-C follow-up (deferred, recorded here):** flip the runtime parsers to strict (typo'd enum ⇒ `BadNodeIdUnknown` instead of wrong-width `Good`) once fleets have run `Error` mode clean — behavior change deliberately not shipped in this plan. **Deferred (T22/T24):** docker-dev `/run` of one typed editor's Writable checkbox + whole-solution serial sweep. | **Report anchor drifts to fold into the next report refresh** (from the R2-02 verification pass): (1) diff --git a/docs/Historian.md b/docs/Historian.md index 97a8abfd..21a88b84 100644 --- a/docs/Historian.md +++ b/docs/Historian.md @@ -69,7 +69,10 @@ and all HistoryRead calls on historized nodes return `GoodNoData` (empty, not an "AllowUntrustedServerCertificate": false, "CaCertificatePath": null, "CallTimeout": "00:00:30", - "MaxTieClusterOverfetch": 65536 + "MaxTieClusterOverfetch": 65536, + "HistoryReadBatchParallelism": 4, + "MaxConcurrentHistoryReadBatches": 16, + "HistoryReadDeadline": "00:01:00" } } ``` @@ -84,6 +87,9 @@ and all HistoryRead calls on historized nodes return `GoodNoData` (empty, not an | `CaCertificatePath` | string\|null | `null` | PEM CA file pinning the gateway's TLS chain. Null/empty uses the OS trust store. | | `CallTimeout` | TimeSpan | `00:00:30` | Per-call deadline applied to each unary gateway read. A read for a tag the historian does not yet know about — e.g. a freshly-historized tag that has **not** been provisioned (see [Tag auto-provisioning](#tag-auto-provisioning-ensuretags)) — can run to this **full** deadline before the gateway errors, so an unprovisioned-tag HistoryRead can block for ~30 s. Lower this to fail faster, at the cost of truncating legitimately long reads. | | `MaxTieClusterOverfetch` | int | `65536` | Maximum samples the server will fetch in one shot to page through a tie cluster (multiple samples sharing one `SourceTimestamp`). A cluster larger than this ceiling fails `BadHistoryOperationUnsupported`. Raise to handle abnormally large tie clusters; the default covers all normal-data cases. | +| `HistoryReadBatchParallelism` | int | `4` | Max historized nodes served **concurrently** within one HistoryRead batch (arch-review 03/S3). The SDK's `HistoryRead*` override surface is synchronous, so each arm bridges once at its boundary; this turns per-node serving from sequential (`N × CallTimeout` worst case) into `⌈N / P⌉ × CallTimeout`. Also caps the gateway-load multiplication a single client can command. `≤ 1` serves sequentially. | +| `MaxConcurrentHistoryReadBatches` | int | `16` | Process-wide limit on HistoryRead batches served **concurrently** (arch-review 03/S3). A flood of history clients degrades to `BadTooManyOperations` rather than exhausting the SDK request-thread pool. With `HistoryReadBatchParallelism` this caps total in-flight gateway reads at `MaxConcurrentHistoryReadBatches × HistoryReadBatchParallelism` (default 16 × 4 = 64). A batch that cannot get a slot within `min(HistoryReadDeadline, 5 s)` fails every handle `BadTooManyOperations`. | +| `HistoryReadDeadline` | TimeSpan | `00:01:00` | Per-request wall-clock deadline for a HistoryRead batch (arch-review 03/S3). A node still in flight at the deadline surfaces `BadTimeout`, so one request can never hold an SDK request thread longer than this regardless of node count. Non-positive = unbounded (a `Validate()` warning). | > **Do not commit `ApiKey` to `appsettings.json`.** Set it via the environment variable > `ServerHistorian__ApiKey`, a secrets store, or a deployment-time overlay. The checked-in default is