fix(localdb): flush the HLC anchor on deregister, bound the sync inbox (0.2.1)

Two adversarial-review findings against 0.2.0. Wire-compatible with 0.1.x/0.2.0 peers: no proto change, no lib_schema_version bump, no on-disk schema change. F4 — DeregisterReplicated destroyed the HLC clock's only durable crash anchor. Clock recovery on open is MAX(hlc_high_water, MAX(oplog.hlc), MAX(row_version.hlc), last_seen_hlc); hlc_high_water is written only by Dispose, and deregistration deletes the table's rows from BOTH ledgers. On a file whose registered tables have all been deregistered, an ungraceful exit (crash, SIGKILL, power loss) therefore left nothing above zero to recover from: the reopened clock restarts from wall-clock time and can re-issue stamps below HLCs peers already hold, which LWW discards as older. The current clock is now flushed inside the deregistration transaction, before the prune, as a separate command so it cannot inflate the "did anything change?" answer. FlushHlcHighWater takes an optional transaction (SQLite rejects a command with no transaction on a connection that has one) and is now MAX-guarded — every writer of that column is flushing a monotonic clock, so a lower value can only be stale. F3 — the sync inbox was Channel.CreateUnbounded, making inbound memory a function of what the PEER sends. The bad case is not hypothetical: a snapshot streams every row of every registered table, needs_snapshot is per-database, and both sides of a pair that enables replication late baseline unconditionally — so a mutual full snapshot of a large dataset (an event log near its storage cap) was buffered whole, in memory, on both nodes at once. The inbox is now bounded at 64 messages with wait-mode backpressure, so the ceiling is ~64 x MaxBatchBytes — a configured number rather than a peer-controlled one — and a full inbox stops draining the gRPC stream, which closes the peer's flow-control window. The pump writes with WriteAsync, not TryWrite: on a full bounded channel TryWrite drops the message, and every inbound message is protocol state. Bounding alone would have deadlocked the very case it was aimed at. The snapshot a node owes was streamed from INSIDE the handshake, before the receive loop existed, so neither side drained its inbox while sending; a mutual snapshot converged only because the inbox was unbounded. The snapshot now runs as the first phase of the outbound task, started alongside the receive loop, so the inbox is drained for the whole life of the session. Interleaving needed no new safety: snapshot rows and deltas both merge through LWW and the snapshot's watermark write is MAX-guarded. Ordering within the outbound lane is preserved (the pump still starts only after the snapshot, from its as-of seq). Per-table needs_snapshot (the finding's optional third part) is NOT done: the flag lives in __localdb_peer_state and narrowing it needs an on-disk schema change, which this fix deliberately avoids. Documented as a follow-up, and the cost is now written down where it is decided — RegisterReplicated(baselineExistingRows) remarks and the README both say that baselining one table re-streams every registered table in both directions, and that high-volume tables should be trimmed before enabling replication late. 188/188 tests pass (was 184). New coverage: the deregistered clock anchor surviving an ungraceful reopen (a second instance over the same file with no Dispose — fails without the flush, recovering at 0), the flush not counting as work done, bounded-inbox backpressure under a slow reader (the pump stalls at the bound and loses nothing, proven by contiguous in-order delivery), and a mutual snapshot over a capacity-1 transport converging — that last one deadlocks and times out against the old handshake-phase snapshot ordering. Version 0.2.1.
2026-08-14 23:34:55 -04:00
parent 1ce0c5955f
commit 522cfbfda6
10 changed files with 376 additions and 43 deletions
@@ -173,6 +173,71 @@ public sealed class GrpcAdapterTests : IAsyncLifetime
        Assert.True(sw.Elapsed < TimeSpan.FromSeconds(3), $"teardown took {sw.Elapsed}");
    }

+    // ---- inbox backpressure ------------------------------------------------------------------
+
+    [Fact]
+    public async Task Inbox_SlowReader_StopsDrainingTheStream_AndLosesNothing()
+    {
+        // The pump must STOP pulling from the gRPC stream once the inbox is full — that stall is
+        // what closes the peer's flow-control window and bounds this process's memory at
+        // InboxCapacity x MaxBatchBytes instead of "whatever the peer decides to send" (a full
+        // snapshot of a large dataset, on both nodes at once, when a pair baselines late).
+        const int capacity = Replication.Internal.GrpcSyncDuplex.InboxCapacity;
+        using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(15));
+        var reader = new CountingReader();
+        var (duplex, readerTask) = Replication.Internal.GrpcSyncDuplex.Create(
+            reader, new HangingWriter(), cts.Token);
+
+        // Nothing reads the inbox: the pump fills it and blocks.
+        await WaitForAsync(() => Task.FromResult(reader.Produced >= capacity), TimeSpan.FromSeconds(5));
+        await Task.Delay(200, cts.Token);
+        // capacity in the channel + at most one in the pump's hand, blocked on WriteAsync.
+        Assert.True(reader.Produced <= capacity + 1,
+            $"the reader pump kept draining an already-full inbox: {reader.Produced} messages read for a {capacity}-message bound");
+
+        // Draining releases it, and every message arrives exactly once in stream order — TryWrite
+        // on a full bounded channel would have dropped them silently instead.
+        for (long expected = 1; expected <= capacity + 1; expected++)
+        {
+            var msg = await ReadInboxAsync(duplex, cts.Token);
+            Assert.Equal(expected, msg.DeltaAck.AppliedThruSeq);
+        }
+        await WaitForAsync(() => Task.FromResult(reader.Produced > capacity + 1), TimeSpan.FromSeconds(5));
+
+        cts.Cancel();
+        try { await readerTask.WaitAsync(TimeSpan.FromSeconds(5)); }
+        catch (OperationCanceledException) { }
+    }
+
+    private static async Task<SyncMessage> ReadInboxAsync(
+        Replication.Internal.SyncDuplex duplex, CancellationToken ct)
+    {
+        await duplex.Inbox.WaitToReadAsync(ct);
+        duplex.Inbox.TryRead(out var message);
+        return message!;
+    }
+
+    // Produces messages as fast as the inbox will take them, numbering each one so the consumer can
+    // prove nothing was dropped or reordered.
+    private sealed class CountingReader : IAsyncStreamReader<SyncMessage>
+    {
+        private long _produced;
+
+        public long Produced => Interlocked.Read(ref _produced);
+
+        public SyncMessage Current { get; private set; } = new();
+
+        public Task<bool> MoveNext(CancellationToken cancellationToken)
+        {
+            cancellationToken.ThrowIfCancellationRequested();
+            Current = new SyncMessage
+            {
+                DeltaAck = new DeltaAck { AppliedThruSeq = Interlocked.Increment(ref _produced) },
+            };
+            return Task.FromResult(true);
+        }
+    }
+
    // ---- validator -------------------------------------------------------------------------

    [Fact]
@@ -420,6 +420,63 @@ public sealed class RegistrationTests : IDisposable
        Assert.False(await NeedsSnapshot(db));
    }

+    // ---- Deregistration and the HLC crash anchor ------------------------------------------
+
+    [Fact]
+    public async Task Deregister_FlushesHlcAnchor_SoAnUngracefulReopenCannotRegress()
+    {
+        // The clock recovers as MAX(hlc_high_water, oplog HLCs, ledger HLCs, last_seen_hlc), and
+        // deregistering the only registered table empties both ledgers. Without the in-transaction
+        // flush, an ungraceful exit right afterwards leaves hlc_high_water at 0 and the reopened
+        // clock restarts from wall-clock time — below stamps the peer already holds.
+        var crashed = Create();
+        try
+        {
+            await CreateOrders(crashed);
+            crashed.RegisterReplicated("orders");
+            await crashed.ExecuteAsync("INSERT INTO orders (id, sku, qty) VALUES (1,'A',1)");
+
+            // A peer HLC an hour ahead, merged as the applier would merge it: this is what makes the
+            // clock outrun wall-clock time, and therefore what a wall-clock restart would regress.
+            var peerHlc = DateTimeOffset.UtcNow.AddHours(1).ToUnixTimeMilliseconds() << 16;
+            crashed.Clock.Observe(peerHlc);
+            var preDereg = crashed.Clock.Current;
+            Assert.True(preDereg >= peerHlc);
+
+            crashed.DeregisterReplicated("orders");
+
+            Assert.Equal(preDereg, await CountAsync(crashed, "SELECT hlc_high_water FROM __localdb_meta WHERE id = 1"));
+
+            // The evidence the clock would otherwise have recovered from is gone.
+            Assert.Equal(0, await CountAsync(crashed, "SELECT COUNT(*) FROM __localdb_oplog"));
+            Assert.Equal(0, await CountAsync(crashed, "SELECT COUNT(*) FROM __localdb_row_version"));
+            Assert.Equal(0, await CountAsync(crashed, "SELECT last_seen_hlc FROM __localdb_peer_state WHERE id = 1"));
+
+            // Ungraceful reopen: a second instance over the same file while the first was NEVER
+            // disposed, so nothing but the deregistration itself can have written the anchor.
+            using var reopened = Create();
+            Assert.True(reopened.Clock.Current >= preDereg,
+                $"recovered clock {reopened.Clock.Current} regressed below the pre-deregistration stamp {preDereg}");
+            Assert.True(reopened.Clock.Next() > preDereg);
+        }
+        finally
+        {
+            crashed.Dispose();
+        }
+    }
+
+    [Fact]
+    public async Task Deregister_HlcFlush_DoesNotCountAsWorkDone()
+    {
+        // The flush is an UPDATE on every call, including the ones that clean up nothing; it must
+        // not turn the "was there anything to do?" answer into a permanent true.
+        using var db = Create();
+        await CreateOrders(db);
+
+        Assert.False(db.DeregisterReplicated("orders"));
+        Assert.False(db.DeregisterReplicated("ghost"));
+    }
+
    private static async Task<long> CountAsync(SqliteLocalDb db, string sql) =>
        (await db.QueryAsync(sql, r => r.GetInt64(0)))[0];

@@ -67,10 +67,15 @@ public sealed class SnapshotResyncTests : IDisposable
        return new Side(db, store, applier, session);
    }

-    private static (SyncDuplex A, SyncDuplex B) DuplexPair()
+    // capacity models a transport that applies backpressure (which the gRPC duplex now is, via its
+    // bounded inbox); null is the historical unbounded pipe.
+    private static (SyncDuplex A, SyncDuplex B) DuplexPair(int? capacity = null)
    {
-        var aToB = Channel.CreateUnbounded<SyncMessage>();
-        var bToA = Channel.CreateUnbounded<SyncMessage>();
+        Channel<SyncMessage> New() => capacity is int c
+            ? Channel.CreateBounded<SyncMessage>(c)
+            : Channel.CreateUnbounded<SyncMessage>();
+        var aToB = New();
+        var bToA = New();
        var a = new SyncDuplex { Send = (m, ct) => aToB.Writer.WriteAsync(m, ct).AsTask(), Inbox = bToA.Reader };
        var b = new SyncDuplex { Send = (m, ct) => bToA.Writer.WriteAsync(m, ct).AsTask(), Inbox = aToB.Reader };
        return (a, b);
@@ -170,6 +175,50 @@ public sealed class SnapshotResyncTests : IDisposable
            IsTombstone = false,
        };

+    [Fact]
+    public async Task MutualSnapshot_OverABoundedTransport_ConvergesWithoutDeadlock()
+    {
+        // BOTH sides owe a snapshot — the ordinary outcome of enabling replication late on a pair,
+        // since baselining flags needs_snapshot on whichever node baselined and both nodes do.
+        //
+        // The snapshot used to be streamed from INSIDE the handshake, before the receive loop
+        // existed, so neither side drained its inbox while sending. That exchange completed only
+        // because the inbox was unbounded and could hold the peer's ENTIRE snapshot in memory —
+        // which is the memory profile this design gave up on. Over a transport that pushes back
+        // (which the gRPC duplex now is, with its bounded inbox) the old shape deadlocks outright.
+        // Streaming the snapshot alongside an already-running receive loop is what makes both the
+        // bound and this test possible.
+        var options = new ReplicationOptions { MaxBatchSize = 2, FlushInterval = TimeSpan.FromMilliseconds(20) };
+        var a = await NewSideAsync(options);
+        var b = await NewSideAsync(options);
+
+        const int rowsPerSide = 60;
+        for (var i = 1; i <= rowsPerSide; i++)
+        {
+            await a.Db.ExecuteAsync("INSERT INTO orders (id, sku, qty) VALUES (@id, 'A', @id)", new { id = i });
+            await b.Db.ExecuteAsync("INSERT INTO orders (id, sku, qty) VALUES (@id, 'B', @id)", new { id = 100 + i });
+        }
+        await a.Store.SetNeedsSnapshotAsync(true, default);
+        await b.Store.SetNeedsSnapshotAsync(true, default);
+
+        // Capacity 1 each way: the tightest possible bound, so a side that stops draining its inbox
+        // to send stalls at once instead of getting away with it on buffer slack.
+        var (da, db) = DuplexPair(capacity: 1);
+        using var cts = new CancellationTokenSource(RunTimeout);
+        var runA = a.Session.RunAsync(da, cts.Token);
+        var runB = b.Session.RunAsync(db, cts.Token);
+
+        await WaitForAsync(
+            async () => (await ReadOrders(a.Db)).Count == 2 * rowsPerSide
+                && (await ReadOrders(b.Db)).Count == 2 * rowsPerSide,
+            RunTimeout);
+        Assert.Equal(await ReadOrders(a.Db), await ReadOrders(b.Db));
+
+        cts.Cancel();
+        await SwallowAsync(runA);
+        await SwallowAsync(runB);
+    }
+
    [Fact]
    public async Task WipedPeer_AfterEverythingWasAckedAndPruned_StillGetsSnapshot()
    {