From c5df43d594b9328f1d08040e97c7c41b69dc42da Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 9 Nov 2026 20:27:06 -0500 Subject: [PATCH] fix(historian): ContinuousHistorization CommitMode code default is PerEntry, matching appsettings/docker-dev/fallback (lmxopcua#776) The Lane H2 remediation made Periodic(100 ms) the options-class default, but appsettings.json, docker-dev and Program.cs's bad-value fallback all used PerEntry, so effective behaviour never changed and the docs contradicted the shipped config. The options default is now PerEntry; Periodic stays opt-in (<= CommitIntervalMs of crash loss in exchange for throughput). Program.cs now uses ContinuousHistorizationOptions.ResolveCommitMode() so the fallback shares the single DefaultCommitMode constant. docs/Historian.md updated. Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV --- docs/Historian.md | 2 +- src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs | 5 +-- .../ContinuousHistorizationOptions.cs | 31 ++++++++++++------- .../ContinuousHistorizationOptionsTests.cs | 13 +++++++- 4 files changed, 34 insertions(+), 17 deletions(-) diff --git a/docs/Historian.md b/docs/Historian.md index 4f04d5c6..4ed72f14 100644 --- a/docs/Historian.md +++ b/docs/Historian.md @@ -275,7 +275,7 @@ and is registered the same way for every backend. |---|---|---|---| | `Enabled` | bool | `false` | `true` **and** a configured `ServerHistorian` wires the FasterLog outbox + the selected backend's value writer and spawns the `ContinuousHistorizationRecorder`. `false` → no recorder; driver tag values are not historized. | | `OutboxPath` | string | `""` | **Directory** holding the FasterLog segment + commit files. Required when `Enabled`. In production set an **absolute** path on durable storage — a relative path resolves against the host's working directory. | -| `CommitMode` | string | `Periodic` | ⚠️ **Default changed from `PerEntry`.** `Periodic` commits on a background timer every `CommitIntervalMs` (a crash can lose at most the last interval of captured values); `PerEntry` fsyncs before the append returns (no loss window). Under `PerEntry` ingestion was capped at `1 / fsync_latency` appends/s. Parsed case-insensitively. ⚠️ The shipped `appsettings.json` / docker-dev still restate `PerEntry` (and the Host falls back to `PerEntry` for an unparseable value) until the Host-side follow-up lands — set `Periodic` explicitly to adopt it now. | +| `CommitMode` | string | `PerEntry` | `PerEntry` fsyncs before the append returns (no loss window; one group-commit fsync per ingest burst). `Periodic` is the opt-in throughput mode: it commits on a background timer every `CommitIntervalMs`, so a crash can lose at most the last interval of captured values in exchange for ingestion no longer being capped at `1 / fsync_latency`. Parsed case-insensitively; an unparseable value falls back to `PerEntry`. The code default, shipped `appsettings.json`, docker-dev and the fallback all agree on `PerEntry` (lmxopcua#776). | | `CommitIntervalMs` | int | `100` | Periodic-mode commit cadence (ms); must be `> 0` under `Periodic`, ignored under `PerEntry`. | | `DrainBatchSize` | int | `1024` | Outbox entries peeked + written per drain pass. **Raised from 64**: the peek is an in-memory scan of the outbox's live index (widening it costs allocation, not I/O), and a pass is now bounded-parallel over `DrainConcurrency` rather than sequential, so a wide batch no longer costs `tags × latency`. At 64 a drain drew down at most 64 entries every `DrainIntervalSeconds` — below the arrival rate of even a modest historized tag set — so any gateway hiccup left the outbox permanently behind, growing toward `Capacity` and shedding oldest samples instead of catching up. | | `DrainIntervalSeconds` | double | `2` | Steady drain cadence (and the post-success reschedule). | diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs index e277a94b..fc3cb39a 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs @@ -315,10 +315,7 @@ if (hasDriver) // outbox depth/dropped gauges here (once, on first resolution) keeps the live instance behind them. builder.Services.AddSingleton(_ => { - var commitMode = Enum.TryParse( - continuousHistorizationOptions.CommitMode, ignoreCase: true, out var parsedMode) - ? parsedMode - : HistorizationCommitMode.PerEntry; + var commitMode = continuousHistorizationOptions.ResolveCommitMode(); var outbox = new FasterLogHistorizationOutbox( continuousHistorizationOptions.OutboxPath, commitMode, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ContinuousHistorizationOptions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ContinuousHistorizationOptions.cs index 026fc424..a28401d8 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ContinuousHistorizationOptions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ContinuousHistorizationOptions.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.Runtime.Historian; @@ -38,20 +39,28 @@ public sealed class ContinuousHistorizationOptions public string OutboxPath { get; init; } = ""; /// - /// Outbox commit cadence: Periodic (the default) commits on a background timer every - /// ms (100 ms by default — a crash can lose at most the last interval of captured - /// values); PerEntry fsyncs before each append returns (no loss window, one fsync per append). Parsed - /// case-insensitively against HistorizationCommitMode. + /// Outbox commit cadence: PerEntry (the default, and what the shipped appsettings.json, docker-dev + /// and the unparseable-value fallback all use) fsyncs before each append returns — no loss window; Periodic + /// is the opt-in throughput mode that commits on a background timer every ms + /// (100 ms by default), so a crash can lose at most the last interval of captured values. Parsed + /// case-insensitively against HistorizationCommitMode; see . /// - /// Why Periodic. Under PerEntry every ingest append waits for an fsync, so ingestion is capped at - /// 1 / fsync_latency appends per second (50 drivers × 1 Hz on a 10 ms-fsync disk kept the recorder half - /// busy; at 100+ bursts/s it fell behind for good). The recorder now appends off its mailbox and coalesces - /// every burst waiting behind a slow append into one group commit, which helps PerEntry too — but it is - /// still one fsync per group on the critical path. PerEntry remains available where no loss window is - /// acceptable. + /// Trade-off. Under PerEntry every ingest append waits for an fsync, so ingestion is capped at + /// 1 / fsync_latency appends per second — but the recorder appends off its mailbox and coalesces + /// every burst waiting behind a slow append into one group commit, so it is one fsync per group, not per + /// value. Set Periodic explicitly where higher ingest throughput matters more than a bounded + /// (<= CommitIntervalMs) crash-loss window. /// /// - public string CommitMode { get; init; } = "Periodic"; + public string CommitMode { get; init; } = DefaultCommitMode.ToString(); + + /// The commit mode used when is not set or cannot be parsed. + public const HistorizationCommitMode DefaultCommitMode = HistorizationCommitMode.PerEntry; + + /// Parses case-insensitively; an unparseable value falls back to + /// so the fallback always agrees with the options default. + public HistorizationCommitMode ResolveCommitMode() => + Enum.TryParse(CommitMode, ignoreCase: true, out var mode) ? mode : DefaultCommitMode; /// Periodic-mode commit cadence in milliseconds; must be positive when /// is Periodic. Ignored under PerEntry. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationOptionsTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationOptionsTests.cs index 9e1bd4e4..5b4f62bb 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationOptionsTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationOptionsTests.cs @@ -1,4 +1,5 @@ using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Runtime.Historian; namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Historian; @@ -36,6 +37,16 @@ public sealed class ContinuousHistorizationOptionsTests }.Validate(), m => m.Contains("CommitIntervalMs")); + [Fact] + public void Unparseable_commit_mode_falls_back_to_the_options_default() + { + var defaults = new ContinuousHistorizationOptions(); + Assert.Equal(HistorizationCommitMode.PerEntry, defaults.ResolveCommitMode()); + Assert.Equal(ContinuousHistorizationOptions.DefaultCommitMode, new ContinuousHistorizationOptions { CommitMode = "garbage" }.ResolveCommitMode()); + Assert.Equal(Enum.Parse(defaults.CommitMode), new ContinuousHistorizationOptions { CommitMode = "garbage" }.ResolveCommitMode()); + Assert.Equal(HistorizationCommitMode.Periodic, new ContinuousHistorizationOptions { CommitMode = "periodic" }.ResolveCommitMode()); + } + /// /// Pins the shipped defaults, which are a tuned set rather than arbitrary numbers: the drain is /// bounded-parallel over an in-memory peek (so a wide DrainBatchSize is cheap and a narrow @@ -49,7 +60,7 @@ public sealed class ContinuousHistorizationOptionsTests { var options = new ContinuousHistorizationOptions(); - Assert.Equal("Periodic", options.CommitMode); // 100 ms group commits — PerEntry capped ingest at 1 / fsync latency + Assert.Equal("PerEntry", options.CommitMode); // matches appsettings.json, docker-dev and the bad-value fallback (lmxopcua#776) Assert.Equal(100, options.CommitIntervalMs); Assert.Equal(1024, options.DrainBatchSize); Assert.Equal(8, options.DrainConcurrency);