fix(opcua): fit the raw end bound against the page's effective cap
H1 fuzz finding (routed to Lane B because AppendEndBoundAsync lives in OtOpcUaNodeManager.cs): a bounded HistoryRead-Raw page sized BELOW NumValuesPerNode by the backend's row ceiling (RawPageLimit.Plan) took the end bound whenever its data fitted, because the fit was judged against NumValuesPerNode instead of the page's effective cap — a full ceiling-sized page came back with cap + 1 values. The bound is now fitted against the page allowance (the ceiling-adjusted data cap plus the start bound a first page already carries); when the page is full the bound goes to the next page through the continuation point. Test: NodeManagerHistoryReadBoundsTests .A_full_ceiling_sized_page_carries_the_end_bound_on_the_next_page (ceiling 4 ⇒ 3 data rows; first page = start bound + 3 with a CP, second page = the end bound alone; control: no ceiling ⇒ one page of 5). Refs #732 #736 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
Joseph Doherty
@@ -6781,13 +6781,17 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
|
||||
// The data is drained — the end bound ends this page when it has room (Part 11: bounds count toward
|
||||
// NumValuesPerNode), else it is the whole next page. Returns the continuation point to emit, if any.
|
||||
async Task<byte[]?> AppendEndBoundAsync(List<DataValueSnapshot> page, DateTime resumedAtUtc)
|
||||
// `pageAllowance` is the EFFECTIVE cap of this page — NumValuesPerNode, less whatever the backend's row
|
||||
// ceiling (RawPageLimit) took off it, plus the start bound a first page already carries — not
|
||||
// NumValuesPerNode itself: against N a ceiling-sized page that was full still took the bound, returning
|
||||
// cap + 1 values (#736 fuzz). 0 = unbounded.
|
||||
async Task<byte[]?> AppendEndBoundAsync(List<DataValueSnapshot> page, DateTime resumedAtUtc, uint pageAllowance)
|
||||
{
|
||||
// The previous returned value stamps an open bound's placeholder (Part 11 Table 1 footnotes a/b): this
|
||||
// page's last, else the last of an earlier page (carried in the cursor), else where the read resumed.
|
||||
var previous = (page.Count > 0 ? page[^1].SourceTimestampUtc : null)
|
||||
?? endBound!.LastReturnedUtc ?? resumedAtUtc;
|
||||
if (numValuesPerNode == 0 || page.Count < numValuesPerNode)
|
||||
if (pageAllowance == 0 || page.Count < pageAllowance)
|
||||
{
|
||||
page.Add(await HistoryBounds.EndBoundAsync(
|
||||
HistorianDataSource, tagname, endBound!, previous, MaxTieClusterOverfetch, nowUtc, ct).ConfigureAwait(false));
|
||||
@@ -6832,7 +6836,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
&& (endBound.DataExhausted || HistoryBounds.DataWindowDrained(startUtc, endBound, resolution)))
|
||||
{
|
||||
var onlyBound = new List<DataValueSnapshot>(1);
|
||||
var afterCp = await AppendEndBoundAsync(onlyBound, startUtc).ConfigureAwait(false);
|
||||
var afterCp = await AppendEndBoundAsync(onlyBound, startUtc, numValuesPerNode).ConfigureAwait(false);
|
||||
results[handle.Index] = new SdkHistoryReadResult
|
||||
{
|
||||
StatusCode = StatusCodes.Good,
|
||||
@@ -7052,7 +7056,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
else if (endBound is not null)
|
||||
{
|
||||
// The data is drained — the end bound ends this page, or is the next one when this one is full.
|
||||
clusterCp = await AppendEndBoundAsync(slice, startUtc).ConfigureAwait(false);
|
||||
clusterCp = await AppendEndBoundAsync(slice, startUtc, readCap).ConfigureAwait(false);
|
||||
}
|
||||
|
||||
results[handle.Index] = new SdkHistoryReadResult
|
||||
@@ -7105,7 +7109,9 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
// The data is drained — the end bound ends this page, or is the next one when this one is full.
|
||||
var page = new List<DataValueSnapshot>(samples.Count + 1);
|
||||
page.AddRange(samples);
|
||||
outboundCp = await AppendEndBoundAsync(page, startUtc).ConfigureAwait(false);
|
||||
// This page's allowance: the (ceiling-adjusted) data cap, plus the start bound it already leads with.
|
||||
var allowance = readCap == 0 ? 0u : readCap + (startBound is not null ? 1u : 0u);
|
||||
outboundCp = await AppendEndBoundAsync(page, startUtc, allowance).ConfigureAwait(false);
|
||||
samples = page;
|
||||
historyData = ToHistoryDataFromSamples(samples);
|
||||
}
|
||||
|
||||
+33
-1
@@ -292,6 +292,35 @@ public sealed class NodeManagerHistoryReadBoundsTests(BoundsServerFixture fx) :
|
||||
Render(values, T("5:00"), T("5:05")).ShouldBe("5:00, 5:02, 5:03, 5:05");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// #736 (H1 fuzz) — a backend row ceiling sizes the page BELOW NumValuesPerNode (RawPageLimit: ceiling 4 ⇒ 3 data
|
||||
/// rows per page). When the data exactly fills that page, the end bound must go to the NEXT page through the
|
||||
/// continuation point, not be appended to a page that is already at its effective cap: the fit used to be
|
||||
/// judged against NumValuesPerNode (10 here), so the page came back with cap + 1 values.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task A_full_ceiling_sized_page_carries_the_end_bound_on_the_next_page()
|
||||
{
|
||||
var source = new SeriesSource(TableData(), reverse: true, inclusiveForwardEnd: false, resolution: 1, ceiling: 4);
|
||||
var (first, error, cp) = await ReadAsync(source, T("5:01"), T("5:06"), max: 10, bounds: true, cp: null);
|
||||
error.StatusCode.Code.ShouldBe(StatusCodes.Good);
|
||||
|
||||
// Start bound 5:00 + the three data rows the ceiling allows = the page's effective cap, exactly.
|
||||
Render(first, T("5:01"), T("5:06")).ShouldBe("5:00, 5:02, 5:03, 5:05");
|
||||
cp.ShouldNotBeNull("the page is full, so the end bound is owed on the next page");
|
||||
|
||||
var (second, error2, cp2) = await ReadAsync(source, T("5:01"), T("5:06"), max: 10, bounds: true, cp);
|
||||
error2.StatusCode.Code.ShouldBe(StatusCodes.Good);
|
||||
Render(second, T("5:01"), T("5:06")).ShouldBe("5:06");
|
||||
cp2.ShouldBeNull();
|
||||
|
||||
// Control: without a ceiling the same read fits on ONE page of NumValuesPerNode — the fit keys on the
|
||||
// effective cap, not on a smaller constant.
|
||||
var (whole, _, cpWhole) = await ReadAsync(Source(Shape.Reverse, TableData()), T("5:01"), T("5:06"), max: 10, bounds: true, cp: null);
|
||||
Render(whole, T("5:01"), T("5:06")).ShouldBe("5:00, 5:02, 5:03, 5:05, 5:06");
|
||||
cpWhole.ShouldBeNull();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// #691 review: no data AND no bound found ⇒ <c>GoodNoData</c> with no values — the answer a tag with no history (and a
|
||||
/// disabled historian) gave before bounds (§6.5.3.2: bounds make the result Good only "if one or both exist"). Only
|
||||
@@ -468,7 +497,7 @@ public sealed class NodeManagerHistoryReadBoundsTests(BoundsServerFixture fx) :
|
||||
/// a point read is the instant. <paramref name="resolution"/> > 1 truncates every window bound to it first (QuestDB
|
||||
/// µs); the series must already be stored at that resolution.
|
||||
/// </summary>
|
||||
private sealed class SeriesSource(IReadOnlyList<DataValueSnapshot> series, bool reverse, bool inclusiveForwardEnd, long resolution)
|
||||
private sealed class SeriesSource(IReadOnlyList<DataValueSnapshot> series, bool reverse, bool inclusiveForwardEnd, long resolution, int ceiling = 0)
|
||||
: IHistorianDataSource
|
||||
{
|
||||
public int ReverseWindowsOnForwardOnly;
|
||||
@@ -477,6 +506,9 @@ public sealed class NodeManagerHistoryReadBoundsTests(BoundsServerFixture fx) :
|
||||
|
||||
public long TimestampResolutionTicks => resolution;
|
||||
|
||||
/// <summary>A strict row ceiling (0 = none), so a page is sized below NumValuesPerNode (RawPageLimit).</summary>
|
||||
public int MaxRawValuesPerRead => ceiling;
|
||||
|
||||
private DateTime Trunc(DateTime t) => new(t.Ticks - t.Ticks % resolution, DateTimeKind.Utc);
|
||||
|
||||
public Task<HistorianRead> ReadRawAsync(
|
||||
|
||||
Reference in New Issue
Block a user