fix(mqtt): reset Sparkplug ingest state at every session/authoring seam
Task 21 review — two Criticals sharing one root cause, plus four follow-ups.
C1/C2 root cause: `OnReconnectedAsync` opened with `Births.Clear(); _trackers.Clear();`
— "not housekeeping, the correctness step" — and that reset was missing from the
other two seams where the view underneath a surviving cache changes.
- C1: the ingestor OUTLIVES a session rebuild. `SessionIdentity` is a strict
superset of `IngestIdentity`, so changing only Host/Port/ClientId/credentials/
TLS/a timeout gives `!SameSession && SameIngest` — `ReinitializeAsync` tears the
session down and re-establishes on the same instance, and `TeardownAsync` touches
no ingest state (the resilience layer re-running `InitializeAsync` is the second
path). A node that rebound alias 5 during the changeover would publish Pressure's
value under the Temperature RawPath at Good. Hoisted into `ResetSessionState`,
called from `AttachTo` (earliest seam — a persistent session can push between
CONNACK and our SUBSCRIBE), `EstablishAsync` and `OnReconnectedAsync`.
- C2: `Register` pruned trackers but not births. While a node is unauthored its
traffic is dropped at `Dispatch`, so its cached birth FREEZES rather than
refreshing; drop-then-re-add across a week mis-routes at Good, and `Births` grew
monotonically. Now evicted on edge-node departure.
Deliberately NARROWER than "absent from ByScope": a device with no authored tags
under a still-authored node keeps receiving traffic, so its birth is live, not
frozen — evicting it would discard correct state. Pinned both ways.
- I1: a birth whose seq is unreadable produced a permanent debounce-paced rebirth
loop. "A birth arrived" is now tracked separately from "the birth established a
baseline". Writing the test showed the cap belonged wider than the review scoped
it: data-before-birth and unknown-alias loop identically (20 messages → 23 NCMDs
against a cap of 3), so all three missing-metadata reasons share one per-node
consecutive cap, re-armed by any applied birth. A seq gap stays uncapped — it
self-limits.
- I2: the oversize `WarnOnce` key was the raw topic, off a group-wide `#`
subscription. `HandleMessage` now parses and filters to authored nodes BEFORE the
size check (also skipping the protobuf parse for discarded traffic), and `_warned`
carries a hard ceiling so a future bad derivation degrades to silence.
- I3: array metrics slipped the unsupported-type gate — `ToDriverDataType` returns
the ELEMENT type, so a String-typed tag published a packed `byte[]` as base64 at
Good. Gated on `IsSparkplugArray`.
- I4: driver-level Sparkplug coverage for the composition-root lines T22 left
untested — the `OnDataChange` re-raise, `ReadAsync` off the shared cache,
subscribe/unsubscribe dispatch, and both directions of the mode gate.
Minors: M1 the oversize test fed unparseable bytes and passed with the size check
deleted (now a real NBIRTH + an at-the-ceiling control); M2 answered in-place (an
absent seq is refused WITHOUT becoming the baseline, so the NCMD assertion is the
complete check); M3 `HostStateObserved` raise wrapped; M5 the legacy `STATE/{host}`
form is now subscribed, so the parser's tolerance is reachable in production; M8
`ResolveBinding` prefers the NAME over a disagreeing alias; M9 a narrowing float
conversion refuses instead of publishing ±Infinity at Good (a publisher's own
infinity still passes through). M4 was already resolved by T22.
Also fixed a race in the new tests: `publish.Topics.Clear()` after a call that
dispatches rebirths off-thread must drain first — it made the C1 sequence-baseline
pin pass vacuously in the first falsifiability run.
Falsifiability: dropping the reset from AttachTo+EstablishAsync reddens 4 (C1);
dropping the Register eviction reddens 2 (C2); widening the eviction predicate to
the reviewer's literal phrasing reddens the keep-live-births pin (C2b). 545/545 MQTT
unit tests green (was 524); whole-solution `--no-incremental` build clean under TWAE.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
This commit is contained in:
Joseph Doherty
@@ -108,6 +108,25 @@ public sealed class SparkplugIngestor
|
||||
/// <summary>The subscribed reference is not an authored Sparkplug tag.</summary>
|
||||
private const uint StatusBadNodeIdUnknown = 0x80340000u;
|
||||
|
||||
/// <summary>
|
||||
/// Hard ceiling on <see cref="_warned"/>. Every key fed to <see cref="WarnOnce"/> is derived
|
||||
/// from authored configuration, so the set is bounded by construction — this is the belt to that
|
||||
/// braces. A key set that grows off <i>wire</i> input is a memory leak on the dispatcher path,
|
||||
/// and getting one of the derivations subtly wrong later must degrade to "stops warning" rather
|
||||
/// than to unbounded growth.
|
||||
/// </summary>
|
||||
private const int MaxWarnKeys = 1024;
|
||||
|
||||
/// <summary>
|
||||
/// How many consecutive <i>metadata</i> rebirths one edge node may be asked for before the
|
||||
/// driver gives up and says so. Bounds the rebirth loop the per-node debounce cannot break: the
|
||||
/// debounce caps the RATE, not the LIFETIME, so a node whose birth this driver can never use —
|
||||
/// one over <see cref="MaxPayloadBytes"/>, or one that simply never comes — would otherwise be
|
||||
/// commanded every debounce period for the life of the process, answering each time with the
|
||||
/// same unusable payload.
|
||||
/// </summary>
|
||||
private const int MaxConsecutiveMetadataRebirths = 3;
|
||||
|
||||
private readonly string _driverId;
|
||||
private readonly ILogger? _logger;
|
||||
private readonly TimeSpan _rebirthDebounce;
|
||||
@@ -125,6 +144,20 @@ public sealed class SparkplugIngestor
|
||||
/// <summary>Last rebirth-NCMD instant per edge node, in UTC ticks — the debounce floor.</summary>
|
||||
private readonly ConcurrentDictionary<SparkplugNodeKey, long> _lastRebirthTicks = new();
|
||||
|
||||
/// <summary>
|
||||
/// Edge nodes whose NBIRTH has been <i>seen</i> in the current session, whether or not its
|
||||
/// <c>seq</c> was readable. Distinct from <see cref="SequenceTracker.IsBirthSynchronized"/>,
|
||||
/// which additionally requires a usable <c>seq</c> — see <see cref="CheckSequence"/>.
|
||||
/// </summary>
|
||||
private readonly ConcurrentDictionary<SparkplugNodeKey, byte> _nodeBirthSeen = new();
|
||||
|
||||
/// <summary>
|
||||
/// Consecutive <i>metadata</i> rebirths requested from an edge node since its last applied
|
||||
/// birth. Capped by <see cref="MaxConsecutiveMetadataRebirths"/> — see
|
||||
/// <see cref="RequestMetadataRebirth"/>.
|
||||
/// </summary>
|
||||
private readonly ConcurrentDictionary<SparkplugNodeKey, int> _metadataRebirths = new();
|
||||
|
||||
/// <summary>Keys whose anomaly has already been logged loudly once, cleared by <see cref="Register"/>.</summary>
|
||||
private readonly ConcurrentDictionary<string, byte> _warned = new(StringComparer.Ordinal);
|
||||
|
||||
@@ -249,6 +282,20 @@ public sealed class SparkplugIngestor
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The <b>pre-3.0</b> primary-host STATE filter (<c>STATE/{hostId}</c>, no namespace prefix), or
|
||||
/// <see langword="null"/> when no host id is configured. Subscribed alongside
|
||||
/// <see cref="StateFilter"/> so the legacy form this driver already parses can actually arrive.
|
||||
/// </summary>
|
||||
public string? LegacyStateFilter
|
||||
{
|
||||
get
|
||||
{
|
||||
var hostId = _options.Sparkplug?.HostId;
|
||||
return string.IsNullOrWhiteSpace(hostId) ? null : $"STATE/{hostId}";
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Wires this ingestor onto a live connection: inbound messages route to
|
||||
/// <see cref="HandleMessage"/>, the connection becomes both transports, and every reconnect runs
|
||||
@@ -259,6 +306,11 @@ public sealed class SparkplugIngestor
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(connection);
|
||||
|
||||
// Binding to a session means the previous one is gone. Reset FIRST, before delivery is wired:
|
||||
// with a persistent session a broker may push queued messages the moment CONNACK lands, ahead
|
||||
// of our own SUBSCRIBE, and those must not resolve against the old session's aliases.
|
||||
ResetSessionState("attach to a new broker session");
|
||||
|
||||
_subscribeTransport = connection;
|
||||
_publishTransport = connection;
|
||||
connection.MessageReceived += HandleMessage;
|
||||
@@ -311,6 +363,32 @@ public sealed class SparkplugIngestor
|
||||
{
|
||||
_trackers.TryRemove(key, out _);
|
||||
_lastRebirthTicks.TryRemove(key, out _);
|
||||
_nodeBirthSeen.TryRemove(key, out _);
|
||||
_metadataRebirths.TryRemove(key, out _);
|
||||
}
|
||||
}
|
||||
|
||||
// …and their BIRTHS, which is the half that actually mis-routes. While an edge node is
|
||||
// unauthored its traffic is dropped at Dispatch, so its cached alias table stops refreshing —
|
||||
// it FREEZES rather than going stale-and-obvious. Deploy A authors Plant1/EdgeA/Filler1,
|
||||
// deploy B drops it, deploy C re-adds it a week later having missed every rebirth in between:
|
||||
// Births.Find still answers with the week-old table and the next DDATA publishes the wrong
|
||||
// metric's value under the right RawPath at Good quality. It is also why Births would otherwise
|
||||
// grow monotonically across redeploys.
|
||||
//
|
||||
// The predicate is edge-node departure ONLY, deliberately narrower than "absent from ByScope".
|
||||
// A device with no authored tags but whose EDGE NODE is still authored keeps receiving traffic
|
||||
// (the Dispatch filter is node-level, and OnDeviceBirth runs ahead of the authored-scope
|
||||
// filter), so its birth is live, not frozen — evicting it would throw away correct state and
|
||||
// cost a needless rebirth round-trip the moment a later deploy authored a tag on it.
|
||||
foreach (var scope in Births.Scopes)
|
||||
{
|
||||
var owner = new SparkplugNodeKey(scope.GroupId, scope.EdgeNodeId);
|
||||
if (!table.EdgeNodes.Contains(owner))
|
||||
{
|
||||
// Forgets the node scope and every device under it in one call; idempotent, so a node
|
||||
// contributing several scopes to this loop costs one eviction and some no-ops.
|
||||
Births.ForgetNode(owner.GroupId, owner.EdgeNodeId);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -359,6 +437,22 @@ public sealed class SparkplugIngestor
|
||||
/// <exception cref="InvalidOperationException">No group id is configured, or every filter was refused.</exception>
|
||||
public async Task EstablishAsync(CancellationToken cancellationToken)
|
||||
{
|
||||
// The SAME reset OnReconnectedAsync performs, and for the same reason — this ingestor OUTLIVES
|
||||
// a session rebuild. `MqttDriver.AdoptOptions` only replaces it when `!SameIngest`, but
|
||||
// `SessionIdentity` is a strict SUPERSET of `IngestIdentity`: changing Host, Port, ClientId,
|
||||
// credentials, TLS or a timeout alone yields `SameSession == false` AND `SameIngest == true`,
|
||||
// so `ReinitializeAsync` tears the session down and re-establishes on this very instance.
|
||||
// `TeardownAsync` touches no ingest state. The driver-host resilience layer re-running
|
||||
// `InitializeAsync` after a fault is the second path onto this line.
|
||||
//
|
||||
// Without the reset, an edge node that restarted during the changeover and rebound alias 5 from
|
||||
// Temperature to Pressure would have its first DDATA on the new session resolved against the
|
||||
// OLD table — publishing Pressure's value under the Temperature RawPath at Good quality. The
|
||||
// late-join rebirth below does not close that: it is dispatched off-thread (DATA can arrive
|
||||
// first), passes the debounce, and is a no-op under `requestRebirthOnGap: false` or against a
|
||||
// node that ignores the NCMD.
|
||||
ResetSessionState("establish a broker session");
|
||||
|
||||
await SubscribeGroupAsync(cancellationToken).ConfigureAwait(false);
|
||||
RequestLateJoinRebirths("initial connect");
|
||||
}
|
||||
@@ -457,13 +551,59 @@ public sealed class SparkplugIngestor
|
||||
/// <exception cref="InvalidOperationException">Every filter was refused — the caller tears the session down.</exception>
|
||||
public async Task OnReconnectedAsync(CancellationToken cancellationToken)
|
||||
{
|
||||
Births.Clear();
|
||||
_trackers.Clear();
|
||||
ResetSessionState("reconnect");
|
||||
|
||||
await SubscribeGroupAsync(cancellationToken).ConfigureAwait(false);
|
||||
RequestLateJoinRebirths("reconnect");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Drops every scrap of per-session ingest state: births, sequence baselines, the birth-seen
|
||||
/// set and the late-join counters. Called from <b>every</b> seam at which the session underneath
|
||||
/// this ingestor changes — <see cref="AttachTo"/>, <see cref="EstablishAsync"/> and
|
||||
/// <see cref="OnReconnectedAsync"/>.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// <b>This is the correctness step, not housekeeping</b> — and the reason it lives in one
|
||||
/// method called from three places rather than inline in one. Every piece of state cleared
|
||||
/// here is a claim about a session that no longer exists: an alias table binds names to
|
||||
/// numbers the <i>previous</i> edge-node session chose, and a sequence baseline is only
|
||||
/// evidence relative to the stream it was taken from. Carried across, an alias resolves to
|
||||
/// whichever metric used to own it (good quality, plausible value, wrong tag) and a fresh
|
||||
/// stream can look contiguous purely by coincidence.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Unconditionally safe.</b> On a freshly-constructed ingestor every collection is
|
||||
/// already empty, so calling it three times during one connect costs three no-ops.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Values are deliberately <b>not</b> staled here. A session change is a driver-side event,
|
||||
/// not a statement about the plant; last observed values remain the best available answer
|
||||
/// until a birth lands, and the connection's own health surface already reports the outage.
|
||||
/// Only a death — the source saying it is offline — stales a tag.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
/// <param name="reason">Why the reset is happening, for the diagnostic log.</param>
|
||||
private void ResetSessionState(string reason)
|
||||
{
|
||||
var births = Births.Count;
|
||||
Births.Clear();
|
||||
_trackers.Clear();
|
||||
_nodeBirthSeen.Clear();
|
||||
_metadataRebirths.Clear();
|
||||
|
||||
if (births > 0)
|
||||
{
|
||||
_logger?.LogInformation(
|
||||
"MQTT driver '{DriverId}': dropped {Count} Sparkplug birth scope(s) and every sequence "
|
||||
+ "baseline — {Reason}. Aliases will be rebuilt from the next birth certificate.",
|
||||
_driverId,
|
||||
births,
|
||||
reason);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Routes one inbound MQTT message: parses the topic, decodes the Sparkplug payload and hands it
|
||||
/// to <see cref="Dispatch"/>. Runs on MQTTnet's dispatcher thread and <b>never throws</b>.
|
||||
@@ -489,26 +629,68 @@ public sealed class SparkplugIngestor
|
||||
return;
|
||||
}
|
||||
|
||||
if (parsed.Type == SparkplugMessageType.STATE)
|
||||
{
|
||||
// A STATE body is a handful of bytes; an oversized one is a broken publisher, not a
|
||||
// dispatcher-thread hazard, and it is logged at Debug rather than through WarnOnce
|
||||
// because its key would be a host id this driver did not choose.
|
||||
if (payload.Length > MaxPayloadBytes)
|
||||
{
|
||||
_logger?.LogDebug(
|
||||
"MQTT driver '{DriverId}': STATE on '{Topic}' exceeded the {Max}-byte ceiling; dropped.",
|
||||
_driverId,
|
||||
topic,
|
||||
MaxPayloadBytes);
|
||||
return;
|
||||
}
|
||||
|
||||
HandleState(parsed, payload);
|
||||
return;
|
||||
}
|
||||
|
||||
// The authored-node filter runs HERE as well as inside Dispatch, and the order matters for
|
||||
// two reasons. It keeps the oversize warning's key bounded — keyed on the raw topic it grew
|
||||
// once per distinct topic off a group-wide '#' subscription, i.e. off wire input a plant (or
|
||||
// a hostile publisher) controls. And it drops an unauthored node's body before the protobuf
|
||||
// parse rather than after, which on a busy group is most of the decode work this driver
|
||||
// would otherwise do for traffic it discards.
|
||||
if (parsed.GroupId is not { } groupId || parsed.EdgeNodeId is not { } edgeNodeId)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
var node = new SparkplugNodeKey(groupId, edgeNodeId);
|
||||
if (!_authored.EdgeNodes.Contains(node))
|
||||
{
|
||||
_logger?.LogDebug(
|
||||
"MQTT driver '{DriverId}': {Type} for unauthored edge node '{Node}'; ignored.",
|
||||
_driverId,
|
||||
parsed.Type,
|
||||
node);
|
||||
return;
|
||||
}
|
||||
|
||||
if (payload.Length > MaxPayloadBytes)
|
||||
{
|
||||
// Refused BEFORE any decode: the whole point of the bound is that one publisher cannot
|
||||
// impose an unbounded protobuf parse on the shared dispatcher thread. Nothing mutates.
|
||||
//
|
||||
// An oversized NBIRTH is the worst case and is called out by name: the driver cannot
|
||||
// learn this node's aliases at all, so every subsequent DATA message is unroutable. The
|
||||
// late-join cap in CheckSequence is what stops that becoming a permanent NCMD loop.
|
||||
WarnOnce(
|
||||
$"oversize:{topic}",
|
||||
"MQTT driver '{DriverId}': '{Topic}' carried {Bytes} bytes, over the {Max}-byte ceiling; dropped.",
|
||||
$"oversize:{node}",
|
||||
"MQTT driver '{DriverId}': {Type} from '{Node}' carried {Bytes} bytes, over the "
|
||||
+ "{Max}-byte ceiling; dropped. Raise maxPayloadBytes if this node's birth is "
|
||||
+ "legitimately this large.",
|
||||
_driverId,
|
||||
topic,
|
||||
parsed.Type,
|
||||
node,
|
||||
payload.Length,
|
||||
MaxPayloadBytes);
|
||||
return;
|
||||
}
|
||||
|
||||
if (parsed.Type == SparkplugMessageType.STATE)
|
||||
{
|
||||
HandleState(parsed, payload);
|
||||
return;
|
||||
}
|
||||
|
||||
Dispatch(parsed, SparkplugCodec.Decode(payload));
|
||||
}
|
||||
catch (Exception ex)
|
||||
@@ -645,6 +827,12 @@ public sealed class SparkplugIngestor
|
||||
// AcceptNodeBirth, never Accept: an NBIRTH restarts the sequence by definition, so running it
|
||||
// through the gap detector would flag a gap on the very message that resynchronized everything
|
||||
// and answer a birth with a demand for another one.
|
||||
// Recorded BEFORE the seq is judged, and independently of the verdict: "a birth arrived" and
|
||||
// "the birth established a sequence baseline" are different facts, and conflating them is the
|
||||
// rebirth loop CheckSequence documents.
|
||||
_nodeBirthSeen[node] = 0;
|
||||
_metadataRebirths.TryRemove(node, out _);
|
||||
|
||||
var bdSeq = SequenceTracker.TryReadBdSeq(payload, out var token) ? token : (ulong?)null;
|
||||
if (!TrackerFor(node).AcceptNodeBirth(payload.Seq, bdSeq))
|
||||
{
|
||||
@@ -682,6 +870,10 @@ public sealed class SparkplugIngestor
|
||||
// A DBIRTH is a sequenced member of the edge node's stream — only the NBIRTH restarts it.
|
||||
CheckSequence(node, payload, "DBIRTH");
|
||||
|
||||
// Metadata arrived, so the give-up counter re-arms: a device that re-births after a bad patch
|
||||
// must not stay permanently un-askable because of requests made before it recovered.
|
||||
_metadataRebirths.TryRemove(node, out _);
|
||||
|
||||
var result = Births.ApplyDeviceBirth(node.GroupId, node.EdgeNodeId, deviceId, payload.Metrics);
|
||||
var scope = new SparkplugScope(node.GroupId, node.EdgeNodeId, deviceId);
|
||||
LogBirthAnomaly(scope.ToString(), result);
|
||||
@@ -725,7 +917,7 @@ public sealed class SparkplugIngestor
|
||||
+ "requesting a rebirth.",
|
||||
_driverId,
|
||||
scope);
|
||||
RequestRebirth(node, "data before birth");
|
||||
RequestMetadataRebirth(node, "data before birth");
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -754,7 +946,7 @@ public sealed class SparkplugIngestor
|
||||
+ "declare; requesting a rebirth.",
|
||||
_driverId,
|
||||
scope);
|
||||
RequestRebirth(node, "unknown alias");
|
||||
RequestMetadataRebirth(node, "unknown alias");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -765,12 +957,19 @@ public sealed class SparkplugIngestor
|
||||
/// </summary>
|
||||
private static SparkplugMetricBinding? ResolveBinding(AliasTable table, SparkplugMetric metric)
|
||||
{
|
||||
if (metric.Alias is { } alias && table.Resolve(alias) is { } byAlias)
|
||||
// NAME FIRST, deliberately. A DATA metric that carries a name has stated its identity in the
|
||||
// one vocabulary that survives a rebirth; the alias is a per-birth compression detail. When the
|
||||
// two disagree — a publisher that recycled an alias mid-stream, or a birth this driver applied
|
||||
// out of order — preferring the alias would resolve to whichever metric currently occupies that
|
||||
// slot, which is precisely the mis-route this type's binding rule exists to prevent. The common
|
||||
// case costs nothing: a real post-birth DATA metric carries no name at all, so this is a null
|
||||
// check before the alias lookup.
|
||||
if (metric.Name is { } name && table.ResolveByName(name) is { } byName)
|
||||
{
|
||||
return byAlias;
|
||||
return byName;
|
||||
}
|
||||
|
||||
return metric.Name is { } name ? table.ResolveByName(name) : null;
|
||||
return metric.Alias is { } alias ? table.Resolve(alias) : null;
|
||||
}
|
||||
|
||||
// ---- deaths ----
|
||||
@@ -878,7 +1077,15 @@ public sealed class SparkplugIngestor
|
||||
online ? "ONLINE" : "OFFLINE");
|
||||
}
|
||||
|
||||
HostStateObserved?.Invoke(this, new SparkplugHostStateEventArgs(state));
|
||||
try
|
||||
{
|
||||
HostStateObserved?.Invoke(this, new SparkplugHostStateEventArgs(state));
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
// Same containment as every other raise here — this runs on MQTTnet's dispatcher thread.
|
||||
_logger?.LogError(ex, "MQTT driver '{DriverId}': a host-state subscriber threw.", _driverId);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Reads the online flag out of a STATE body — v3.0 JSON first, then the legacy text form.</summary>
|
||||
@@ -976,6 +1183,25 @@ public sealed class SparkplugIngestor
|
||||
return; // A published metric nobody authored a tag for. Not an error — that is most of them.
|
||||
}
|
||||
|
||||
// Arrays slip past the unsupported-type gate below, because ToDriverDataType maps every
|
||||
// *Array variant to its ELEMENT type and is therefore never null for one. Design §3.5 defers
|
||||
// array support, and the failure is not benign: the value rides in bytes_value as a byte[] at
|
||||
// ValueKind.Scalar, so a numeric target throws its way to BadTypeMismatch (safe, wrong reason)
|
||||
// while a String/Reference target reaches Convert.ToBase64String and publishes a packed binary
|
||||
// buffer as a plausible string at GOOD quality. Gate on the array bit itself.
|
||||
if (binding.DataType.IsSparkplugArray())
|
||||
{
|
||||
WarnOnce(
|
||||
$"array-type:{key}",
|
||||
"MQTT driver '{DriverId}': metric '{Metric}' on '{Scope}' is a Sparkplug array "
|
||||
+ "({DataType}); array metrics are not supported in v1 and its tag(s) are not being fed.",
|
||||
_driverId,
|
||||
binding.Name,
|
||||
scope,
|
||||
binding.DataType);
|
||||
return;
|
||||
}
|
||||
|
||||
var birthType = binding.DataType.ToDriverDataType();
|
||||
if (birthType is null)
|
||||
{
|
||||
@@ -1187,17 +1413,76 @@ public sealed class SparkplugIngestor
|
||||
return;
|
||||
}
|
||||
|
||||
if (!tracker.IsBirthSynchronized)
|
||||
if (tracker.IsBirthSynchronized)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
// "Birth-synchronized" in SequenceTracker's sense means a birth established a usable SEQUENCE
|
||||
// BASELINE — it is false both when no NBIRTH has arrived (a genuine late join, which a rebirth
|
||||
// fixes) and when one arrived carrying a seq this driver could not read (which a rebirth does
|
||||
// NOT fix: the node answers with the same unusable birth). Reacting to the two identically is
|
||||
// what turns the second into a permanent NCMD loop at the debounce cadence, defeating the guard
|
||||
// OnNodeBirth deliberately applies one method over. Whether a birth ARRIVED is this type's own
|
||||
// fact to keep, so it keeps it.
|
||||
if (_nodeBirthSeen.ContainsKey(node))
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
WarnOnce(
|
||||
$"latejoin:{node}",
|
||||
"MQTT driver '{DriverId}': {Kind} from '{Node}' arrived with no NBIRTH seen since connect; "
|
||||
+ "requesting a rebirth.",
|
||||
_driverId,
|
||||
kind,
|
||||
node);
|
||||
|
||||
// The residual loop the birth-seen flag cannot see: an NBIRTH that never reaches Dispatch at
|
||||
// all, because it exceeded MaxPayloadBytes or because the node never sends one. Nothing about
|
||||
// asking again changes that, so it goes through the capped path.
|
||||
RequestMetadataRebirth(node, "late join (no NBIRTH observed)");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Requests a rebirth for a <b>missing-metadata</b> reason — late join, data before birth, or an
|
||||
/// alias the current birth does not declare — under a per-node cap on consecutive attempts.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// <b>Why these three and not a sequence gap.</b> A gap is self-limiting: the tracker
|
||||
/// resynchronizes onto the observed value, so the very next message is contiguous and asks
|
||||
/// for nothing. The three reasons routed here are not — every one of them means "this
|
||||
/// driver holds no usable catalog for this node", and if the node cannot supply one (its
|
||||
/// birth is over the payload ceiling, or it ignores NCMD entirely) then <i>every</i>
|
||||
/// subsequent message re-raises the same condition. The debounce caps the rate at one
|
||||
/// request per period; only this caps the lifetime.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Consecutive, not cumulative.</b> Any applied birth clears the counter, so a node that
|
||||
/// recovers is not permanently penalised for having once been unreachable.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
/// <param name="node">The edge node to command.</param>
|
||||
/// <param name="reason">Why, for the log.</param>
|
||||
private void RequestMetadataRebirth(SparkplugNodeKey node, string reason)
|
||||
{
|
||||
var attempts = _metadataRebirths.AddOrUpdate(node, 1, static (_, n) => n + 1);
|
||||
if (attempts > MaxConsecutiveMetadataRebirths)
|
||||
{
|
||||
WarnOnce(
|
||||
$"latejoin:{node}",
|
||||
"MQTT driver '{DriverId}': {Kind} from '{Node}' arrived with no NBIRTH seen since connect; "
|
||||
+ "requesting a rebirth.",
|
||||
$"metadata-exhausted:{node}",
|
||||
"MQTT driver '{DriverId}': '{Node}' has not delivered a usable birth certificate after "
|
||||
+ "{Attempts} rebirth request(s); giving up until one arrives on its own. Its tags will "
|
||||
+ "not update. Check the node's birth payload size against maxPayloadBytes, its seq "
|
||||
+ "field, and that it honours NCMD Node Control/Rebirth.",
|
||||
_driverId,
|
||||
kind,
|
||||
node);
|
||||
RequestRebirth(node, "late join (no NBIRTH observed)");
|
||||
node,
|
||||
MaxConsecutiveMetadataRebirths);
|
||||
return;
|
||||
}
|
||||
|
||||
RequestRebirth(node, reason);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -1359,6 +1644,15 @@ public sealed class SparkplugIngestor
|
||||
if (StateFilter is { } stateFilter)
|
||||
{
|
||||
filters.Add(new MqttTopicSubscription(stateFilter, Qos: 1, SeedRetained: true));
|
||||
|
||||
// The pre-3.0 form, which HandleState already parses. Subscribing only the v3.0 topic made
|
||||
// that tolerance unreachable in production — the legacy branch could be exercised by a test
|
||||
// calling HandleMessage directly and by nothing else. Design §3.1 is explicit: emit v3.0,
|
||||
// tolerate legacy on receive, and tolerating requires asking for it.
|
||||
filters.Add(new MqttTopicSubscription(
|
||||
LegacyStateFilter!,
|
||||
Qos: 1,
|
||||
SeedRetained: true));
|
||||
}
|
||||
|
||||
var transport = _subscribeTransport;
|
||||
@@ -1427,7 +1721,10 @@ public sealed class SparkplugIngestor
|
||||
/// </summary>
|
||||
private void WarnOnce(string key, string message, params object?[] args)
|
||||
{
|
||||
if (_warned.TryAdd(key, 0))
|
||||
// The ceiling is unreachable by design — see MaxWarnKeys. It exists so that a derivation that
|
||||
// turns out NOT to be bounded (as the oversize path's raw-topic key was) degrades to silence
|
||||
// instead of to unbounded growth on the dispatcher path.
|
||||
if (_warned.Count < MaxWarnKeys && _warned.TryAdd(key, 0))
|
||||
{
|
||||
#pragma warning disable CA2254 // Template is a compile-time constant at every call site.
|
||||
_logger?.LogWarning(message, args);
|
||||
@@ -1643,7 +1940,18 @@ internal static class SparkplugValueCoercion
|
||||
&& Assign(pf, out value);
|
||||
}
|
||||
|
||||
value = Convert.ToSingle(raw, CultureInfo.InvariantCulture);
|
||||
var asSingle = Convert.ToSingle(raw, CultureInfo.InvariantCulture);
|
||||
|
||||
// Convert.ToSingle SATURATES rather than throwing: a Double of 1e300 onto a Float32
|
||||
// tag becomes +Infinity, which would publish at GOOD quality as a number no gauge
|
||||
// can render and no comparison behaves sensibly against. A source that was itself
|
||||
// infinite is passed through — that is the publisher's own value, not our overflow.
|
||||
if (!float.IsFinite(asSingle) && IsFiniteSource(raw))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
value = asSingle;
|
||||
return true;
|
||||
|
||||
case DriverDataType.Float64:
|
||||
@@ -1653,7 +1961,13 @@ internal static class SparkplugValueCoercion
|
||||
&& Assign(pd, out value);
|
||||
}
|
||||
|
||||
value = Convert.ToDouble(raw, CultureInfo.InvariantCulture);
|
||||
var asDouble = Convert.ToDouble(raw, CultureInfo.InvariantCulture);
|
||||
if (!double.IsFinite(asDouble) && IsFiniteSource(raw))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
value = asDouble;
|
||||
return true;
|
||||
|
||||
case DriverDataType.DateTime:
|
||||
@@ -1679,6 +1993,17 @@ internal static class SparkplugValueCoercion
|
||||
return true;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Whether the wire value was itself a finite number — i.e. whether a non-finite result is
|
||||
/// <i>our</i> narrowing overflow rather than the publisher's own Infinity/NaN.
|
||||
/// </summary>
|
||||
private static bool IsFiniteSource(object raw) => raw switch
|
||||
{
|
||||
float f => float.IsFinite(f),
|
||||
double d => double.IsFinite(d),
|
||||
_ => true,
|
||||
};
|
||||
|
||||
private static bool TryCoerceBoolean(object raw, out object? value)
|
||||
{
|
||||
switch (raw)
|
||||
|
||||
Reference in New Issue
Block a user