fix(mtconnect): an unanswered write degrades; one success writer for reads and writes (review I-3, m7)
Coordinator ruling on F3: a write with no reply (transport failure / deadline) sets Degraded with the stamp preserved and a type-only LastError, through a new _writeDegraded flag folded into PublishHealthy. One success writer is shared: RecordReadAnswered (a /current answer, an Agent refusal, a live-index read) clears the read AND write degradations; RecordWriteAnswered clears the write one; neither clears the other path's stream degradation. Tests both directions (answered write clears it, good read clears it) plus the converse (an answered write does not clear a failed read); red with each half removed. m7: a good /current resets the refusal-dedup latch, so a refusal recurring after a recovery is logged again (tested). Refs #743 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
@@ -429,8 +429,12 @@ different `level|text` wire format, and are out of scope for v1.
|
||||
`DriverHealth.LastError`, which is type-only by repo-wide discipline. A refused write is not
|
||||
evidence the read path degraded, but it **is** the Agent answering: any answer — accepted or
|
||||
refused — advances `LastSuccessfulRead`, with the state following the read / `/sample` flags
|
||||
(a write cannot clear either path's degradation). A write that got **no** answer leaves health
|
||||
to those two paths, which own liveness.
|
||||
(a write cannot clear either of those paths' degradation). A write that got **no** answer
|
||||
(transport failure, deadline) sets **Degraded** with the stamp preserved and a type-only
|
||||
`LastError` (review 2026-10-21 I-3, coordinator ruling). One success writer is shared: that
|
||||
degradation is cleared by the next answered **write** or the next good **read** (a `/current`
|
||||
answer or a live-index read), so it cannot stick on a driver still talking to the Agent; a
|
||||
`/sample` chunk alone does not clear it.
|
||||
|
||||
**One request per device (review 2026-10-21 T-6).** The slots that survive the refusals above
|
||||
go out as one `POST {agent}/{device}?id1=v1&id2=v2…` per device (the Agent's PUT surface takes
|
||||
|
||||
@@ -369,6 +369,13 @@ public sealed partial class MTConnectDriver
|
||||
/// <summary>Whether the <c>/current</c> read path is currently failing. See <see cref="PublishHealthy"/>.</summary>
|
||||
private volatile bool _readDegraded;
|
||||
|
||||
/// <summary>
|
||||
/// Whether the last Agent write got no reply (review I-3 / coordinator ruling: an unanswered write degrades).
|
||||
/// Cleared by the next answered read OR write (<see cref="RecordReadAnswered"/> / <see cref="RecordWriteAnswered"/>),
|
||||
/// so it cannot latch on a driver that is still talking to the Agent.
|
||||
/// </summary>
|
||||
private volatile bool _writeDegraded;
|
||||
|
||||
/// <summary>The last <c>/current</c> refusal text logged, so a persistent refusal warns once, not per read.</summary>
|
||||
private string? _lastReadRefusal;
|
||||
|
||||
@@ -761,8 +768,9 @@ public sealed partial class MTConnectDriver
|
||||
results[i] = snapshot.Get(binding.ToDataItemId(fullReferences[i]));
|
||||
}
|
||||
|
||||
_readDegraded = false;
|
||||
PublishHealthy(DateTime.UtcNow);
|
||||
// m7: the next refusal after a recovery is news again.
|
||||
Volatile.Write(ref _lastReadRefusal, null);
|
||||
RecordReadAnswered(DateTime.UtcNow);
|
||||
|
||||
return results;
|
||||
}
|
||||
@@ -779,8 +787,7 @@ public sealed partial class MTConnectDriver
|
||||
// scope, an unsupported request) — it answered. The batch is Bad, but this is not a comms fault: the
|
||||
// read path is not degraded by it (that would reconnect-loop a driver over a config problem), the stamp
|
||||
// advances, and the Agent's text is logged once per distinct refusal.
|
||||
_readDegraded = false;
|
||||
PublishHealthy(DateTime.UtcNow);
|
||||
RecordReadAnswered(DateTime.UtcNow);
|
||||
if (!string.Equals(Interlocked.Exchange(ref _lastReadRefusal, ex.Message), ex.Message, StringComparison.Ordinal))
|
||||
{
|
||||
_logger.LogWarning(
|
||||
@@ -830,10 +837,9 @@ public sealed partial class MTConnectDriver
|
||||
|
||||
// The read path is answering (from a stream that is delivering), so a degradation a failed /current left
|
||||
// behind is no longer true of it. The stamp stays the pump's — this read heard nothing from the Agent itself.
|
||||
if (_readDegraded)
|
||||
if (_readDegraded || _writeDegraded)
|
||||
{
|
||||
_readDegraded = false;
|
||||
PublishHealthy(ReadHealth().LastSuccessfulRead ?? DateTime.UtcNow);
|
||||
RecordReadAnswered(ReadHealth().LastSuccessfulRead ?? DateTime.UtcNow);
|
||||
}
|
||||
|
||||
return results;
|
||||
@@ -868,7 +874,7 @@ public sealed partial class MTConnectDriver
|
||||
// nowhere near LastError, which is type-only by repo-wide discipline. A refused write is
|
||||
// not evidence the read path degraded — but it IS the Agent answering, so any answer
|
||||
// (accepted or refused) advances LastSuccessfulRead with the state following the read /
|
||||
// stream flags; a write that got no answer leaves health to those paths, which own liveness.
|
||||
// stream flags; a write that got no answer degrades (stamp preserved) until the next answered read or write.
|
||||
// Slots that survive the local refusals go out as ONE request per device (R06:T-6).
|
||||
// is null — a caller bug.
|
||||
public async Task<IReadOnlyList<WriteResult>> WriteAsync(
|
||||
@@ -1017,8 +1023,8 @@ public sealed partial class MTConnectDriver
|
||||
catch (Exception ex)
|
||||
{
|
||||
// A transport failure, a blown per-call deadline, or a disposed client — no answer arrived. Logged with
|
||||
// the exception (its TYPE is the diagnosis) and reported per slot; health is left to the read and
|
||||
// stream paths, which own liveness.
|
||||
// the exception (its TYPE is the diagnosis), reported per slot, and Degraded with the stamp preserved
|
||||
// (RecordWriteNoReply) until the next answered read or write.
|
||||
_logger.LogWarning(
|
||||
ex,
|
||||
"MTConnect driver {DriverInstanceId} could not write {Count} DataItem(s) ({DataItemIds}) on device '{DeviceName}' to {AgentUri}.",
|
||||
@@ -1029,6 +1035,8 @@ public sealed partial class MTConnectDriver
|
||||
results[write.Slot] = new WriteResult(BadCommunicationError);
|
||||
}
|
||||
|
||||
RecordWriteNoReply(options, ex);
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -1063,18 +1071,42 @@ public sealed partial class MTConnectDriver
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A write got an answer from the Agent: <see cref="DriverHealth.LastSuccessfulRead"/> advances, and the state
|
||||
/// follows the read / stream flags exactly as their own successes do (<see cref="PublishHealthy"/> — a write
|
||||
/// cannot clear either path's degradation, but it is evidence the Agent is reachable).
|
||||
/// A write got an answer from the Agent: <see cref="DriverHealth.LastSuccessfulRead"/> advances, a prior
|
||||
/// unanswered-write degradation clears, and the state follows the read / stream flags (a write cannot clear
|
||||
/// either of THOSE paths' degradation, but it is evidence the Agent is reachable).
|
||||
/// </summary>
|
||||
private void RecordWriteAnswered()
|
||||
{
|
||||
_writeDegraded = false;
|
||||
if (ReadHealth().State is DriverState.Healthy or DriverState.Degraded)
|
||||
{
|
||||
PublishHealthy(DateTime.UtcNow);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A write got NO answer (transport failure, deadline): Degraded with the stamp preserved — the health contract
|
||||
/// for a comms fault. Type-only text. Cleared by the next answered read or write.
|
||||
/// </summary>
|
||||
private void RecordWriteNoReply(MTConnectDriverOptions options, Exception ex)
|
||||
{
|
||||
_writeDegraded = true;
|
||||
Degrade($"MTConnect write to {AgentDisplay(options)} got no reply ({ex.GetType().Name}).");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The ONE success writer the read path and the write path share (review I-3): an answered read clears the
|
||||
/// read AND write degradations (the Agent is answering requests), then publishes Healthy unless the
|
||||
/// <c>/sample</c> stream is still failing.
|
||||
/// </summary>
|
||||
/// <param name="at">The stamp to publish.</param>
|
||||
private void RecordReadAnswered(DateTime at)
|
||||
{
|
||||
_readDegraded = false;
|
||||
_writeDegraded = false;
|
||||
PublishHealthy(at);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Renders a value to the invariant string a PUT query carries, or reports that it has no such
|
||||
/// form.
|
||||
@@ -3162,6 +3194,7 @@ public sealed partial class MTConnectDriver
|
||||
// re-initialize is precisely the event that means an operator changed something.
|
||||
_streamDegraded = false;
|
||||
_readDegraded = false;
|
||||
_writeDegraded = false;
|
||||
|
||||
WriteHealth(new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null));
|
||||
|
||||
@@ -3619,7 +3652,7 @@ public sealed partial class MTConnectDriver
|
||||
{
|
||||
var current = ReadHealth();
|
||||
|
||||
if (_streamDegraded || _readDegraded)
|
||||
if (_streamDegraded || _readDegraded || _writeDegraded)
|
||||
{
|
||||
if (current.State != DriverState.Faulted)
|
||||
{
|
||||
|
||||
@@ -104,4 +104,65 @@ public sealed class MTConnectBatchedWriteTests
|
||||
driver.GetHealth().LastSuccessfulRead.ShouldNotBeNull().ShouldBeGreaterThan(before);
|
||||
driver.GetHealth().State.ShouldBe(DriverState.Healthy);
|
||||
}
|
||||
|
||||
/// <summary>Review I-3 (coordinator ruling): a write that got NO reply degrades health, with the stamp preserved.</summary>
|
||||
[Fact]
|
||||
public async Task A_write_with_no_reply_degrades_and_preserves_the_stamp()
|
||||
{
|
||||
var (driver, client) = await WritableAsync();
|
||||
var before = driver.GetHealth();
|
||||
before.State.ShouldBe(DriverState.Healthy, "positive control: Healthy before the failed write");
|
||||
client.WriteFailure = new HttpRequestException("connection refused");
|
||||
|
||||
var results = await driver.WriteAsync([new WriteRequest(PosRaw, 1.5)], Ct);
|
||||
|
||||
results.Single().StatusCode.ShouldBe(BadCommunicationError);
|
||||
var h = driver.GetHealth();
|
||||
h.State.ShouldBe(DriverState.Degraded);
|
||||
h.LastSuccessfulRead.ShouldBe(before.LastSuccessfulRead);
|
||||
h.LastError.ShouldNotBeNull().ShouldContain(nameof(HttpRequestException));
|
||||
}
|
||||
|
||||
/// <summary>The unanswered-write degradation is cleared by the next ANSWERED write (it cannot latch).</summary>
|
||||
[Fact]
|
||||
public async Task An_answered_write_clears_an_unanswered_write_degradation()
|
||||
{
|
||||
var (driver, client) = await WritableAsync();
|
||||
client.WriteFailure = new HttpRequestException("connection refused");
|
||||
await driver.WriteAsync([new WriteRequest(PosRaw, 1.5)], Ct);
|
||||
driver.GetHealth().State.ShouldBe(DriverState.Degraded);
|
||||
|
||||
client.WriteFailure = null;
|
||||
await driver.WriteAsync([new WriteRequest(PosRaw, 2.5)], Ct);
|
||||
|
||||
driver.GetHealth().State.ShouldBe(DriverState.Healthy);
|
||||
}
|
||||
|
||||
/// <summary>…and by the next good READ (the other direction of the shared health writer).</summary>
|
||||
[Fact]
|
||||
public async Task A_good_read_clears_an_unanswered_write_degradation()
|
||||
{
|
||||
var (driver, client) = await WritableAsync();
|
||||
client.WriteFailure = new HttpRequestException("connection refused");
|
||||
await driver.WriteAsync([new WriteRequest(PosRaw, 1.5)], Ct);
|
||||
driver.GetHealth().State.ShouldBe(DriverState.Degraded);
|
||||
|
||||
(await driver.ReadAsync([PosRaw], Ct)).Single().StatusCode.ShouldBe(Good);
|
||||
|
||||
driver.GetHealth().State.ShouldBe(DriverState.Healthy);
|
||||
}
|
||||
|
||||
/// <summary>The converse: a failed READ is not cleared by an answered write (each path clears only what it proves).</summary>
|
||||
[Fact]
|
||||
public async Task An_answered_write_does_not_clear_a_failed_read()
|
||||
{
|
||||
var (driver, client) = await WritableAsync();
|
||||
client.CurrentFailure = new HttpRequestException("connection refused");
|
||||
await driver.ReadAsync([PosRaw], Ct);
|
||||
driver.GetHealth().State.ShouldBe(DriverState.Degraded);
|
||||
|
||||
await driver.WriteAsync([new WriteRequest(PosRaw, 2.5)], Ct);
|
||||
|
||||
driver.GetHealth().State.ShouldBe(DriverState.Degraded);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -228,4 +228,34 @@ public sealed class MTConnectReadPathTests
|
||||
[Fact]
|
||||
public void A_garbage_answer_is_not_an_agent_refusal() =>
|
||||
MTConnectXml.IsAgentRefusal(ParseFailure("<html><body>proxy error</body></html>")).ShouldBeFalse();
|
||||
|
||||
/// <summary>Review m7: a refusal that recurs AFTER a good read is news again and is logged again.</summary>
|
||||
[Fact]
|
||||
public async Task A_refusal_recurring_after_a_good_read_is_logged_again()
|
||||
{
|
||||
var logger = new RecordingDriverLogger();
|
||||
var client = CannedAgentClient.FromFixtures();
|
||||
var driver = new MTConnectDriver(Opts(), "mt-refusal-again", _ => client, logger);
|
||||
await driver.InitializeAsync("{}", Ct);
|
||||
try
|
||||
{
|
||||
var refusal = ParseFailure(NoDeviceError);
|
||||
client.CurrentFailure = refusal;
|
||||
await driver.ReadAsync(["dev1_pos"], Ct);
|
||||
await driver.ReadAsync(["dev1_pos"], Ct);
|
||||
logger.WarningsSnapshot().Count(w => w.Contains("NO_DEVICE")).ShouldBe(1, "positive control: deduplicated while it persists");
|
||||
|
||||
client.CurrentFailure = null;
|
||||
await driver.ReadAsync(["dev1_pos"], Ct);
|
||||
client.CurrentFailure = refusal;
|
||||
await driver.ReadAsync(["dev1_pos"], Ct);
|
||||
|
||||
logger.WarningsSnapshot().Count(w => w.Contains("NO_DEVICE")).ShouldBe(2);
|
||||
}
|
||||
finally
|
||||
{
|
||||
client.CurrentFailure = null;
|
||||
await driver.ShutdownAsync(CancellationToken.None);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user