fix(mtconnect): read-path restart publishes the new cursor; document the review trade-offs (review m6, m1, m4, m5, m8)

- m6: a /current read that sees a new Agent instanceId now also publishes the new
  process's cursor (generation-checked, captured with the read's session), so a
  later subscription does not open at the dead instance's sequence and flicker
  through a clear + re-baseline. Pinned in the T-2 test.
- Documented (code + docs/drivers/MTConnect.md): m1 the bounded stale-read window of
  a stream stalled without failing; m4 a refused multi-pair PUT's retry may re-send
  an already-applied pair (harmless); m5 a non-Agent 200 now reads reachable to the
  ping; m8 a refused raised count takes one transient blip before the step-back.
- m10 confirmed: no host key is persisted (DriverHostStatus dropped, #521) - the
  redacted HostName only changes an in-memory /hosts key for a query-bearing URI.

Refs #743

Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
Joseph Doherty
2026-10-22 16:33:14 -04:00
parent b2c064bec3
commit e03f343f14
4 changed files with 44 additions and 7 deletions
@@ -83,6 +83,11 @@ public interface IMTConnectAgentClient : IDisposable
/// <see cref="ProbeAsync"/> would for an unreachable or refusing Agent. The default delegates to
/// <see cref="ProbeAsync"/> and discards the model.
/// </summary>
/// <remarks>
/// Trade-off (review m5): a success status from something that is NOT an Agent — a proxy serving an HTML page
/// under 200 — now reads as reachable, where <see cref="ProbeAsync"/> would have failed to parse it. The data
/// paths still report that misconfiguration (the read / stream fail to parse).
/// </remarks>
/// <param name="ct">Cancellation token (the caller's deadline).</param>
/// <returns>A task completing when the Agent has answered with a success status.</returns>
async Task PingAsync(CancellationToken ct) => _ = await ProbeAsync(ct).ConfigureAwait(false);
@@ -714,7 +714,13 @@ public sealed partial class MTConnectDriver
// One read of the session reference: the client used below is the one that was live at this
// instant, never a half-swapped pairing (see the _session remarks).
var client = Volatile.Read(ref _session)?.Client;
IMTConnectAgentClient? client;
long readGeneration;
lock (_subscriptionLock)
{
client = Volatile.Read(ref _session)?.Client;
readGeneration = _sessionGeneration;
}
var options = Volatile.Read(ref _options);
// Read once, for the same reason the session is: every reference in this batch must be
@@ -749,9 +755,14 @@ public sealed partial class MTConnectDriver
// rediscovery. Only while the session this read used is still installed: a read that raced a
// re-initialise against a different Agent would otherwise announce a "restart" the operator caused.
if (current.InstanceId != Interlocked.Read(ref _announcedInstanceId)
&& ReferenceEquals(Volatile.Read(ref _session)?.Client, client))
&& IsCurrentSessionGeneration(readGeneration))
{
AnnounceAgentRestart(current.InstanceId, options);
// Review m6: the session's cursor still named the dead instance, so the next pump would open there and
// spend its first chunk on a clear + re-baseline (a Bad flicker). Move it to the new process — through
// the same generation-checked publish the pump uses, so it can only land on the session this read used.
PublishAgentCursor(readGeneration, current.InstanceId, current.NextSequence);
}
// Built from the SAME authored tags as the shared index, so the authored-vs-unknown
@@ -810,6 +821,13 @@ public sealed partial class MTConnectDriver
/// trusted as the Agent's current state: no pump, a pump bound to a retired session, one that has not yet
/// delivered, or a degraded stream. Touches no health — the pump owns the stream's.
/// </summary>
/// <remarks>
/// <b>Stale window (review m1).</b> A stream that stalls WITHOUT failing (no chunk, no heartbeat) keeps
/// <c>_streamDegraded</c> clear until the client's idle watchdog fires (2 × <c>HeartbeatMs</c> +
/// <c>RequestTimeoutMs</c>); for that bounded window reads answer Good from an index nothing is updating, where a
/// <c>/current</c> would have noticed. Subscriptions have the identical exposure; the window is the same one
/// the watchdog already accepts for the data plane.
/// </remarks>
/// <param name="fullReferences">The batch.</param>
/// <param name="binding">The tag table the batch is resolved against.</param>
/// <returns>One snapshot per reference in order, or <see langword="null"/> to fall back to <c>/current</c>.</returns>
@@ -1063,7 +1081,10 @@ public sealed partial class MTConnectDriver
return;
}
// The Agent refused the request as a whole: find out which slot(s) it objects to, one by one.
// The Agent refused the request as a whole: find out which slot(s) it objects to, one by one. (Review m4:
// an Agent may have applied some pairs before refusing — cppagent collects put errors per key — so the retry
// can re-send a pair that already landed. Harmless for value-setting: the Agent's duplicate filter drops an
// identical repeat.)
foreach (var write in batch)
{
await WriteBatchAsync(client, options, [write], results, cancellationToken).ConfigureAwait(false);