test(historian): bounded-wait durability read; honest nondeterminism notes on the #491 controls

2026-08-17 04:07:08 -04:00
parent 20022083df
commit 6b46c485ef
2 changed files with 56 additions and 1 deletions
@@ -1,3 +1,4 @@
+using System.Diagnostics;
 using System.Linq;
 using FASTER.core;
 using Xunit;
@@ -188,7 +189,10 @@ public sealed class FasterLogHistorizationOutboxTests : IDisposable
        {
            await writer.AppendBatchAsync([a, b, c], TestContext.Current.CancellationToken);

-            Assert.Equal(new[] { a.Id, b.Id, c.Id }, ReadCommitted(dir));
+            Guid[] committed = await ReadCommittedWhenVisibleAsync(
+                dir, expected: 3, TestContext.Current.CancellationToken);
+
+            Assert.Equal(new[] { a.Id, b.Id, c.Id }, committed);
        }
        finally
        {
@@ -196,6 +200,36 @@ public sealed class FasterLogHistorizationOutboxTests : IDisposable
        }
    }

+    // Bounded poll over ReadCommitted for the group-commit pin above. AppendBatchAsync's commit has
+    // already returned by the time we look, but an INDEPENDENT FasterLog learns about it only by
+    // re-reading the on-disk commit metadata — and how promptly that write becomes visible to a
+    // second reader is a filesystem property, not a FasterLog guarantee (recorded as fragile on
+    // macOS, where it is not on the Windows/Linux timescale). A single-shot read is therefore
+    // timing-dependent and green partly by luck. Each attempt calls ReadCommitted, which builds a
+    // FRESH device + FasterLog and so re-reads the metadata rather than reusing a stale view —
+    // verified: a reader opened after a later commit does observe that commit.
+    //
+    // The bounded wait does NOT weaken the claim: the writer is never disposed during the poll (the
+    // only other thing that could commit), and PerEntry mode runs no periodic-commit loop — so
+    // anything that becomes visible inside the window was still made durable by AppendBatchAsync's
+    // own group commit and nothing else. A missing commit stays missing for the whole 5 s and the
+    // test still goes red.
+    //
+    // On timeout the last read is returned rather than thrown on, so the caller's Assert.Equal
+    // reports the actual shortfall (which ids were durable) instead of a bare timeout message.
+    private static async Task<Guid[]> ReadCommittedWhenVisibleAsync(string dir, int expected, CancellationToken ct)
+    {
+        var elapsed = Stopwatch.StartNew();
+        Guid[] ids = ReadCommitted(dir);
+        while (ids.Length < expected && elapsed.Elapsed < TimeSpan.FromSeconds(5))
+        {
+            await Task.Delay(25, ct);
+            ids = ReadCommitted(dir);
+        }
+
+        return ids;
+    }
+
    // Opens an independent read-only FasterLog over the outbox's directory and returns the ids of
    // every record the writer has COMMITTED (BeginAddress .. CommittedUntilAddress). Uncommitted
    // enqueues sit past CommittedUntilAddress and are therefore invisible here — which is exactly
@@ -353,6 +353,18 @@ public sealed class ContinuousHistorizationRecorderTests : TestKit
    ///         The yield is the whole point: every other test here uses an outbox that returns
    ///         <c>ValueTask.CompletedTask</c>, so the continuation runs inline with the context still
    ///         installed and the defect is invisible. The production FasterLog outbox fsyncs per entry.
+    ///         The outbox double must therefore complete on a POOL thread
+    ///         (<c>Task.Delay(1).ConfigureAwait(false)</c>); a bare <c>Task.Yield()</c> posts back to the
+    ///         CAPTURED ActorTaskScheduler context and is just as vacuous as completing synchronously.
+    ///     </para>
+    ///     <para>
+    ///         ⚠️ The falsification control is NONDETERMINISTIC UNDER LOAD. Re-introducing the defect is
+    ///         only observable when the thread that resumes the continuation happens to carry no Akka
+    ///         actor cell in its ThreadStatic slot; on a loaded parallel test sweep the resuming pool
+    ///         thread can already be servicing this ActorSystem, the <c>Self</c> read succeeds, and the
+    ///         mutation passes. So a SINGLE green run of the intended-red mutation is not proof this
+    ///         regression is caught — re-run the mutation several times, ideally on an otherwise idle
+    ///         machine, before concluding the test is falsifiable.
    ///     </para>
    /// </summary>
    [Fact]
@@ -931,6 +943,15 @@ public sealed class ContinuousHistorizationRecorderTests : TestKit
    ///         <c>Task.Yield()</c>, which posts back to the CAPTURED ActorTaskScheduler context — leaves the
    ///         context installed and the defect invisible.
    ///     </para>
+    ///     <para>
+    ///         ⚠️ The falsification control is NONDETERMINISTIC UNDER LOAD. Re-introducing the defect is
+    ///         only observable when the thread that resumes the continuation happens to carry no Akka
+    ///         actor cell in its ThreadStatic slot; on a loaded parallel test sweep the resuming pool
+    ///         thread can already be servicing this ActorSystem, the <c>Self</c> read succeeds, and the
+    ///         mutation passes. So a SINGLE green run of the intended-red mutation is not proof this
+    ///         regression is caught — re-run the mutation several times, ideally on an otherwise idle
+    ///         machine, before concluding the test is falsifiable.
+    ///     </para>
    /// </remarks>
    [Fact]
    public async Task A_yielding_batch_append_neither_faults_the_actor_nor_loses_values()