perf(historian): batch ingest — one group commit per value burst

2026-08-16 04:33:35 -04:00
parent aad7583b79
commit 384568b47a
4 changed files with 455 additions and 73 deletions
@@ -718,6 +718,225 @@ public sealed class ContinuousHistorizationRecorderTests : TestKit
        Assert.Equal(1, writer.MaxObservedConcurrency);
    }

+    // ---------------- Batched ingest: ONE group commit per value burst ----------------
+
+    /// <summary>
+    ///     A whole driver burst delivered as one <see cref="VirtualTagActor.DependencyValueBatch"/> costs
+    ///     exactly ONE durable append, and every value in it still reaches the writer.
+    /// </summary>
+    /// <remarks>
+    ///     The load-bearing assertion is <c>AppendBatchCallCount == 1</c> paired with
+    ///     <c>AppendCallCount == 0</c>. "All five values arrived" was already true of the loop-over-the-
+    ///     per-value-handler batch path this replaces, so on its own it does not distinguish the two
+    ///     implementations at all; the call counts are what state the actual change — five per-entry
+    ///     fsyncs (production <c>FasterLogHistorizationOutbox</c> under <c>PerEntry</c> commit mode)
+    ///     collapsed into one group commit.
+    /// </remarks>
+    [Fact]
+    public async Task A_burst_appends_once_and_drains_all_values()
+    {
+        var mux = CreateTestProbe();
+        var writer = new FakeValueWriter { Succeed = true };
+        var outbox = new InMemoryOutbox();
+
+        var refs = new[] { "T1", "T2", "T3", "T4", "T5" };
+        IActorRef rec = Sys.ActorOf(ContinuousHistorizationRecorder.Props(
+            mux.Ref, writer, outbox, historizedRefs: refs,
+            drainInterval: TimeSpan.FromMilliseconds(50)));
+        mux.ExpectMsg<DependencyMuxActor.RegisterInterest>();
+
+        var stamp = new DateTime(2026, 8, 15, 9, 0, 0, DateTimeKind.Utc);
+        rec.Tell(new VirtualTagActor.DependencyValueBatch(
+            refs.Select((r, i) => new VirtualTagActor.DependencyValueChanged(r, (double)(i + 1), stamp))
+                .ToArray()));
+
+        await AwaitAssertAsync(
+            () =>
+            {
+                var written = writer.Snapshot();
+                Assert.Equal(5, written.Count);
+                Assert.Equal(refs.OrderBy(x => x, StringComparer.Ordinal),
+                    written.Select(w => w.Tag).OrderBy(x => x, StringComparer.Ordinal));
+                return Task.CompletedTask;
+            },
+            TimeSpan.FromSeconds(10));
+
+        Assert.Equal(1, outbox.AppendBatchCallCount);
+        Assert.Equal(0, outbox.AppendCallCount);
+
+        // …and the pass acked: the whole prefix left the outbox.
+        await AwaitAssertAsync(async () =>
+        {
+            var status = await rec.Ask<ContinuousHistorizationRecorder.RecorderStatus>(
+                ContinuousHistorizationRecorder.GetStatus.Instance, TimeSpan.FromSeconds(3));
+            Assert.Equal(0, status.QueuedDepth);
+            Assert.Equal(5, status.TotalRecorded);
+        }, TimeSpan.FromSeconds(10));
+    }
+
+    /// <summary>
+    ///     The batched append must also survive genuinely yielding — the Gitea #491 bug class, on the
+    ///     path that now owns the only await in the ingest handler.
+    /// </summary>
+    /// <remarks>
+    ///     Mirrors <see cref="A_yielding_append_neither_faults_the_actor_nor_loses_the_value"/>, and for
+    ///     the same reason: the ingest handler awaits
+    ///     <see cref="IHistorizationOutbox.AppendBatchAsync"/> with <c>ConfigureAwait(false)</c>, so a
+    ///     <c>Self</c> read after that await finds no ActorContext, throws
+    ///     <see cref="NotSupportedException"/> into supervision, and RESTARTS the actor — which in
+    ///     production discards every historized ref (the deployed set arrives later via
+    ///     <see cref="ContinuousHistorizationRecorder.UpdateHistorizedRefs"/>).
+    ///     <para>
+    ///         The outbox double must complete on a POOL thread (<c>Task.Delay(1).ConfigureAwait(false)</c>),
+    ///         which is what a real group-commit fsync does. A synchronously-completing fake — or a bare
+    ///         <c>Task.Yield()</c>, which posts back to the CAPTURED ActorTaskScheduler context — leaves the
+    ///         context installed and the defect invisible.
+    ///     </para>
+    /// </remarks>
+    [Fact]
+    public async Task A_yielding_batch_append_neither_faults_the_actor_nor_loses_values()
+    {
+        var mux = CreateTestProbe();
+        var writer = new FakeValueWriter { Succeed = true };
+        var outbox = new InMemoryOutbox(yieldOnAppend: true);
+
+        IActorRef rec = Sys.ActorOf(ContinuousHistorizationRecorder.Props(
+            mux.Ref, writer, outbox, historizedRefs: new[] { "A", "B", "C" },
+            drainInterval: TimeSpan.FromMilliseconds(200)));
+        mux.ExpectMsg<DependencyMuxActor.RegisterInterest>();
+
+        var stamp = new DateTime(2026, 8, 15, 10, 0, 0, DateTimeKind.Utc);
+        rec.Tell(new VirtualTagActor.DependencyValueBatch(new[]
+        {
+            new VirtualTagActor.DependencyValueChanged("A", 1.0, stamp),
+            new VirtualTagActor.DependencyValueChanged("B", 2.0, stamp),
+            new VirtualTagActor.DependencyValueChanged("C", 3.0, stamp),
+        }));
+
+        // Every value in the burst reaches the writer…
+        await AwaitAssertAsync(
+            () =>
+            {
+                Assert.Equal(3, writer.Snapshot().Count);
+                return Task.CompletedTask;
+            },
+            TimeSpan.FromSeconds(10));
+
+        // …and — the load-bearing half — the actor is still the SAME incarnation, holding the ref set it
+        // was seeded with. A restart would reset TotalRecorded and re-register interest from PreStart.
+        var status = await rec.Ask<ContinuousHistorizationRecorder.RecorderStatus>(
+            ContinuousHistorizationRecorder.GetStatus.Instance, TimeSpan.FromSeconds(3));
+        Assert.Equal(3, status.TotalRecorded);
+        mux.ExpectNoMsg(TimeSpan.FromMilliseconds(300));   // no PreStart re-register == no restart
+    }
+
+    /// <summary>
+    ///     A closed capture gate suppresses every item of a batch, and appends NOTHING (Gitea #537 on the
+    ///     batched path).
+    /// </summary>
+    /// <remarks>
+    ///     Extends <see cref="A_closed_capture_gate_suppresses_the_value"/> to the batch handler: the
+    ///     rewrite runs the gate per item while collecting entries, so a batched path that hoisted the
+    ///     gate read (or skipped it entirely) would silently double-record on the Secondary of every
+    ///     redundant pair. <c>SuppressedNotPrimary == 3</c> is the positive observation — "the outbox is
+    ///     empty" is equally true of a recorder that never ran.
+    /// </remarks>
+    [Fact]
+    public async Task The_capture_gate_suppresses_items_within_a_batch()
+    {
+        var mux = CreateTestProbe();
+        var writer = new FakeValueWriter();
+        var outbox = new InMemoryOutbox();
+
+        IActorRef rec = Sys.ActorOf(ContinuousHistorizationRecorder.Props(
+            mux.Ref, writer, outbox, historizedRefs: new[] { "A", "B", "C" },
+            captureGate: () => false));
+
+        var stamp = new DateTime(2026, 8, 15, 11, 0, 0, DateTimeKind.Utc);
+        rec.Tell(new VirtualTagActor.DependencyValueBatch(new[]
+        {
+            new VirtualTagActor.DependencyValueChanged("A", 1.0, stamp),
+            new VirtualTagActor.DependencyValueChanged("B", 2.0, stamp),
+            new VirtualTagActor.DependencyValueChanged("C", 3.0, stamp),
+        }));
+
+        await AwaitAssertAsync(async () =>
+        {
+            var status = await rec.Ask<ContinuousHistorizationRecorder.RecorderStatus>(
+                ContinuousHistorizationRecorder.GetStatus.Instance, TimeSpan.FromSeconds(3));
+            Assert.Equal(3, status.SuppressedNotPrimary);
+            Assert.Equal(0, status.TotalRecorded);
+            Assert.Equal(0, status.QueuedDepth);
+        });
+
+        Assert.Empty(writer.Snapshot());
+        Assert.Equal(0, outbox.AppendBatchCallCount);
+        Assert.Equal(0, outbox.AppendCallCount);
+    }
+
+    /// <summary>
+    ///     One item whose gate step THROWS is skipped; the rest of the burst still records, and the actor
+    ///     does not restart.
+    /// </summary>
+    /// <remarks>
+    ///     Batching turns a per-item throw into a tail-dropper: unguarded, it escapes the handler with the
+    ///     whole burst still uncommitted (the group commit happens after the loop), so every value —
+    ///     including the ones already collected — is lost, and supervision restarts the actor, which in
+    ///     production discards the historized-ref set. The realistic source is a faulting
+    ///     <c>captureGate</c>, which is what this drives: the gate throws on exactly the SECOND of three
+    ///     items.
+    ///     <para>
+    ///         The load-bearing assertion is that BOTH surviving values reached the writer — the one BEFORE
+    ///         the fault and the one AFTER it. The trailing one alone would not distinguish the guard from
+    ///         a restart that happened to re-drain, and the leading one alone is not evidence the loop
+    ///         continued.
+    ///     </para>
+    /// </remarks>
+    [Fact]
+    public async Task A_faulting_item_does_not_drop_the_rest_of_the_burst()
+    {
+        var mux = CreateTestProbe();
+        var writer = new FakeValueWriter { Succeed = true };
+        var outbox = new InMemoryOutbox();
+        var gateCalls = 0;
+
+        IActorRef rec = Sys.ActorOf(ContinuousHistorizationRecorder.Props(
+            mux.Ref, writer, outbox, historizedRefs: new[] { "A", "B", "C" },
+            drainInterval: TimeSpan.FromMilliseconds(50),
+            captureGate: () => Interlocked.Increment(ref gateCalls) == 2
+                ? throw new InvalidOperationException("role view unavailable")
+                : true));
+        mux.ExpectMsg<DependencyMuxActor.RegisterInterest>();
+
+        var stamp = new DateTime(2026, 8, 15, 12, 0, 0, DateTimeKind.Utc);
+        rec.Tell(new VirtualTagActor.DependencyValueBatch(new[]
+        {
+            new VirtualTagActor.DependencyValueChanged("A", 1.0, stamp),
+            new VirtualTagActor.DependencyValueChanged("B", 2.0, stamp),
+            new VirtualTagActor.DependencyValueChanged("C", 3.0, stamp),
+        }));
+
+        await AwaitAssertAsync(
+            () =>
+            {
+                var written = writer.Snapshot();
+                Assert.Contains(written, w => w.Tag == "A");
+                Assert.Contains(written, w => w.Tag == "C");
+                Assert.DoesNotContain(written, w => w.Tag == "B");
+                return Task.CompletedTask;
+            },
+            TimeSpan.FromSeconds(10));
+
+        var status = await rec.Ask<ContinuousHistorizationRecorder.RecorderStatus>(
+            ContinuousHistorizationRecorder.GetStatus.Instance, TimeSpan.FromSeconds(3));
+        Assert.Equal(1, status.IngestFaults);
+        Assert.Equal(2, status.TotalRecorded);
+        // The fault is NOT a "value the historian cannot carry" — that counter must stay clean, or an
+        // ingest-dependency outage is indistinguishable from bad data on the /hosts surface.
+        Assert.Equal(0, status.DroppedNonNumeric);
+        mux.ExpectNoMsg(TimeSpan.FromMilliseconds(300));   // no PreStart re-register == no restart
+    }
+
    /// <summary>
    ///     <see cref="IHistorianValueWriter"/> double that BLOCKS every call until <see cref="Release"/>,
    ///     recording the peak number of calls in flight simultaneously. That peak is what distinguishes a
@@ -874,6 +1093,8 @@ public sealed class ContinuousHistorizationRecorderTests : TestKit
        private readonly List<HistorizationOutboxEntry> _entries = new();
        private readonly int _capacity;
        private long _dropped;
+        private int _appendCalls;
+        private int _appendBatchCalls;

        private readonly bool _yieldOnAppend;

@@ -905,8 +1126,39 @@ public sealed class ContinuousHistorizationRecorderTests : TestKit
            }
        }

+        /// <summary>Lifetime count of single-entry <see cref="AppendAsync"/> calls. Distinguishes the
+        /// batched ingest path from the per-value loop it replaced — both record the same values, so only
+        /// the call shape tells them apart.</summary>
+        public int AppendCallCount
+        {
+            get
+            {
+                lock (_gate)
+                {
+                    return _appendCalls;
+                }
+            }
+        }
+
+        /// <summary>Lifetime count of <see cref="AppendBatchAsync"/> calls — one per group commit.</summary>
+        public int AppendBatchCallCount
+        {
+            get
+            {
+                lock (_gate)
+                {
+                    return _appendBatchCalls;
+                }
+            }
+        }
+
        public ValueTask AppendAsync(HistorizationOutboxEntry entry, CancellationToken ct)
        {
+            lock (_gate)
+            {
+                _appendCalls++;
+            }
+
            if (_yieldOnAppend)
            {
                return AppendYieldingAsync(entry);
@@ -918,6 +1170,11 @@ public sealed class ContinuousHistorizationRecorderTests : TestKit

        public ValueTask AppendBatchAsync(IReadOnlyList<HistorizationOutboxEntry> entries, CancellationToken ct)
        {
+            lock (_gate)
+            {
+                _appendBatchCalls++;
+            }
+
            if (entries.Count == 0)
            {
                return ValueTask.CompletedTask;