fix(historian): live writers rethrow pass cancellation; gateway writes normalised (lmxopcua#746)

Every IHistorianValueWriter caught Exception and returned false, including
OperationCanceledException from the drain's own token. The recorder's guard
("a cancelled pass is not a tag failure") was therefore dead code: at shutdown
mid-pass each cancelled group could be booked as a consecutive failure and,
with a sibling that already wrote, tip a healthy tag toward quarantine.
SqlServer, QuestDb and Gateway writers now rethrow a cancellation of the
caller's token; a cancellation-shaped error on a live token is still an
ordinary failed write.

The Gateway writer relabelled a Local-kind timestamp as UTC instead of
converting it, so the same value could land at two different times on two
backends. Stamped samples now go through LiveWritePreparer (Local -> UTC,
non-finite doubles not sent, duplicate timestamps keep the first); unstamped
samples are still left for the gateway to stamp, in batch order.

Rebased from the unpushed remed/lane-H2 branch (originally authored 2026-10-22
in session_01Ub9hGmYLQvn9XETzw7CMsT) onto current master.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
Joseph Doherty
2026-10-22 16:42:13 -04:00
parent bc7e58ed64
commit 7c5cc9c7e5
7 changed files with 204 additions and 29 deletions
@@ -2,6 +2,7 @@ using Google.Protobuf.WellKnownTypes;
using Microsoft.Extensions.Logging;
using ZB.MOM.WW.HistorianGateway.Contracts.Grpc;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
using ZB.MOM.WW.OtOpcUa.Driver.Historian.Sql.Common;
namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Recorder;
@@ -14,11 +15,11 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Recorder;
/// </summary>
/// <remarks>
/// <para>
/// <b>Non-throwing by contract.</b> The recorder's drain loop stays simple by treating the
/// writer as never throwing: any gateway/transport error (and a non-success, non-queued ack)
/// is mapped to <c>false</c> so the recorder retains the outbox entries and retries. Only the
/// failure category (the exception type name) is logged — never tag values, hostnames, or
/// credentials.
/// <b>Non-throwing by contract, except for cancellation.</b> Any gateway/transport error (and a
/// non-success, non-queued ack) is mapped to <c>false</c> so the recorder retains the outbox entries and
/// retries; a cancellation of the caller's token is rethrown (it ends the drain pass and must not count as
/// this tag's failure). Only the failure category (the exception type name) is logged — never tag values,
/// hostnames, or credentials.
/// </para>
/// <para>
/// A success ack OR a store-forward-queued ack maps to <c>true</c>: a value the gateway
@@ -41,6 +42,11 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Recorder;
/// never thrown; a batch with none left is acked without calling the gateway. The recorder already drops
/// those kinds at capture for this backend, so the skip is a backstop.
/// </para>
/// <para>
/// <b>Batch normalisation is the SQL backends'</b> (<see cref="LiveWritePreparer.Prepare"/>): a Local
/// timestamp is converted to UTC, NaN / ±Infinity are not sent (they become Empty), and duplicate timestamps in
/// a batch keep the first. An unstamped value is left unstamped for the gateway to stamp.
/// </para>
/// </remarks>
public sealed class GatewayHistorianValueWriter : IHistorianValueWriter, IAsyncDisposable
{
@@ -71,47 +77,47 @@ public sealed class GatewayHistorianValueWriter : IHistorianValueWriter, IAsyncD
try
{
// The SQL backends' batch normalisation (LiveWritePreparer): a Local timestamp is converted to UTC (it used to be
// relabelled, shifting it by the node's offset), a non-finite double becomes Empty (so it is not sent — the live
// path is numeric-only), and a duplicate timestamp keeps the first. An UNSTAMPED value stays unstamped — the
// gateway stamps it — so each stamped value is normalised on its own (Prepare would stamp a null with the node's
// clock) and the batch keeps its order.
var seen = new HashSet<long>();
var liveValues = new List<HistorianLiveValue>(values.Count);
foreach (HistorizationValue value in values)
{
if (!value.Value.TryGetDouble(out double number))
if (value.TimestampUtc is null)
{
// The gateway live path is numeric-only; non-numeric kinds are skipped, never thrown.
_logger.LogDebug("Skipping non-numeric {Kind} value for live write.", value.Value.Kind);
if (ToLive(value.Value, value.StatusCode) is { } unstamped) liveValues.Add(unstamped);
continue;
}
var live = new HistorianLiveValue
{
NumericValue = number,
// Documented-dead input on the WriteLiveValues SQL path (0.2.0 Contracts C-002 —
// server-managed, silently discarded). Populated deliberately from the value's
// status; Good maps to 192 as before. See the class <remarks>.
Quality = Mapping.GatewayQualityMapper.ToDaQuality(value.StatusCode),
};
if (value.TimestampUtc is { } timestampUtc)
{
// Timestamp.FromDateTime requires Utc kind; coerce defensively. A null timestamp
// leaves the proto field unset -> the gateway's SQL writer server-stamps now.
live.Timestamp = Timestamp.FromDateTime(DateTime.SpecifyKind(timestampUtc, DateTimeKind.Utc));
}
var row = LiveWritePreparer.Prepare([value], static () => DateTime.UtcNow)[0];
if (!seen.Add(row.TimeUtc.Ticks)) continue; // first wins, as on the SQL backends
if (ToLive(row.Value, row.StatusCode) is not { } live) continue;
live.Timestamp = Timestamp.FromDateTime(row.TimeUtc); // Utc kind (Prepare normalises it)
liveValues.Add(live);
}
if (liveValues.Count == 0)
{
// Every value was non-numeric: nothing to send, trivially drained.
// Every value was non-numeric (or non-finite): nothing to send, trivially drained.
return true;
}
WriteAck ack = await _client.WriteLiveValuesAsync(tag, liveValues, ct).ConfigureAwait(false);
return ack.Success || ack.Queued;
}
catch (OperationCanceledException) when (ct.IsCancellationRequested)
{
// The drain pass was cancelled (shutdown): rethrow — reported as false it would be booked as THIS tag's
// failure and could tip a healthy tag over its attempt budget. A cancellation-shaped error on a live token
// (the gateway's own deadline) falls through to the catch-all: an ordinary failed write.
throw;
}
catch (Exception exception)
{
// NEVER throw out of the writer — the recorder's drain expects a bool so its retain/retry
// Otherwise never throw out of the writer — the recorder's drain expects a bool so its retain/retry
// logic stays simple. Log only the failure category (no value content, hostnames, or creds).
_logger.LogDebug(
"WriteLiveValues failed ({Exception}); recorder will retain and retry.",
@@ -120,6 +126,25 @@ public sealed class GatewayHistorianValueWriter : IHistorianValueWriter, IAsyncD
}
}
/// <summary>The proto live value of a numeric sample (no timestamp), or null for a kind with no double form.</summary>
private HistorianLiveValue? ToLive(HistorianValue value, uint statusCode)
{
if (!value.TryGetDouble(out double number) || !double.IsFinite(number))
{
// The gateway live path is numeric-only; non-numeric kinds (and non-finite doubles) are skipped, never thrown.
_logger.LogDebug("Skipping non-numeric {Kind} value for live write.", value.Kind);
return null;
}
return new HistorianLiveValue
{
NumericValue = number,
// Documented-dead input on the WriteLiveValues SQL path (0.2.0 Contracts C-002 — server-managed, silently
// discarded). Populated deliberately from the value's status; Good maps to 192. See the class remarks.
Quality = Mapping.GatewayQualityMapper.ToDaQuality(statusCode),
};
}
/// <summary>
/// Disposes the underlying gateway client (and its gRPC channel). The DI container owns this
/// writer as a singleton, so this fires once at host shutdown — closing the channel gracefully.
@@ -14,7 +14,8 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.QuestDb;
/// semantics: QuestDB's <c>DEDUP UPSERT KEYS(ts, tag)</c> would let a re-sent sample overwrite the stored one (and a
/// re-sent live sample resurrect a tombstone), so the writer reads which timestamps already exist for the tag in
/// the batch's range (tombstones count as present, T0 Q11 range form) and inserts only the absent rows.
/// Never throws; returns false on any failure so the recorder keeps its entries.
/// Returns false on any failure so the recorder keeps its entries — except a cancellation of the caller's token, which
/// is rethrown (it ends the pass; it is not this tag's failure).
/// </summary>
/// <remarks>
/// Batch normalisation is <see cref="LiveWritePreparer.Prepare"/> at µs resolution (non-finite doubles become Empty
@@ -101,6 +102,10 @@ internal sealed class QuestDbHistorianValueWriter : IHistorianValueWriter
await _wal.WaitAsync(_conn.SampleTable, ct).ConfigureAwait(false); // timeout/suspension does not change the result (R5)
return true;
}
catch (OperationCanceledException) when (ct.IsCancellationRequested)
{
throw; // the drain pass was cancelled (shutdown) — not a write failure of this tag
}
catch (Exception ex)
{
_log.LogWarning("QuestDB historian live-value write failed ({Exception}); will retry.", ex.GetType().Name);
@@ -9,8 +9,9 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer;
/// <summary>
/// <see cref="IHistorianValueWriter"/> over the SQL <c>Sample</c> table. Redelivery-idempotent: rows are
/// inserted only where <c>(TagId, SourceTimeUtc)</c> is absent, so the recorder's at-least-once redelivery
/// leaves the first-stored sample untouched. Never throws; returns false on any failure so the recorder keeps
/// the entries and its per-tag quarantine keeps working.
/// leaves the first-stored sample untouched. Returns false on any failure so the recorder keeps the entries and its
/// per-tag quarantine keeps working — except a cancellation of the caller's token, which is rethrown: it ends the
/// PASS, and booked as a tag failure it could tip a healthy tag over its attempt budget.
/// </summary>
/// <remarks>
/// <b>Non-finite doubles.</b> NaN / ±Infinity cannot live in a SQL <c>float</c> (the TVP throws). They are stored
@@ -90,6 +91,10 @@ internal sealed class SqlServerHistorianValueWriter : IHistorianValueWriter
}
}
}
catch (OperationCanceledException) when (ct.IsCancellationRequested)
{
throw; // the drain pass was cancelled (shutdown) — not a write failure of this tag
}
catch (Exception ex)
{
_log.LogWarning("SQL historian live-value write failed ({Exception}); will retry.", ex.GetType().Name);