From 3e6268351791e75b91ff509e0d6d5c85d41296f1 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 10:41:37 -0400 Subject: [PATCH] chore(historian): review cleanups for the raw cap - GatewayHistorianDataSource.ReadRawWindowAsync keeps the cap + 1 probe budget (budgetCap) separate from the client's cap: a strict-ceiling or crowding refusal now reports Limit = the server cap that applied and advises setting NumValuesPerNode at or below it (it said "lower NumValuesPerNode" with Limit = cap + 1 to a client that sent 0). - Fuzz: the oracle-source over-cap assertion was vacuous (cap 0 never counts); it now checks no reverse window reached the forward-only shape. Class docs name the post-floor cap set (3 ... 1000) and distinguish NumValuesPerNode 1 / 2 from the smallest server cap. - HistoryPaging.IsFullPage doc says it judges the REQUEST cap (page + ties + lookahead), which is finite under any limit; AdvertiseMaxReturnDataValues swallows any exception (returns false for the caller to log), as its doc promised. Refs #736 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .../GatewayHistorianDataSource.cs | 30 +++++++++++-------- .../HistoryPaging.cs | 23 +++++++------- .../NodeManagerHistoryReadRawCapFuzzTests.cs | 8 ++--- 3 files changed, 35 insertions(+), 26 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayHistorianDataSource.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayHistorianDataSource.cs index af0c2195..e5c19a87 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayHistorianDataSource.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayHistorianDataSource.cs @@ -232,32 +232,38 @@ public sealed class GatewayHistorianDataSource : IHistorianDataSource, IAsyncDis // The gateway seam caps with an int; OPC UA hands us a uint, so clamp to int range. var cap = (int)Math.Min(maxValuesPerNode, int.MaxValue); var serverCap = _maxRawValuesPerRead; + // Under the server raw cap, a read asking for everything or for more than the cap runs as a capped read of + // cap + 1 (budgetCap); `cap` stays what the client asked for, and a refusal names the cap that applied (limit). var probe = serverCap > 0 && (cap == 0 || cap > serverCap); - if (probe) cap = (int)Math.Min(int.MaxValue, (long)serverCap + 1); - if (cap == 0) + var budgetCap = probe ? (int)Math.Min(int.MaxValue, (long)serverCap + 1) : cap; + var limit = probe ? serverCap : cap; + var advice = probe + ? $"set NumValuesPerNode at or below ServerHistorian:MaxRawValuesPerRead ({serverCap}) to page" + : cap == 0 ? "set NumValuesPerNode to page" : "lower NumValuesPerNode"; + if (budgetCap == 0) { var all = await PassAsync(int.MaxValue).ConfigureAwait(false); if (all.Result.Complete) return all.Result.Samples; // Only reachable on a strict gateway: the ceiling re-read delivered the whole ceiling without reaching // the window end — the window may hold more, and "all values" is never answered from a truncated read. - throw new HistoryReadLimitExceededException(cap, + throw new HistoryReadLimitExceededException(limit, $"The raw read of '{fullReference}' holds more than the HistorianGateway's row ceiling ({all.Budget}, its " + "Historian:ReadLimits:MaxValuesCeiling with AllowStreamingBeyondCeiling=false); set NumValuesPerNode to page, " + "or narrow the window.") { PreWindowRows = Crowding(all.Result.PreWindowRows) }; } - var asked = (int)Math.Min(int.MaxValue, (long)cap + RawAggregateReader.PreWindowAllowance); + var asked = (int)Math.Min(int.MaxValue, (long)budgetCap + RawAggregateReader.PreWindowAllowance); var pass = await PassAsync(asked).ConfigureAwait(false); if (pass.Result.Complete) return Bounded(pass.Result.Samples); // A strict gateway's ceiling already bounded that read: a larger budget would only be refused again. The // refusal carries the pre-window rows it saw: the server re-reads the page that many values smaller. if (pass.Budget < asked) - throw new HistoryReadLimitExceededException(cap, + throw new HistoryReadLimitExceededException(limit, $"The raw read of '{fullReference}' could not be read completely within the HistorianGateway's row ceiling " + - $"({pass.Budget}, its Historian:ReadLimits:MaxValuesCeiling with AllowStreamingBeyondCeiling=false); lower " + - "NumValuesPerNode or narrow the window.") + $"({pass.Budget}, its Historian:ReadLimits:MaxValuesCeiling with AllowStreamingBeyondCeiling=false); {advice}, " + + "or narrow the window.") { PreWindowRows = Crowding(pass.Result.PreWindowRows) }; // The margin crowded the window out: re-read with room for the pre-window rows. A read that was itself cut short saw @@ -269,14 +275,14 @@ public sealed class GatewayHistorianDataSource : IHistorianDataSource, IAsyncDis var room = Math.Min(Math.Max(1, preWindow), _maxPreWindowRows); for (var attempt = 0; attempt < MaxCrowdedReReads && preWindow <= _maxPreWindowRows; attempt++) { - var budget = (int)Math.Min(int.MaxValue, (long)cap + room + RawAggregateReader.PreWindowAllowance); + var budget = (int)Math.Min(int.MaxValue, (long)budgetCap + room + RawAggregateReader.PreWindowAllowance); // doubling can leap over the ceiling while the cluster still fits. Spend the remaining room once. if (_ceiling.Learned is var known and > 0 && budget > known) { - var available = known - cap - RawAggregateReader.PreWindowAllowance; + var available = known - budgetCap - RawAggregateReader.PreWindowAllowance; if (available <= 0) break; room = Math.Min(room, available); - budget = cap + room + RawAggregateReader.PreWindowAllowance; + budget = budgetCap + room + RawAggregateReader.PreWindowAllowance; } if (budget <= pass.Budget) break; // no more room, never retry the same full read pass = await PassAsync(budget).ConfigureAwait(false); @@ -287,7 +293,7 @@ public sealed class GatewayHistorianDataSource : IHistorianDataSource, IAsyncDis room = (int)Math.Min(_maxPreWindowRows, Math.Max(preWindow, 2L * room)); } - throw new HistoryReadLimitExceededException(cap, + throw new HistoryReadLimitExceededException(limit, $"The raw read of '{fullReference}' could not be read completely: more than {pass.Result.PreWindowRows} rows lie in " + $"the {RawAggregateReader.BoundaryMargin.TotalMilliseconds} ms before the window and exhausted the row budget.") { PreWindowRows = preWindow }; @@ -308,7 +314,7 @@ public sealed class GatewayHistorianDataSource : IHistorianDataSource, IAsyncDis // One drained pass; a strict gateway's refusal of the budget is retried once at the ceiling it names. Task<((List Samples, int PreWindowRows, bool Complete) Result, int Budget)> PassAsync(int budget) => _ceiling.ReadAsync(budget, - b => ReadRawPassAsync(fullReference, startUtc, endUtc, cap, b, cancellationToken), + b => ReadRawPassAsync(fullReference, startUtc, endUtc, budgetCap, b, cancellationToken), (r, _) => !r.Complete); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/HistoryPaging.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/HistoryPaging.cs index 3dc4ab79..bee16369 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/HistoryPaging.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/HistoryPaging.cs @@ -131,8 +131,9 @@ internal static class HistoryPaging /// "the maximum number of values that can be returned by the Server for each Node", 0 = no limit). The cap is /// the most values one HistoryRead-Raw page returns (ServerHistorian:MaxRawValuesPerRead), so a client can /// size its NumValuesPerNode and expect a continuation point past it. Idempotent; a re-sync with a - /// different cap overwrites. Never throws: a missing diagnostics manager or node leaves the SDK default (0) and - /// returns false — HistoryRead itself still enforces the cap. + /// different cap overwrites. Never throws: a missing diagnostics manager or node, or any SDK failure writing the + /// property, leaves the SDK default (0) and returns false for the caller to log — HistoryRead itself still + /// enforces the cap. /// /// The running server. /// The cap (<= 0 advertises 0 = no limit). @@ -151,20 +152,22 @@ internal static class HistoryPaging return true; } } - catch (ServiceResultException) + catch (Exception) { - return false; + return false; // advisory only: the caller logs; the cap is enforced by HistoryRead regardless } } /// - /// Decide whether a just-returned page is "full" and therefore MAY be followed by more data — - /// the signal to emit a continuation point. A page is full when the client asked for a finite - /// cap ( > 0) and the backend returned exactly that many - /// samples. A short page (or an unlimited 0 request) is terminal. + /// Decide whether a just-returned read is "full" and therefore MAY be followed by more data — + /// the signal to emit a continuation point. A read is full when it was issued with a finite request + /// cap ( > 0 — the RawPagePlan.RequestCap: page + boundary ties + + /// lookahead, which under a backend limit or the server raw cap is always finite, uncapped client reads included) + /// and the backend returned at least that many samples. A short read is terminal; so is a read issued with a + /// request cap of 0, which only happens with no limit at all (one shot of the whole window). /// - /// The number of samples the backend returned for this page. - /// The client's per-page cap; 0 means "no limit". + /// The number of samples the backend returned for this read. + /// The request cap the read was issued with; 0 means "no limit". /// true when a continuation point should be emitted; otherwise false. public static bool IsFullPage(int returnedCount, uint numValuesPerNode) => numValuesPerNode > 0 && returnedCount >= numValuesPerNode; diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadRawCapFuzzTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadRawCapFuzzTests.cs index eb64ef64..6a9e78cd 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadRawCapFuzzTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadRawCapFuzzTests.cs @@ -13,7 +13,7 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; /// bugs that the hand-written cases missed (silent truncation, an endless cursor, an empty page between every value), so /// the cap is proved the only way that has worked: random series (bursts, gaps, sub-millisecond stamps, tie clusters), /// random windows (empty, on a tie, one step past a tie, between samples, forward / reverse / end-only, with and without -/// bounds) and random caps — NumValuesPerNode 0 / 1 / 2 / small / huge, the server cap 2 … 1000 or off, the +/// bounds) and random caps — NumValuesPerNode 0 / 1 / 2 / small / huge, the server cap 3 … 1000 or off, the /// tie-cluster budget 3 / 8 / 65536 — paged through a REAL SDK node manager against a wire-faithful fake of each backend /// (SqlServer: tick precision, native reverse, TOP; QuestDB: µs truncation of bounds AND stamps, native reverse, /// LIMIT; Gateway: forward only, [start, end) after its own boundary-row filtering, walked back by the @@ -26,8 +26,8 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; /// must never see a request for 0 or more than the cap (the "bounded by the same cap" invariant); and a /// BadTooManyOperations is accepted only when a tie cluster (or, on the Gateway shape, one millisecond) is /// larger than the budget that the cap and MaxTieClusterOverfetch leave — after an exact prefix. The run is -/// non-vacuous by construction: it asserts that empty windows, mid-tie windows, reverse reads, caps of 1 and 2 and -/// every shape occurred, and that most iterations were exact. +/// non-vacuous by construction: it asserts that empty windows, mid-tie windows, reverse reads, NumValuesPerNode +/// of 1 and 2, the smallest server cap (3) and every shape occurred, and that most iterations were exact. /// /// /// Iterations: OTOPCUA_PAGING_FUZZ_ITERATIONS (default 10 000, the lane gate). Seeds are stable, so a failure @@ -106,7 +106,7 @@ public sealed class NodeManagerHistoryReadRawCapFuzzTests(BoundsServerFixture fx truthCp.ShouldBeNull($"oracle (no cap, N = 0) is one page: {tag}"); if (!bounds) Render(truth).ShouldBe(Render(Oracle(series, start, end, direction, resolution)), $"oracle vs series: {tag}"); - truthSource.OverCapRequests.ShouldBe(0, $"the uncapped oracle source must never be asked for more than it allows: {tag}"); + truthSource.BadRequests.ShouldBe(0, $"a reverse window reached a forward-only backend on the oracle read: {tag}"); var windowTies = LargestTieCluster(truth); if (truth.Count == 0) Count("empty");