diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/HistoryBounds.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/HistoryBounds.cs index 2dbf03a7..25bf3a16 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/HistoryBounds.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/HistoryBounds.cs @@ -17,8 +17,13 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer; /// The end time was not given (a one-sided read's sentinel): there is no end bound to look up. /// The data is drained and the end bound did not fit on the previous page. /// The last value's time returned on an earlier page (stamps an open bound's placeholder). +/// +/// Gitea #692: the end bound an earlier page already looked up (a stored sample — never a placeholder), so the page that +/// returns it does not read the backend again. null = not looked up yet. +/// internal sealed record HistoryEndBound( - DateTime AtUtc, bool Reverse, bool Inclusive, bool Open, bool DataExhausted = false, DateTime? LastReturnedUtc = null); + DateTime AtUtc, bool Reverse, bool Inclusive, bool Open, bool DataExhausted = false, DateTime? LastReturnedUtc = null, + DataValueSnapshot? Resolved = null); /// /// Gitea #691: OPC UA Part 11 bounding values for HistoryRead-Raw (ReadRawModifiedDetails.ReturnBounds), composed @@ -59,6 +64,15 @@ internal static class HistoryBounds { private static readonly TimeSpan OpenBoundStep = TimeSpan.FromSeconds(1); + /// + /// Gitea #692: the placeholders this server synthesised (), by reference. A placeholder is known by + /// this marker, never by its shape: a stored row can legitimately carry Bad_BoundNotFound with a null value (a + /// client may write one through HistoryUpdate), and that row is data. + /// + private static readonly System.Runtime.CompilerServices.ConditionalWeakTable Placeholders = new(); + + private static readonly object PlaceholderMark = new(); + /// The end bound owed by a fresh read over the normalised window. /// The (normalised) start: the later time of a reverse read. /// The (normalised) end; a one-sided read's sentinel (tick 0 / MaxValue) is an open bound. @@ -66,11 +80,13 @@ internal static class HistoryBounds public static HistoryEndBound For(DateTime startUtc, DateTime endUtc) => new(endUtc, Reverse: startUtc > endUtc, Inclusive: startUtc != endUtc, Open: IsOpen(endUtc)); - /// Whether a returned value is a placeholder rather than a stored sample. + /// + /// Whether a returned value is a placeholder this server synthesised rather than a stored sample + /// — by an explicit marker (#692), so a stored row whose status happens to be Bad_BoundNotFound is not taken for one. + /// /// The value. - /// true for a Bad_BoundNotFound placeholder. - public static bool IsPlaceholder(DataValueSnapshot value) => - value.StatusCode == StatusCodes.BadBoundNotFound && value.Value is null; + /// true only for an instance produced. + public static bool IsPlaceholder(DataValueSnapshot value) => Placeholders.TryGetValue(value, out _); /// The pending end bound to store behind a continuation point, remembering the page's last returned time. /// The pending end bound (null = bounds not requested). @@ -84,8 +100,12 @@ internal static class HistoryBounds /// The Bad_BoundNotFound placeholder (null value) stamped at . /// The bound's time. /// The placeholder. - public static DataValueSnapshot NotFound(DateTime atUtc) => - new(null, StatusCodes.BadBoundNotFound, atUtc, atUtc); + public static DataValueSnapshot NotFound(DateTime atUtc) + { + var placeholder = new DataValueSnapshot(null, StatusCodes.BadBoundNotFound, atUtc, atUtc); + Placeholders.AddOrUpdate(placeholder, PlaceholderMark); + return placeholder; + } /// /// The start bound to return ahead of the data, or null when a stored sample sits on StartTime (that sample is @@ -123,6 +143,8 @@ internal static class HistoryBounds return NotFound(new DateTime(ticks, DateTimeKind.Utc)); } + if (bound.Resolved is { } resolved) return resolved; // #692: an earlier page already looked it up + DataValueSnapshot? found; if (bound.Reverse) found = await LatestAtOrBeforeAsync(source, tagname, bound.AtUtc, maxTieCluster, nowUtc, ct).ConfigureAwait(false); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/HistoryPaging.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/HistoryPaging.cs index b618dbf9..28082027 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/HistoryPaging.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/HistoryPaging.cs @@ -66,6 +66,11 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer; /// with ReturnBounds = true only (Gitea #691): the end bound still owed — /// Part 11 returns it with the last page. null = bounds not requested. /// +/// +/// only (Gitea #692): the previous page was a slice of an oversized tie cluster at +/// that still has un-emitted ties — the next page over-fetches the cluster directly +/// instead of first re-reading N + skip + 1 rows that are known to be ties. +/// internal sealed record HistoryContinuationState( string Tagname, DateTime NextStartUtc, @@ -79,7 +84,8 @@ internal sealed record HistoryContinuationState( long SegmentWidthTicks = 0, long EmptyFromTicks = 0, long EmptyToTicks = 0, - HistoryEndBound? PendingEndBound = null) + HistoryEndBound? PendingEndBound = null, + bool WithinTieCluster = false) { /// The walk-back hint this cursor carries (#686/#689/#691). public ReverseWalkHint WalkHint => new(SegmentWidthTicks, EmptyFromTicks, EmptyToTicks); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index eb0c81d8..735effdf 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -533,7 +533,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// /// The upper bound on the bounded over-fetch uses to page WITHIN an /// oversized "tie cluster" — more raw samples sharing one SourceTimestamp than the client's per-page - /// cap. When a resume read stalls on such a cluster (the boundary-tie trim empties the page), the + /// cap. When a resume read stalls on such a cluster (every row of its N + skip + 1 read, past the + /// trimmed ties, still sits on the boundary timestamp — #692: only a genuine cluster larger than the page), the /// paging over-fetches up to MaxTieClusterOverfetch + 1 ties at that single timestamp (a /// start == end read) and slices through them via . /// A cluster strictly larger than this still fails that node BadTooManyOperations (the absurd-burst @@ -6190,11 +6191,16 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// /// Fresh read (no inbound continuation point): read the window from /// details.StartTime to capped at - /// . If the page comes back FULL (exactly the cap, and the - /// cap is > 0), store a resume cursor and emit a continuation point. + /// + 1 — one lookahead row (#692). If the lookahead row came + /// back, return the first N, store a resume cursor and emit a continuation point; otherwise the page is + /// the last and carries none (so a total that is an exact multiple of N ends on a full page, not on an + /// empty trailing one). /// Resume read (inbound continuation point present): take the stored cursor, read - /// the next page from the boundary forward, trim already-emitted boundary ties, and emit a - /// FRESH continuation point only if THIS page is also full — else null (done). + /// N + boundarySkip + 1 rows from the boundary (the ties about to be trimmed, the page, and the + /// lookahead row), trim the already-emitted boundary ties, and continue exactly as a fresh read — so every + /// page carries N values. (Asking for only N used to net N − skip per resumed page, and at N = 1 an EMPTY + /// page between every value: the trimmed page looked like a stalled tie cluster and paid a + /// + 1 over-fetch to step one tick — Gitea #692.) /// /// The resume cursor is tie-safe (see / /// ): the next page resumes from the boundary @@ -6238,6 +6244,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 var boundarySkip = 0; var walkHint = default(ReverseWalkHint); // #686/#689/#691: the walk-back's width + proven-empty stretch, page to page HistoryEndBound? endBound = null; // #691: the end bound still owed (null = bounds not requested) + var withinTieCluster = false; // #692: the previous page sliced an oversized tie cluster that still has ties string tagname; if (inboundCp is { Length: > 0 }) @@ -6260,6 +6267,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 numValuesPerNode = state.NumValuesPerNode; walkHint = state.WalkHint; endBound = state.PendingEndBound; + withinTieCluster = state.WithinTieCluster; } else { @@ -6314,18 +6322,29 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 } // #691 review: whether a bounded read whose start bound was NOT found has any data or a found end bound — only - // asked when a one-value page would otherwise be the start-bound placeholder alone. - async Task BoundedReadHasAnythingAsync() + // asked when a one-value page would otherwise be the start-bound placeholder alone. #692: what it learns rides the + // cursor — the walk hint its walk-back computed, and, when there is no data, the end bound it found (so the next + // page is that bound with no backend read at all). + async Task<(bool Any, HistoryEndBound Pending)> BoundedReadHasAnythingAsync() { var src = HistorianDataSource; - IReadOnlyList first = startUtc > endUtc && !src.SupportsReverseRaw - ? (await ReverseRawReader.ReadAsync(src, tagname, startUtc, endUtc, 1, MaxTieClusterOverfetch, nowUtc, 0, ct) - .ConfigureAwait(false)).Samples - : (await src.ReadRawAsync(tagname, startUtc, endUtc, 1, ct).ConfigureAwait(false)).Samples; - if (first.Any(x => HistoryBounds.InDataWindow(x, endBound!, resolution))) return true; + IReadOnlyList first; + if (startUtc > endUtc && !src.SupportsReverseRaw) + { + var walked = await ReverseRawReader.ReadAsync( + src, tagname, startUtc, endUtc, 1, MaxTieClusterOverfetch, nowUtc, walkHint, ct) + .ConfigureAwait(false); + walkHint = walked.Hint; + first = walked.Samples; + } + else first = (await src.ReadRawAsync(tagname, startUtc, endUtc, 1, ct).ConfigureAwait(false)).Samples; + + if (first.Any(x => HistoryBounds.InDataWindow(x, endBound!, resolution))) return (true, endBound!); var end = await HistoryBounds.EndBoundAsync( src, tagname, endBound!, startUtc, MaxTieClusterOverfetch, nowUtc, ct).ConfigureAwait(false); - return !HistoryBounds.IsPlaceholder(end); + return HistoryBounds.IsPlaceholder(end) + ? (false, endBound!) + : (true, endBound! with { DataExhausted = true, Resolved = end }); } // #691: a resumed bounded read whose data is drained (the end bound did not fit on the previous page, or the @@ -6354,14 +6373,18 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 startBound = await HistoryBounds.StartBoundAsync( HistorianDataSource, tagname, startUtc, endBound.Reverse, MaxTieClusterOverfetch, nowUtc, ct) .ConfigureAwait(false); - if (startBound is not null && numValuesPerNode == 1 - && (!HistoryBounds.IsPlaceholder(startBound) || await BoundedReadHasAnythingAsync().ConfigureAwait(false))) + var (anything, pending) = startBound is not null && numValuesPerNode == 1 + ? HistoryBounds.IsPlaceholder(startBound) + ? await BoundedReadHasAnythingAsync().ConfigureAwait(false) + : (true, endBound) + : (false, endBound); + if (startBound is not null && numValuesPerNode == 1 && anything) { // The page is the start bound alone; the data starts at StartTime on the next page. var boundOnlyCp = _historyContinuationStore.Save(session, new HistoryContinuationState( tagname, startUtc, endUtc, 0, numValuesPerNode, SegmentWidthTicks: walkHint.WidthTicks, EmptyFromTicks: walkHint.EmptyFromTicks, EmptyToTicks: walkHint.EmptyToTicks, - PendingEndBound: HistoryBounds.Carry(endBound, [startBound]))); + PendingEndBound: HistoryBounds.Carry(pending, [startBound]))); results[handle.Index] = new SdkHistoryReadResult { StatusCode = StatusCodes.Good, @@ -6388,60 +6411,78 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 if (startBound is not null && numValuesPerNode > 0) readCap = numValuesPerNode - 1; } + // #692: the page's read asks for the boundary ties it is about to trim (a resumed page re-reads them, because the + // cursor resumes AT the boundary timestamp, inclusive) plus ONE lookahead row past the page. Without them a resumed + // page netted N − skip values, and at N = 1 nothing at all: the lone boundary sample came back, was trimmed, the + // "full" empty page was taken for a stalled tie cluster, and a MaxTieClusterOverfetch + 1 over-fetch stepped one + // tick to return an EMPTY page with a continuation point — between every value. The lookahead row is what says + // "more data follows": a page whose read found no row past it is the last, so the read no longer ends with an + // empty trailing page either. + var resumed = inboundCp is { Length: > 0 }; + var requestCap = readCap == 0 + ? 0u + : (uint)Math.Min(uint.MaxValue, (long)readCap + (resumed ? Math.Max(0, boundarySkip) : 0) + 1L); + // HistoryRead is NOT under the node-manager Lock — awaiting the async source is safe. // A reverse window (explicit, resumed, or the end-only read above) goes to the backend only when it declares it // can read newest-first; otherwise it is walked back from bounded forward reads (#686 — the Gateway rejects // start > end, which failed the node and counted as a backend fault). + // A cursor left inside an oversized tie cluster (#692) skips this read: every row it could return is a tie. var source = HistorianDataSource; - HistorianRead sourceResult; - if (startUtc > endUtc && !source.SupportsReverseRaw) + IReadOnlyList samples = []; + var backendFull = false; + if (!withinTieCluster) { - var page = await ReverseRawReader.ReadAsync( - source, tagname, startUtc, endUtc, readCap, MaxTieClusterOverfetch, - nowUtc, walkHint, ct) - .ConfigureAwait(false); - walkHint = page.Hint; - sourceResult = new HistorianRead(page.Samples, ContinuationPoint: null); - } - else - { - sourceResult = await source - .ReadRawAsync(tagname, startUtc, endUtc, readCap, ct) - .ConfigureAwait(false); - } - - var backendFull = HistoryPaging.IsFullPage(sourceResult.Samples.Count, readCap); - - // On a resume read, drop the boundary ties already returned on the prior page. - var samples = inboundCp is { Length: > 0 } - ? HistoryPaging.TrimBoundaryDuplicates(sourceResult.Samples, startUtc, boundarySkip) - : sourceResult.Samples; - - // #691: a bounded read's data window excludes the end time (a sample there is the end bound). A backend whose - // forward end is inclusive (the gateway's own) returns it as data; dropping it also means the data is drained. - if (endBound is not null) - { - var inside = samples.Where(x => HistoryBounds.InDataWindow(x, endBound, resolution)).ToList(); - if (inside.Count != samples.Count) + HistorianRead sourceResult; + if (startUtc > endUtc && !source.SupportsReverseRaw) { - samples = inside; - backendFull = false; + var page = await ReverseRawReader.ReadAsync( + source, tagname, startUtc, endUtc, requestCap, MaxTieClusterOverfetch, + nowUtc, walkHint, ct) + .ConfigureAwait(false); + walkHint = page.Hint; + sourceResult = new HistorianRead(page.Samples, ContinuationPoint: null); + } + else + { + sourceResult = await source + .ReadRawAsync(tagname, startUtc, endUtc, requestCap, ct) + .ConfigureAwait(false); + } + + backendFull = HistoryPaging.IsFullPage(sourceResult.Samples.Count, requestCap); + + // On a resume read, drop the boundary ties already returned on the prior page. + samples = resumed + ? HistoryPaging.TrimBoundaryDuplicates(sourceResult.Samples, startUtc, boundarySkip) + : sourceResult.Samples; + + // #691: a bounded read's data window excludes the end time (a sample there is the end bound). A backend whose + // forward end is inclusive (the gateway's own) returns it as data; dropping it also means the data is drained. + if (endBound is not null) + { + var inside = samples.Where(x => HistoryBounds.InDataWindow(x, endBound, resolution)).ToList(); + if (inside.Count != samples.Count) + { + samples = inside; + backendFull = false; + } } } - // Oversized tie cluster: a resume read whose FULL backend page does not advance past the resume - // boundary timestamp `startUtc`. That happens when more samples share `startUtc` than the page - // cap — a tie cluster larger than NumValuesPerNode. The fixed-(start,end,cap) backend can only - // ever return the first `cap` of those ties, so the (timestamp, skip) cursor alone can never page - // past (or even reliably within) the cluster: either the boundary-tie trim empties the page, or - // it re-emits the same head ties forever. Detect BOTH stall shapes — an empty trimmed page, or a - // non-empty page whose LAST sample is still at `startUtc` (no forward progress) — and, rather - // than fail the read, over-fetch the WHOLE cluster with an explicit, BOUNDED cap (a start==end - // read at the boundary timestamp) and page WITHIN the timestamp via SliceTieCluster. - var stalledOnTieCluster = inboundCp is { Length: > 0 } - && backendFull - && (samples.Count == 0 - || (samples[^1].SourceTimestampUtc ?? DateTime.MinValue) == startUtc); + // Oversized tie cluster: a resume read whose FULL backend read does not advance past the resume boundary + // timestamp `startUtc` — every row it returned past the trimmed ties, the lookahead row included, still sits on + // `startUtc`. That happens only when more samples share `startUtc` than the page plus the already-emitted ties + // plus one: a genuine tie cluster larger than the page. The fixed-(start,end,cap) backend can only ever return + // the first `cap` of those ties, so growing the read page by page would re-read the cluster quadratically. + // Instead over-fetch the WHOLE cluster with an explicit, BOUNDED cap (a start==end read at the boundary + // timestamp) and page WITHIN the timestamp via SliceTieCluster. (#692: before the read asked for the trimmed ties + // and a lookahead row, a resumed N = 1 page tripped this on every single sample.) + var stalledOnTieCluster = resumed + && (withinTieCluster + || (backendFull + && (samples.Count == 0 + || (samples[^1].SourceTimestampUtc ?? DateTime.MinValue) == startUtc))); if (stalledOnTieCluster) { // The over-fetch cap MUST be explicit and non-zero: a cap of 0 falls back to the backend's @@ -6493,7 +6534,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 var clusterState = new HistoryContinuationState( tagname, resumeAt, endUtc, nextSkip, numValuesPerNode, SegmentWidthTicks: walkHint.WidthTicks, EmptyFromTicks: walkHint.EmptyFromTicks, EmptyToTicks: walkHint.EmptyToTicks, - PendingEndBound: HistoryBounds.Carry(endBound, slice)); + PendingEndBound: HistoryBounds.Carry(endBound, slice), + WithinTieCluster: resumeAt == startUtc); clusterCp = _historyContinuationStore.Save(session, clusterState); } else if (endBound is not null) @@ -6512,6 +6554,24 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 return; } + // #692: more than the page came back (the lookahead row, after trimming) ⇒ there is more data: return the page and + // resume after it. Otherwise the data is drained and this is the last data page. + var more = readCap > 0 && samples.Count > readCap; + if (more) samples = samples.Take((int)readCap).ToList(); + + HistoryContinuationState? nextState = null; + if (more) + { + // A page that never left the boundary timestamp (ties continuing a resumed page) adds to the ties already + // emitted there, or the next page would re-emit the earlier ones. + HistoryPaging.ComputeResumeCursor(samples, out var nextStart, out var skip); + if (resumed && nextStart == startUtc) skip += boundarySkip; + nextState = new HistoryContinuationState( + tagname, nextStart, endUtc, skip, numValuesPerNode, SegmentWidthTicks: walkHint.WidthTicks, + EmptyFromTicks: walkHint.EmptyFromTicks, EmptyToTicks: walkHint.EmptyToTicks, + PendingEndBound: HistoryBounds.Carry(endBound, samples)); + } + // #691: the start bound (first page only) leads the data. if (startBound is not null) { @@ -6520,18 +6580,11 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 samples = led; } - // The "full page" test is against the RAW backend count (before trimming): the backend honoured - // the cap, so a full backend page ⇒ there may be more even if we trimmed some boundary ties. var historyData = ToHistoryDataFromSamples(samples); byte[]? outboundCp = null; - if (backendFull && samples.Count > 0) + if (nextState is not null) { - HistoryPaging.ComputeResumeCursor(samples, out var nextStart, out var skip); - var nextState = new HistoryContinuationState( - tagname, nextStart, endUtc, skip, numValuesPerNode, SegmentWidthTicks: walkHint.WidthTicks, - EmptyFromTicks: walkHint.EmptyFromTicks, EmptyToTicks: walkHint.EmptyToTicks, - PendingEndBound: HistoryBounds.Carry(endBound, samples)); // Save may return null (no session on this request) ⇒ degrade to single-shot for this node. // Built AFTER historyData so a failure projecting samples can never orphan a stored cursor. outboundCp = _historyContinuationStore.Save(session, nextState); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadPageFillTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadPageFillTests.cs new file mode 100644 index 00000000..36c7cf3e --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadPageFillTests.cs @@ -0,0 +1,376 @@ +using Opc.Ua; +using Opc.Ua.Server; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using HistorianRead = ZB.MOM.WW.OtOpcUa.Core.Abstractions.HistoryReadResult; +using SdkHistoryReadResult = Opc.Ua.HistoryReadResult; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; + +/// +/// Gitea #692: a paged HistoryRead-Raw fills every page. With NumValuesPerNode = 1 a resumed page used to re-read +/// only the boundary sample, trim it, take the empty page for a stalled tie cluster and over-fetch +/// MaxTieClusterOverfetch + 1 rows to step one tick — an EMPTY page (GoodNoData + continuation point) between every +/// value (8 values = 17 pages / 25 backend reads), on every backend, forward and reverse, with or without ReturnBounds. +/// Larger pages lost one value per resumed page the same way. Every page must now carry exactly N values (bounds count), +/// the over-fetch must fire only for a genuine tie cluster, and the read must not end with an empty trailing page when the +/// last data page already knew it was the last. +/// +public sealed class NodeManagerHistoryReadPageFillTests(BoundsServerFixture fx) : IClassFixture +{ + private static readonly DateTime T0 = new(2026, 10, 12, 19, 0, 0, DateTimeKind.Utc); + + public enum Shape { Native, ForwardOnly, Microsecond } + + public enum Direction { Forward, Reverse, EndOnly } + + /// The live-gate repro (#692): 8 values one second apart, read over [19:00:00.1, 19:00:10.5). + private static DataValueSnapshot[] EightValues() => + Enumerable.Range(1, 8).Select(i => Sample(i, T0.AddSeconds(i))).ToArray(); + + private static DataValueSnapshot Sample(double v, DateTime t) => new(v, StatusCodes.Good, t, t); + + private static (DateTime Start, DateTime End) Window(Direction direction) => direction switch + { + Direction.Forward => (T0.AddMilliseconds(100), T0.AddMilliseconds(10_500)), + Direction.Reverse => (T0.AddMilliseconds(10_500), T0.AddMilliseconds(100)), + _ => (DateTime.MinValue, T0.AddMilliseconds(10_500)), + }; + + public static TheoryData AllShapes() + { + var data = new TheoryData(); + foreach (var s in Enum.GetValues()) + foreach (var d in Enum.GetValues()) + foreach (var b in new[] { false, true }) + data.Add(s, d, b); + return data; + } + + /// + /// N = 1 paged to completion: every page carries exactly one value, the pages concatenate to the one-page answer, the + /// read never asks the backend for a tie-cluster over-fetch, and the backend read count stays proportional to the + /// pages (native backends: one data read per page plus the bound lookups). + /// + [Theory] + [MemberData(nameof(AllShapes))] + public async Task One_value_per_page_returns_one_value_on_every_page(Shape shape, Direction direction, bool bounds) + { + var source = Source(shape, EightValues()); + var (start, end) = Window(direction); + + var expected = await SinglePageAsync(source, start, end, bounds); + expected.Count.ShouldBeGreaterThanOrEqualTo(8); + source.ResetCounts(); + + var pages = await DrainPagesAsync(source, start, end, 1, bounds); + + Render(pages.SelectMany(p => p)).ShouldBe(Render(expected)); + pages.Select(p => p.Count).ShouldAllBe(c => c == 1, $"pages: {string.Join(",", pages.Select(p => p.Count))}"); + pages.Count.ShouldBe(expected.Count); + source.MaxCapRequested.ShouldBeLessThanOrEqualTo((uint)(ReverseWalk.DiscoveryRows + 2), + "no tie-cluster over-fetch for a series without ties"); + if (shape != Shape.ForwardOnly) + source.Reads.ShouldBeLessThanOrEqualTo(pages.Count + 4, $"{source.Reads} backend reads for {pages.Count} pages"); + } + + /// The live-gate numbers, pinned: 8 values at N = 1 on a native backend is 8 pages and 8 backend reads (was 17 / 25). + [Fact] + public async Task Eight_values_at_one_per_page_cost_eight_pages_and_eight_reads() + { + var source = Source(Shape.Native, EightValues()); + var (start, end) = Window(Direction.Forward); + + var pages = await DrainPagesAsync(source, start, end, 1, bounds: false); + + pages.Count.ShouldBe(8, $"{source.Reads} reads, {source.RequestedRows} rows requested"); + source.Reads.ShouldBe(8); + source.RequestedRows.ShouldBeLessThanOrEqualTo(8 * 3); + } + + /// A series whose tie clusters straddle the page boundaries at every page size. + private static DataValueSnapshot[] TiedValues() + { + var seconds = new[] { 1, 2, 2, 3, 3, 3, 4, 5, 5, 6, 7, 7, 7, 8 }; + return seconds.Select((s, i) => Sample(i, T0.AddSeconds(s))).ToArray(); + } + + public static TheoryData TiedShapes() + { + var data = new TheoryData(); + foreach (var s in Enum.GetValues()) + foreach (var d in Enum.GetValues()) + foreach (var b in new[] { false, true }) + foreach (var n in new uint[] { 1, 2, 3 }) + data.Add(s, d, b, n); + return data; + } + + /// + /// #669 tie semantics at the page boundary are kept: ties that straddle a page boundary come back once each, in the + /// read's order, at N = 1/2/3 — and every page that has a continuation point is full, no page is empty. + /// + [Theory] + [MemberData(nameof(TiedShapes))] + public async Task Tie_clusters_straddling_page_boundaries_page_without_loss_duplication_or_short_pages( + Shape shape, Direction direction, bool bounds, uint n) + { + var source = Source(shape, TiedValues()); + var (start, end) = Window(direction); + var expected = await SinglePageAsync(source, start, end, bounds); + + var pages = await DrainPagesAsync(source, start, end, n, bounds); + + Render(pages.SelectMany(p => p)).ShouldBe(Render(expected)); + pages.ShouldAllBe(p => p.Count > 0, "no empty page"); + for (var i = 0; i < pages.Count - 1; i++) + pages[i].Count.ShouldBe((int)n, $"page {i} of {pages.Count}: {string.Join(",", pages.Select(p => p.Count))}"); + } + + /// + /// A read whose total is an exact multiple of the page size ends on its last full page: the lookahead row tells the + /// server there is nothing more, so there is no trailing 0-value page (the #692 gate's reverse N = 2 observation). + /// + [Theory] + [InlineData(Shape.Native, Direction.Forward)] + [InlineData(Shape.Native, Direction.Reverse)] + [InlineData(Shape.ForwardOnly, Direction.Forward)] + [InlineData(Shape.ForwardOnly, Direction.Reverse)] + [InlineData(Shape.Microsecond, Direction.Reverse)] + public async Task An_exact_multiple_ends_on_its_last_full_page(Shape shape, Direction direction) + { + var (start, end) = Window(direction); + var pages = await DrainPagesAsync(Source(shape, EightValues()), start, end, 2, bounds: false); + + pages.Select(p => p.Count).ShouldBe([2, 2, 2, 2]); + } + + /// + /// #692 (low): a one-value bounded read whose start bound is not found asks whether the read holds anything. When the + /// answer is "only the end bound", the bound it just found rides the continuation point — the next page is served + /// without reading the backend again. + /// + [Theory] + [InlineData(Shape.Native, Direction.Forward)] + [InlineData(Shape.Native, Direction.Reverse)] + [InlineData(Shape.ForwardOnly, Direction.Forward)] + [InlineData(Shape.ForwardOnly, Direction.Reverse)] + [InlineData(Shape.Microsecond, Direction.Forward)] + public async Task A_start_bound_only_page_carries_the_end_bound_it_found(Shape shape, Direction direction) + { + // Forward [5s, 7s): nothing at/before 5s, nothing inside, the end bound at 9s. Reverse (5s ← 7s]: mirrored. + var series = direction == Direction.Forward ? new[] { Sample(9, T0.AddSeconds(9)) } : [Sample(3, T0.AddSeconds(3))]; + var source = Source(shape, series); + var (start, end) = direction == Direction.Forward ? (T0.AddSeconds(5), T0.AddSeconds(7)) : (T0.AddSeconds(7), T0.AddSeconds(5)); + + var (first, _, cp) = await ReadAsync(source, start, end, 1, bounds: true, cp: null); + first.Single().StatusCode.Code.ShouldBe(StatusCodes.BadBoundNotFound); + cp.ShouldNotBeNull(); + + source.ResetCounts(); + var (second, _, cp2) = await ReadAsync(source, start, end, 1, bounds: true, cp: cp); + second.Single().Value.ShouldBe(direction == Direction.Forward ? 9d : 3d); + cp2.ShouldBeNull(); + source.Reads.ShouldBe(0, "the end bound was found by the first page and carried"); + } + + /// + /// #692 (low): a one-value bounded reverse read on a forward-only backend walks back to answer "is there any data"; + /// the walk hint that walk computed rides the continuation point (it used to be dropped). + /// + [Fact] + public async Task A_start_bound_only_page_carries_the_walk_hint_it_computed() + { + var source = Source(Shape.ForwardOnly, EightValues()); + var store = new RecordingStore(); + await fx.Gate.WaitAsync(TestContext.Current.CancellationToken); + try + { + fx.Nm.HistoryContinuationStore = store; + fx.Nm.HistorianDataSource = source; + // Reverse (0.1s ← 30s]: no sample at or after 30s ⇒ the start bound is a placeholder; the data is below. + var (values, _, cp) = Read(T0.AddSeconds(30), T0.AddMilliseconds(100), 1, bounds: true, cp: null); + values.Single().StatusCode.Code.ShouldBe(StatusCodes.BadBoundNotFound); + cp.ShouldNotBeNull(); + store.Saved.Last().WalkHint.WidthTicks.ShouldBeGreaterThan(0); + } + finally + { + fx.Nm.HistoryContinuationStore = new InMemoryHistoryContinuationStore(); + fx.Gate.Release(); + } + } + + /// + /// #692 (low): only a SERVER-synthesised bound is a placeholder. A stored row that happens to carry + /// Bad_BoundNotFound with a null value (a client can write one through HistoryUpdate) is data — it used to make + /// the read look "all placeholders" and come back GoodNoData with no values. + /// + [Fact] + public async Task A_stored_row_with_the_bound_not_found_status_is_data_not_a_placeholder() + { + var stored = new DataValueSnapshot(null, StatusCodes.BadBoundNotFound, T0.AddSeconds(6), T0.AddSeconds(6)); + HistoryBounds.IsPlaceholder(stored).ShouldBeFalse(); + HistoryBounds.IsPlaceholder(HistoryBounds.NotFound(T0)).ShouldBeTrue(); + + var (values, _, cp) = await ReadAsync(Source(Shape.Native, [stored]), T0.AddSeconds(5), T0.AddSeconds(7), 0, bounds: true, cp: null); + cp.ShouldBeNull(); + values.Count.ShouldBe(3, "start placeholder, the stored row, end placeholder"); + values[1].SourceTimestamp.ShouldBe(T0.AddSeconds(6)); + } + + // --- helpers -------------------------------------------------------------------------------- + + private static string Render(IEnumerable values) => + string.Join(" | ", values.Select(v => $"{v.SourceTimestamp:HH:mm:ss.fffffff}={v.Value ?? "null"}/{v.StatusCode.Code:X8}")); + + private async Task> SinglePageAsync(CountingSource source, DateTime start, DateTime end, bool bounds) + { + var (values, error, cp) = await ReadAsync(source, start, end, 10_000, bounds, cp: null); + error.StatusCode.Code.ShouldBe(StatusCodes.Good); + cp.ShouldBeNull(); + return values.ToList(); + } + + private async Task>> DrainPagesAsync( + CountingSource source, DateTime start, DateTime end, uint max, bool bounds) + { + var pages = new List>(); + byte[]? cp = null; + do + { + var (values, error, next) = await ReadAsync(source, start, end, max, bounds, cp); + error.StatusCode.Code.ShouldBe(StatusCodes.Good); + pages.Add(values); + cp = next; + pages.Count.ShouldBeLessThan(80, "paging must terminate"); + } + while (cp is not null); + + return pages; + } + + private async Task<(IReadOnlyList Values, ServiceResult Error, byte[]? Cp)> ReadAsync( + IHistorianDataSource source, DateTime start, DateTime end, uint max, bool bounds, byte[]? cp) + { + await fx.Gate.WaitAsync(TestContext.Current.CancellationToken); + try + { + fx.Nm.HistorianDataSource = source; + return Read(start, end, max, bounds, cp); + } + finally { fx.Gate.Release(); } + } + + private (IReadOnlyList Values, ServiceResult Error, byte[]? Cp) Read( + DateTime start, DateTime end, uint max, bool bounds, byte[]? cp) + { + var context = new OperationContext(new RequestHeader(), secureChannelContext: null, RequestType.HistoryRead, identity: null); + var details = new ReadRawModifiedDetails + { + StartTime = start, EndTime = end, NumValuesPerNode = max, ReturnBounds = bounds, IsReadModified = false, + }; + var nodesToRead = new List { new() { NodeId = fx.NodeId, ContinuationPoint = cp } }; + var results = new List { null! }; + var errors = new List { null! }; + fx.Nm.HistoryRead(context, details, TimestampsToReturn.Both, releaseContinuationPoints: false, nodesToRead, results, errors); + + var values = new List(); + if (results[0]?.HistoryData is { } ho && !ExtensionObject.IsNull(ho)) + values.AddRange(((HistoryData)ExtensionObject.ToEncodeable(ho)).DataValues); + return (values, errors[0], results[0]?.ContinuationPoint); + } + + private static CountingSource Source(Shape shape, IReadOnlyList series) => shape switch + { + Shape.Native => new CountingSource(series, reverse: true, resolution: 1), + Shape.ForwardOnly => new CountingSource(series, reverse: false, resolution: 1), + _ => new CountingSource(series, reverse: true, resolution: 10), + }; + + /// Records every saved cursor (to inspect what rides a continuation point). + private sealed class RecordingStore : IHistoryContinuationStore + { + private readonly InMemoryHistoryContinuationStore _inner = new(); + public List Saved { get; } = []; + + public byte[]? Save(ISession? session, HistoryContinuationState state) + { + Saved.Add(state); + return _inner.Save(session, state); + } + + public HistoryContinuationState? TryTake(ISession? session, byte[] continuationPoint) => _inner.TryTake(session, continuationPoint); + + public void Release(ISession? session, byte[] continuationPoint) => _inner.Release(session, continuationPoint); + } + + /// + /// A series-backed historian that counts what it is asked for. reverse: serves start > end newest-first + /// (SqlServer/QuestDB); otherwise refuses it (the Gateway — the server walks back with forward reads). Forward reads + /// are [start, end), a point read the instant; resolution > 1 truncates the window bounds (QuestDB µs). + /// + private sealed class CountingSource(IReadOnlyList series, bool reverse, long resolution) : IHistorianDataSource + { + private readonly Lock _gate = new(); + + public int Reads { get; private set; } + public long RequestedRows { get; private set; } + public uint MaxCapRequested { get; private set; } + + public void ResetCounts() + { + lock (_gate) { Reads = 0; RequestedRows = 0; MaxCapRequested = 0; } + } + + public bool SupportsReverseRaw => reverse; + + public long TimestampResolutionTicks => resolution; + + private DateTime Trunc(DateTime t) => new(t.Ticks - t.Ticks % resolution, DateTimeKind.Utc); + + public Task ReadRawAsync( + string fullReference, DateTime startUtc, DateTime endUtc, uint maxValuesPerNode, CancellationToken cancellationToken) + { + lock (_gate) + { + Reads++; + RequestedRows += maxValuesPerNode; + if (maxValuesPerNode > MaxCapRequested) MaxCapRequested = maxValuesPerNode; + } + + static DateTime Ts(DataValueSnapshot s) => s.SourceTimestampUtc ?? DateTime.MinValue; + startUtc = Trunc(startUtc); + endUtc = Trunc(endUtc); + List window; + if (startUtc > endUtc) + { + if (!reverse) throw new InvalidOperationException("start_time must be less than or equal to end_time. (InvalidArgument)"); + window = series.Where(s => Ts(s) > endUtc && Ts(s) <= startUtc).Reverse().ToList(); + } + else if (startUtc == endUtc) window = series.Where(s => Ts(s) == startUtc).ToList(); + else window = series.Where(s => Ts(s) >= startUtc && Ts(s) < endUtc).ToList(); + + var page = maxValuesPerNode == 0 ? window : window.Take((int)maxValuesPerNode).ToList(); + return Task.FromResult(new HistorianRead(page, null)); + } + + public Task ReadProcessedAsync( + string fullReference, DateTime startUtc, DateTime endUtc, TimeSpan interval, + HistoryAggregateType aggregate, CancellationToken cancellationToken) => + Task.FromResult(new HistorianRead(Array.Empty(), null)); + + public Task ReadAtTimeAsync( + string fullReference, IReadOnlyList timestampsUtc, CancellationToken cancellationToken) => + Task.FromResult(new HistorianRead(Array.Empty(), null)); + + public Task ReadEventsAsync( + string? sourceName, DateTime startUtc, DateTime endUtc, int maxEvents, CancellationToken cancellationToken) => + Task.FromResult(new HistoricalEventsResult(Array.Empty(), null)); + + public HistorianHealthSnapshot GetHealthSnapshot() => NullHistorianDataSource.Instance.GetHealthSnapshot(); + + public void Dispose() { } + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadPagingTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadPagingTests.cs index b881d737..4a471f56 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadPagingTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadPagingTests.cs @@ -29,8 +29,8 @@ public sealed class NodeManagerHistoryReadPagingTests : IDisposable /// A full first page emits a continuation point; resuming returns subsequent pages; the /// final short page clears the continuation point — and the union of pages is the complete series with - /// no duplicate or skipped sample. (Resumed pages net slightly fewer than the cap because each resume - /// re-reads + trims the boundary sample to stay tie-safe — see the design; the union is what matters.) + /// no duplicate or skipped sample. (#692: every page is full — a resume re-reads the boundary ties it trims plus one + /// lookahead row, so 250 at 100 is exactly three pages of 100, 100 and 50.) [Fact] public async Task Raw_pages_full_series_across_continuation_points() { @@ -64,7 +64,7 @@ public sealed class NodeManagerHistoryReadPagingTests : IDisposable while (cp is not null); sawCp.ShouldBeTrue("a 250-sample series capped at 100 must page"); - pageCount.ShouldBeGreaterThan(1); + pageCount.ShouldBe(3, "100 + 100 + 50 — every page full (#692)"); // The union is the exact series — no duplicates, no skips, in order. collected.Count.ShouldBe(250); @@ -132,11 +132,11 @@ public sealed class NodeManagerHistoryReadPagingTests : IDisposable await host.DisposeAsync(); } - /// An exactly-full series whose total is a multiple of the cap pages to a trailing - /// EMPTY page (GoodNoData, no CP) — the server can't know the prior page was the last until it reads - /// past the end. + /// An exactly-full series whose total is a multiple of the cap ends on its last full page: each read asks for + /// one lookahead row past the page (#692), so the server knows the page is the last and emits no continuation point — + /// there is no trailing EMPTY page any more. A stale point resumed anyway still terminates with GoodNoData. [Fact] - public async Task Raw_exact_multiple_terminates_with_empty_final_page() + public async Task Raw_exact_multiple_ends_on_its_last_full_page() { var (host, server) = await BootAsync(); var nm = server.NodeManager!; @@ -149,12 +149,17 @@ public sealed class NodeManagerHistoryReadPagingTests : IDisposable var (r1, _, cp1) = ReadRaw(nm, nodeId, Epoch, Epoch.AddHours(1), max: 100, inboundCp: null); r1.Count.ShouldBe(100); - cp1.ShouldNotBeNull("a full page emits a CP even when it happens to be the last"); + cp1.ShouldBeNull("the lookahead row found nothing past the page — it is the last"); - var (r2, e2, cp2) = ReadRaw(nm, nodeId, Epoch, Epoch.AddHours(1), max: 100, inboundCp: cp1); - r2.Count.ShouldBe(0); + // Positive control: one more sample past the page ⇒ the same full page DOES carry a continuation point. + nm.HistorianDataSource = new SeriesHistorianDataSource(MakeSeries(count: 101, stepSeconds: 1)); + var (r2, e2, cp2) = ReadRaw(nm, nodeId, Epoch, Epoch.AddHours(1), max: 100, inboundCp: null); + r2.Count.ShouldBe(100); e2.StatusCode.Code.ShouldBe(StatusCodes.Good); - cp2.ShouldBeNull("the empty trailing page terminates paging"); + cp2.ShouldNotBeNull(); + var (r3, _, cp3) = ReadRaw(nm, nodeId, Epoch, Epoch.AddHours(1), max: 100, inboundCp: cp2); + r3.ShouldBe([100d]); + cp3.ShouldBeNull(); await host.DisposeAsync(); } @@ -675,8 +680,9 @@ public sealed class NodeManagerHistoryReadPagingTests : IDisposable var (values, error, next) = ReadRaw(nm, nodeId, DateTime.MinValue, end, max: 3, inboundCp: cp); error.StatusCode.Code.ShouldBe(StatusCodes.Good); values.ShouldNotBeEmpty(); - values[^1].ShouldBe((double)(expected - 2), $"page {page} continues exactly"); - expected = (int)values[^1]; + // #692: every page is full (N = 3) — a resumed page used to net N − 1 after trimming its boundary sample. + values.ShouldBe([expected, expected - 1d, expected - 2d], $"page {page} continues exactly"); + expected -= 3; next.ShouldNotBeNull(); cp = next; diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadTests.cs index a3c8a266..0ecffbe1 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadTests.cs @@ -66,7 +66,7 @@ public sealed class NodeManagerHistoryReadTests : IDisposable fake.LastTagname.ShouldBe("WW.Temp"); fake.LastStart.ShouldBe(start); fake.LastEnd.ShouldBe(end); - fake.LastMaxValues.ShouldBe(100u); + fake.LastMaxValues.ShouldBe(101u, "the page plus one lookahead row (#692) — it tells the server whether more follows"); errors[0].StatusCode.Code.ShouldBe(StatusCodes.Good); results[0].StatusCode.Code.ShouldBe(StatusCodes.Good);