fix(driver.sql): advance LastSuccessfulRead on evidence the database answered (Driver.Sql-006)

2026-08-10 08:02:49 -04:00
parent ee1f1b28e8
commit a0a49d3395
2 changed files with 136 additions and 4 deletions
@@ -595,8 +595,11 @@ public sealed class SqlDriver
    /// <summary>
    ///     Classifies one poll's snapshots into a health verdict and a host state.
    ///     <list type="bullet">
-    ///         <item>Any Good snapshot ⇒ the database answered: <c>LastSuccessfulRead</c> advances and the
-    ///         host is Running.</item>
+    ///         <item>Any Good snapshot ⇒ Healthy and the host is Running.</item>
+    ///         <item>Any snapshot that is <b>evidence the database answered</b>
+    ///         (<see cref="IsAnswerEvidence"/> — Good, but also an absent row, a NULL cell, an uncoercible
+    ///         cell) ⇒ <c>LastSuccessfulRead</c> advances and the host is Running, <em>without</em> changing
+    ///         the health state. That clock is what the comms-loss watchdog reads (Driver.Sql-006).</item>
    ///         <item>Any <b>connection-class</b> Bad snapshot (<see cref="SqlStatusCodes.BadTimeout"/>) ⇒
    ///         Degraded; when nothing at all answered, the host is Stopped.</item>
    ///         <item>Any <b>query-failure</b> Bad snapshot (<see cref="SqlStatusCodes.BadCommunicationError"/>)
@@ -623,18 +626,21 @@ public sealed class SqlDriver
        var good = 0;
        var unreachable = 0;
        var queryFailed = 0;
+        var answered = 0;
        foreach (var snapshot in snapshots)
        {
            if (SqlStatusCodes.IsGood(snapshot.StatusCode)) good++;
            else if (IsConnectionClass(snapshot.StatusCode)) unreachable++;
            else if (IsQueryFailureClass(snapshot.StatusCode)) queryFailed++;
+
+            if (IsAnswerEvidence(snapshot.StatusCode)) answered++;
        }

        if (unreachable > 0)
        {
            Degrade(
                $"{unreachable} of {snapshots.Count} Sql tag(s) timed out against {Endpoint} on the last poll.");
-            if (good > 0 || queryFailed > 0)
+            if (answered > 0)
            {
                // Partial: something answered, so the endpoint is up — but the driver is not healthy.
                TouchLastSuccessfulRead();
@@ -663,7 +669,18 @@ public sealed class SqlDriver
            return;
        }

-        if (good == 0) return;   // authoring-class Bad only — not a statement about the database.
+        if (answered == 0) return;   // authoring-class Bad only — not a statement about the database.
+
+        if (good == 0)
+        {
+            // The database answered, just not with a value: an absent row, a NULL cell, a cell that would
+            // not coerce. Those are data/authoring facts, so the health STATE is deliberately left exactly
+            // as it was — but they are still proof the transport works, and the evidence clock the
+            // comms-loss watchdog reads must say so (Driver.Sql-006).
+            TouchLastSuccessfulRead();
+            TransitionHostTo(HostState.Running);
+            return;
+        }

        // Route through the Faulted guard, not a raw WriteHealth: a poll still in flight when a concurrent
        // ReinitializeAsync has recorded Faulted (the engine's ~5 s teardown wait is shorter than the 15 s
@@ -696,6 +713,34 @@ public sealed class SqlDriver
    private static bool IsQueryFailureClass(uint statusCode)
        => statusCode is SqlStatusCodes.BadCommunicationError;

+    /// <summary>
+    ///     True for the status codes that are proof the database <b>answered</b> this poll — the evidence
+    ///     <c>LastSuccessfulRead</c> records, which is not the same question as "is this value good".
+    /// </summary>
+    /// <remarks>
+    ///     <para>Driver.Sql-006. <c>DriverInstanceActor</c>'s comms-loss watchdog force-reconnects a driver
+    ///     that is non-Healthy <b>and</b> whose <c>LastSuccessfulRead</c> has been frozen for 90 s (Gitea
+    ///     #527), on the stated premise that the clock is the honest evidence the transport works. Advancing
+    ///     it on Good alone made a truncated staging table or a deleted key set look — to that watchdog —
+    ///     exactly like a dead socket, so a database answering every poll on time got its connection torn
+    ///     down and rebuilt every 90 s, forever.</para>
+    ///     <para>What is deliberately excluded is as load-bearing as what is included:
+    ///     <see cref="SqlStatusCodes.BadTimeout"/> is the database <em>not</em> answering, and
+    ///     <see cref="SqlStatusCodes.BadNodeIdUnknown"/> / <see cref="SqlStatusCodes.BadConfigurationError"/>
+    ///     / <see cref="SqlStatusCodes.BadInternalError"/> are minted before any statement is executed. A
+    ///     blanket "anything that is not a timeout" would keep the clock fresh on a driver that never once
+    ///     reached the database, which disables the watchdog rather than fixing it.</para>
+    /// </remarks>
+    /// <param name="statusCode">The status code to classify.</param>
+    /// <returns>True when the code could only have been produced by a database that answered.</returns>
+    private static bool IsAnswerEvidence(uint statusCode) => statusCode is
+        SqlStatusCodes.Good                    // the row is there and readable
+        or SqlStatusCodes.BadNoData            // the query ran; that row is absent
+        or SqlStatusCodes.Uncertain            // a present row with a NULL cell (nullIsBad=false)
+        or SqlStatusCodes.Bad                  // …the same row under nullIsBad=true
+        or SqlStatusCodes.BadTypeMismatch      // a cell came back and would not coerce
+        or SqlStatusCodes.BadCommunicationError;  // the connection opened; the server refused the query
+
    /// <summary>
    ///     Routes a poll-loop reader failure to the health surface (05/STAB-9). Deliberately does <b>not</b>
    ///     touch host state: the engine also reports its own contract violations here, which say nothing