From ccda27d2207ee9a4170c1d45fc398d3f0d2fee19 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 12 Oct 2026 17:20:41 -0400 Subject: [PATCH] =?UTF-8?q?fix(historian):=20#691.4=20review=20=E2=80=94?= =?UTF-8?q?=20uncapped=20Gateway=20events=20reads=20no=20longer=20pre-size?= =?UTF-8?q?=20to=20int.MaxValue?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GatewayHistorianDataSource.ReadEventsAsync sized its result list to maxEvents, which is int.MaxValue for NumValuesPerNode = 0: every uncapped Gateway events read (forward on master, and the new walked reverse one) threw 'Array dimensions exceeded' and counted as a backend failure. Capacity is bounded at 1024. The events walk delegate moved to ReverseEventsReader so it is tested over the real Gateway source (forward uncapped, reverse uncapped, reverse capped). --- docs/Historian.md | 3 + .../GatewayHistorianDataSource.cs | 3 +- .../OtOpcUaNodeManager.cs | 19 +---- .../ReverseEventsReader.cs | 54 +++++++++++++ .../FakeHistorianGatewayClient.cs | 5 +- .../GatewayReverseEventsTests.cs | 76 +++++++++++++++++++ 6 files changed, 142 insertions(+), 18 deletions(-) create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/ReverseEventsReader.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/GatewayReverseEventsTests.cs diff --git a/docs/Historian.md b/docs/Historian.md index c14a98ff..fca9ccf6 100644 --- a/docs/Historian.md +++ b/docs/Historian.md @@ -787,6 +787,9 @@ says is complete carries no continuation point. `NumValuesPerNode == 0` stays un oversized-cluster over-fetch keeps exactly the events at `T` (the gateway's end is inclusive) in the same order. - **"More exists"** is found by walking for one event past the page (the walk has no source truncation flag to hand on). - **Uncapped** reverse reads are one forward read of the window, sorted and reversed. + - ⚠️ **Also fixed (#691.4 review): uncapped events reads on Gateway.** `NumValuesPerNode = 0` reaches the source as + `int.MaxValue`, and `GatewayHistorianDataSource` pre-sized its result list to it ("Array dimensions exceeded") — every + uncapped Gateway events read, forward too, failed the node and counted as a backend failure. The capacity is now bounded. - A timestamp holding more than `MaxTieClusterOverfetch` events ⇒ **`BadTooManyOperations`** for that node. - ⚠️ **Gateway prerequisite:** the gateway's `RuntimeDb:EventReadMaxRows` must be `0` (its default, unlimited) or larger than the walk's biggest read (`2N + 18` rows, a discovery read 1,026, a tie-cluster read up to `2·MaxTieClusterOverfetch + 4`). 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 b25790be..a5041334 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 @@ -425,7 +425,8 @@ public sealed class GatewayHistorianDataSource : IHistorianDataSource, IAsyncDis try { var hasCap = maxEvents > 0; - var collected = new List(hasCap ? maxEvents : 0); + // Bounded initial capacity: an uncapped read arrives as int.MaxValue ("Array dimensions exceeded", #691.4 review). + var collected = new List(hasCap ? Math.Min(maxEvents, 1024) : 0); var truncated = false; await foreach (var wireEvent in _client diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 57ab3fc8..8488e171 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -5311,22 +5311,9 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // Walk for one more than the fetch: finding it is the "more events exist" signal a source gives with its // continuation point (the walk itself cannot flag truncation). var want = numValuesPerNode == 0 ? 0u : (uint)Math.Min(uint.MaxValue - 1L, (long)fetch + 1); - var budget = Math.Max(1, MaxTieClusterOverfetch); - var walkSource = sourceName; - var (rows, next) = await ReverseWalk.WalkAsync( - async (s, e, cap, token) => - { - var r = await source.ReadEventsAsync(walkSource, s, e, cap == 0 ? int.MaxValue : cap, reverse: false, token) - .ConfigureAwait(false); - return new ForwardSegmentRead(r.Events, r.ContinuationPoint is not null); - }, - ev => ev.EventTimeUtc.Ticks, EventIdTieOrder.Instance, startUtc, endUtc, want, budget, - source.TimestampResolutionTicks, Clock.GetUtcNow().UtcDateTime, walkHint, - at => new HistoryReadLimitExceededException(budget, - $"A reverse events read of '{walkSource}' at {at:O} found more than {budget} events at one " + - "timestamp; the backend reads forward only, so the latest of them cannot be selected. Increase " + - "ServerHistorian:MaxTieClusterOverfetch."), - ct) + var (rows, next) = await ReverseEventsReader.ReadAsync( + source, sourceName, startUtc, endUtc, want, MaxTieClusterOverfetch, Clock.GetUtcNow().UtcDateTime, + walkHint, ct) .ConfigureAwait(false); walkHint = next; var more = want > 0 && rows.Count > fetch; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/ReverseEventsReader.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/ReverseEventsReader.cs new file mode 100644 index 00000000..52ab3bfd --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/ReverseEventsReader.cs @@ -0,0 +1,54 @@ +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer; + +/// One walked-back reverse events page (). +/// At most the requested count, newest first by (EventTimeUtc, EventId). +/// What the next page of the same read resumes with. +internal readonly record struct ReverseEventsPage(List Events, ReverseWalkHint Hint); + +/// +/// Gitea #691.4: serves a Part 11 reverse events read — (lo, hi] newest-first — on a backend that reads events +/// forward only ( = false: Gateway, Null) with the +/// . A segment read is cut short when the source flags truncation (its continuation point); ties +/// are ordered by because the gateway orders by event time only. +/// +internal static class ReverseEventsReader +{ + /// Reads (lo, hi] newest-first from forward event reads. + /// A backend that reads events forward. + /// Event-source filter (null = all). + /// The later (inclusive) bound. + /// The earlier (exclusive) bound. + /// How many events; 0 = all in the window (one forward read). + /// The tie budget (MaxTieClusterOverfetch) — a ceiling only. + /// Now, for the look-ahead clamp. + /// What the previous page carried (default = none). + /// Cancellation. + /// The events, newest first, and the hint for the next page. + /// One timestamp holds more than the tie budget. + public static async Task ReadAsync( + IHistorianDataSource source, string? sourceName, DateTime hi, DateTime lo, uint want, int maxTieCluster, + DateTime nowUtc, ReverseWalkHint hint, CancellationToken ct) + { + var budget = Math.Max(1, maxTieCluster); + var (rows, next) = await ReverseWalk.WalkAsync( + async (start, end, cap, token) => + { + // cap 0 = the whole window: int.MaxValue is the events contract's "no cap" (a non-positive value would + // mean the backend's DEFAULT cap). + var r = await source.ReadEventsAsync(sourceName, start, end, cap == 0 ? int.MaxValue : cap, reverse: false, token) + .ConfigureAwait(false); + return new ForwardSegmentRead(r.Events, r.ContinuationPoint is not null); + }, + ev => ev.EventTimeUtc.Ticks, EventIdTieOrder.Instance, hi, lo, want, budget, source.TimestampResolutionTicks, + nowUtc, hint, + at => new HistoryReadLimitExceededException(budget, + $"A reverse events read of '{sourceName}' at {at:O} found more than {budget} events at one timestamp; the " + + "backend reads forward only, so the latest of them cannot be selected. Increase " + + "ServerHistorian:MaxTieClusterOverfetch."), + ct) + .ConfigureAwait(false); + return new ReverseEventsPage(rows, next); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/FakeHistorianGatewayClient.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/FakeHistorianGatewayClient.cs index 6cb69cc8..5ee232b0 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/FakeHistorianGatewayClient.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/FakeHistorianGatewayClient.cs @@ -54,6 +54,9 @@ public sealed class FakeHistorianGatewayClient : IHistorianGatewayClient public int LastReadEventsMaxEvents; public int ReadEventsCallCount; + /// When set, ReadEvents serves EventsSource(start, end) instead of . + public Func>? EventsSource; + /// Thrown (deferred to first enumeration) by every read method when set. public Exception? ThrowOnRead; @@ -187,7 +190,7 @@ public sealed class FakeHistorianGatewayClient : IHistorianGatewayClient LastReadEventsEndUtc = endUtc; LastReadEventsMaxEvents = maxEvents; ReadEventsCallCount++; - return ToAsyncStream(Events, ThrowOnRead, ct); + return ToAsyncStream(EventsSource is null ? Events : EventsSource(startUtc, endUtc).ToList(), ThrowOnRead, ct); } public Task WriteLiveValuesAsync( diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/GatewayReverseEventsTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/GatewayReverseEventsTests.cs new file mode 100644 index 00000000..7fecc48c --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/GatewayReverseEventsTests.cs @@ -0,0 +1,76 @@ +using Google.Protobuf.WellKnownTypes; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; +using Xunit; +using ZB.MOM.WW.HistorianGateway.Contracts.Grpc; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.OpcUaServer; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests; + +/// +/// Gitea #691.4 review: events read uncapped (and walked back newest-first) through the REAL +/// over a gateway-shaped fake — inclusive [start, end], ordered by event time. +/// +public sealed class GatewayReverseEventsTests +{ + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + private static readonly DateTime T0 = new(2026, 10, 12, 18, 0, 0, DateTimeKind.Utc); + + private static FakeHistorianGatewayClient Store(int count) + { + var stored = Enumerable.Range(0, count) + .Select(i => new HistorianEvent { Id = $"e{i:0000}", Type = "Alarm", EventTime = Timestamp.FromDateTime(T0.AddSeconds(i)) }) + .ToList(); + return new FakeHistorianGatewayClient + { + EventsSource = (start, end) => stored.Where(e => e.EventTime.ToDateTime() >= start && e.EventTime.ToDateTime() <= end), + }; + } + + /// + /// NumValuesPerNode = 0 reaches the source as int.MaxValue; the result list used to be pre-sized to it + /// ("Array dimensions exceeded") — every uncapped Gateway events read failed and counted as a backend failure. + /// + [Fact] + public async Task An_uncapped_forward_events_read_is_served() + { + var fake = Store(30); + var ds = new GatewayHistorianDataSource(fake, NullLogger.Instance); + + var r = await ds.ReadEventsAsync(null, T0, T0.AddMinutes(1), int.MaxValue, Ct); + + r.Events.Count.ShouldBe(30); + r.ContinuationPoint.ShouldBeNull(); + ds.GetHealthSnapshot().TotalFailures.ShouldBe(0); + } + + [Fact] + public async Task An_uncapped_reverse_events_read_is_one_forward_read_newest_first() + { + var fake = Store(30); + var ds = new GatewayHistorianDataSource(fake, NullLogger.Instance); + + var page = await ReverseEventsReader.ReadAsync(ds, null, hi: T0.AddSeconds(25), lo: T0.AddSeconds(5), want: 0, + maxTieCluster: 65536, nowUtc: T0.AddHours(1), hint: default, Ct); + + page.Events.Select(e => e.EventId).ShouldBe(Enumerable.Range(6, 20).Reverse().Select(i => $"e{i:0000}")); + fake.ReadEventsCallCount.ShouldBe(1); + ds.GetHealthSnapshot().TotalFailures.ShouldBe(0); + } + + [Fact] + public async Task A_capped_reverse_events_read_walked_back_over_the_gateway_returns_the_latest_newest_first() + { + var fake = Store(300); + var ds = new GatewayHistorianDataSource(fake, NullLogger.Instance); + + var page = await ReverseEventsReader.ReadAsync(ds, null, hi: T0.AddSeconds(200), lo: T0.AddSeconds(50), want: 10, + maxTieCluster: 65536, nowUtc: T0.AddHours(1), hint: default, Ct); + + page.Events.Select(e => e.EventId).ShouldBe(Enumerable.Range(191, 10).Reverse().Select(i => $"e{i:0000}")); + fake.LastReadEventsStartUtc.ShouldBeLessThanOrEqualTo(fake.LastReadEventsEndUtc); + ds.GetHealthSnapshot().TotalFailures.ShouldBe(0); + } +}