dohertj2/lmxopcua
1
0
Fork 0
Code Issues 1 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(driver-historian-wonderware): resolve Medium code-review finding (Driver.Historian.Wonderware-009)

Browse Source 
Apply _config.MaxValuesPerRead as a bucket cap in ReadAggregateAsync,
mirroring the existing cap in ReadRawAsync. Without this guard a processed
read over a wide time range with a small IntervalMs could accumulate an
unbounded HistorianAggregateSample list; if the serialised reply exceeded
the 16 MiB FrameWriter frame cap WriteAsync would throw and the client
correlation-id wait would hang. Truncation now logs a Warning with a hint
to widen IntervalMs or reduce the time range.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
2026-05-22 09:24:25 -04:00
parent 7474631992
commit 1723f5d5cd
2 changed files with 17 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
code-reviews/Driver.Historian.Wonderware/findings.md
View File

@@ -235,7 +235,7 @@ treat an SDK error as an empty history.
| Severity | Medium | | Severity | Medium |
| Category | Performance and resource management | | Category | Performance and resource management |
| Location | `Backend/HistorianDataSource.cs:382-395`, `Ipc/Contracts.cs:85-99` | | Location | `Backend/HistorianDataSource.cs:382-395`, `Ipc/Contracts.cs:85-99` |
| Status | Open | | Status | Resolved |
**Description:** `ReadAggregateAsync` drains `query.MoveNext` into `results` with **Description:** `ReadAggregateAsync` drains `query.MoveNext` into `results` with
no upper bound, unlike `ReadRawAsync`, which honours `maxValues` / no upper bound, unlike `ReadRawAsync`, which honours `maxValues` /
@@ -252,7 +252,7 @@ sidecar holds the whole result set in memory.
`ReadProcessedRequest`. Reject or truncate result sets that would exceed the frame `ReadProcessedRequest`. Reject or truncate result sets that would exceed the frame
cap with an explicit error reply rather than letting `WriteAsync` throw. cap with an explicit error reply rather than letting `WriteAsync` throw.
**Resolution:** _(open)_ **Resolution:** Resolved 2026-05-22 — applied `_config.MaxValuesPerRead` as a bucket cap in `ReadAggregateAsync` mirroring the raw-read path; truncation logs a Warning with the limit and a hint to widen `IntervalMs` or reduce the time range.
### Driver.Historian.Wonderware-010 ### Driver.Historian.Wonderware-010

15
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware/Backend/HistorianDataSource.cs
View File

@@ -373,6 +373,12 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
return Task.FromResult(results); return Task.FromResult(results);
} }
// Apply the same bucket cap as the raw-read path so a wide time range with a
// small IntervalMs cannot produce an unbounded result set that would overflow
// the 16 MiB FrameWriter frame cap and lose the entire reply.
var bucketLimit = _config.MaxValuesPerRead;
var bucketCount = 0;
while (query.MoveNext(out error)) while (query.MoveNext(out error))
{ {
ct.ThrowIfCancellationRequested(); ct.ThrowIfCancellationRequested();
@@ -386,6 +392,15 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
Value = value, Value = value,
TimestampUtc = timestamp, TimestampUtc = timestamp,
}); });
bucketCount++;
if (bucketLimit > 0 && bucketCount >= bucketLimit)
{
Log.Warning(
"HistoryRead aggregate ({Aggregate}): {Tag} truncated at {Limit} buckets — widen IntervalMs or reduce time range",
aggregateColumn, tagName, bucketLimit);
break;
}
} }
query.EndQuery(out _); query.EndQuery(out _);