fix(localdb): flush the control lane before a faulting session cancels its writer (0.3.1)
A session that faulted during the handshake cancelled its writer loop immediately, which could kill its OWN Handshake while it still sat in the control lane: the enqueue -> receive -> validate -> fault path has no yields, while the writer's wakeup waits in the thread-pool queue, so with the peer's handshake already buffered the cancel won the race almost every time (39/40 in the new pinning test). Mismatch detection is symmetric — the peer computes its fail-closed verdict FROM our Handshake — so the healthy peer never learned anything: in tests it waited out the full 15 s receive cap (the register-row-41 flake on SyncSessionTests.Handshake_DigestMismatch_FailsClosed), and in production it would misclassify a permanent schema mismatch as a generic disconnect and reconnect-loop at the fast backoff instead of taking the classified FailedPrecondition -> LogError + max-backoff path. RunAsync's fault teardown now completes both lanes so the writer drains what is already enqueued and exits on its own, with a 2 s deferred cancel as the backstop for a wedged transport. Caller-initiated cancellation is unchanged (the linked token is already cancelled, so shutdown is not delayed), and nothing on the wire changes — a 0.3.0 peer simply receives the Handshake it was always owed. New SyncSessionTests.Handshake_FaultTeardown_StillTransmitsTheLocalHandshake pins the race (40 concurrent preloaded-mismatch sessions; 39/40 red pre-fix, green post-fix). BatchBudgetTests hardened separately: the register's other flaky test (Pump_RowLargerThanTheBudget_IsSentAloneRatherThanStallingTheStream) awaits the scripted-peer channel without ever observing the session task, so ANY session fault surfaced as an opaque 15 s timeout. A run-observing NextAsync overload now races the session task at every read site in the class, so the next flake reports the session's real exception instead. (The handshake-race root cause does not structurally apply to this test — no handshake fault occurs there — so its intermittent timeout is attributed to the fault-hiding wait, now removed.) Suite 200/200 green. Closes ScadaBridge deferred-work register row 41 (cross-repo).
This commit is contained in:
@@ -5,7 +5,7 @@
|
||||
<Nullable>enable</Nullable>
|
||||
<ImplicitUsings>enable</ImplicitUsings>
|
||||
<LangVersion>latest</LangVersion>
|
||||
<Version>0.3.0</Version>
|
||||
<Version>0.3.1</Version>
|
||||
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
|
||||
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
|
||||
<!-- NU1510 (package-pruning advice): the Replication and Tests projects carry a
|
||||
|
||||
@@ -27,6 +27,10 @@ internal sealed class SyncSession
|
||||
// inbound batch rate (the peer's flow control), so its lane can safely be unbounded.
|
||||
private const int DataLaneCapacity = 16;
|
||||
|
||||
// How long a faulting session's teardown lets the writer keep flushing already-enqueued
|
||||
// messages before the backstop cancel; only ever reached when the transport itself stalls.
|
||||
private static readonly TimeSpan FaultFlushGrace = TimeSpan.FromSeconds(2);
|
||||
|
||||
private readonly SqliteLocalDb _db;
|
||||
private readonly OplogStore _store;
|
||||
private readonly LwwApplier _applier;
|
||||
@@ -140,7 +144,20 @@ internal sealed class SyncSession
|
||||
{
|
||||
// Handshake-phase faults (and the primary rethrow above) land here: the writer may
|
||||
// still be running, so stop and observe it before the primary exception surfaces.
|
||||
linked.Cancel();
|
||||
// It must be allowed to DRAIN, not be cancelled outright: the Handshake enqueued at
|
||||
// the top of HandshakeAsync can still be sitting in the control lane (the enqueue →
|
||||
// receive → validate → fault path has no yields, while the writer's wakeup waits in
|
||||
// the thread-pool queue), and that message is the only thing that lets the peer
|
||||
// compute its own fail-closed verdict — mismatch detection is symmetric, so this
|
||||
// node's locally-thrown verdict reaches the peer only as the Handshake it transmits.
|
||||
// Cancelling first killed it almost every time the peer's handshake was already
|
||||
// buffered, leaving the healthy peer to misread a schema mismatch as a generic
|
||||
// disconnect and reconnect-loop at the fast backoff. Completing the lanes lets the
|
||||
// writer send what is already enqueued and exit on its own; the deferred cancel is
|
||||
// the backstop that keeps a wedged transport from holding teardown hostage.
|
||||
control.Writer.TryComplete();
|
||||
data.Writer.TryComplete();
|
||||
linked.CancelAfter(FaultFlushGrace);
|
||||
await DrainSecondaryAsync(writer);
|
||||
throw;
|
||||
}
|
||||
|
||||
@@ -70,6 +70,20 @@ public sealed class BatchBudgetTests : IDisposable
|
||||
return msg!;
|
||||
}
|
||||
|
||||
// Reads the next scripted-peer message while also observing the session task: a session that
|
||||
// faults would otherwise leave this wait to run out the whole test timeout, reporting an
|
||||
// opaque cancellation instead of the session's real exception.
|
||||
private static async Task<SyncMessage> NextAsync(Task run, ChannelReader<SyncMessage> reader, CancellationToken ct)
|
||||
{
|
||||
var next = NextAsync(reader, ct);
|
||||
if (ReferenceEquals(await Task.WhenAny(run, next), run))
|
||||
{
|
||||
await run;
|
||||
throw new InvalidOperationException("The sync session ended before the expected message arrived.");
|
||||
}
|
||||
return await next;
|
||||
}
|
||||
|
||||
private static async Task SwallowAsync(Task task)
|
||||
{
|
||||
try { await task; }
|
||||
@@ -140,7 +154,7 @@ public sealed class BatchBudgetTests : IDisposable
|
||||
var seen = new List<long>();
|
||||
while (seen.Count < 20)
|
||||
{
|
||||
var msg = await NextAsync(fromSession, cts.Token);
|
||||
var msg = await NextAsync(run, fromSession, cts.Token);
|
||||
if (msg.MsgCase != SyncMessage.MsgOneofCase.DeltaBatch)
|
||||
continue;
|
||||
Assert.True(BatchBytes(msg.DeltaBatch) <= budget,
|
||||
@@ -180,7 +194,7 @@ public sealed class BatchBudgetTests : IDisposable
|
||||
var batches = new List<DeltaBatch>();
|
||||
while (batches.Sum(b => b.Entries.Count) < 2)
|
||||
{
|
||||
var msg = await NextAsync(fromSession, cts.Token);
|
||||
var msg = await NextAsync(run, fromSession, cts.Token);
|
||||
if (msg.MsgCase == SyncMessage.MsgOneofCase.DeltaBatch)
|
||||
batches.Add(msg.DeltaBatch);
|
||||
}
|
||||
@@ -220,7 +234,7 @@ public sealed class BatchBudgetTests : IDisposable
|
||||
var seen = 0;
|
||||
while (seen < 12)
|
||||
{
|
||||
var msg = await NextAsync(fromSession, cts.Token);
|
||||
var msg = await NextAsync(run, fromSession, cts.Token);
|
||||
if (msg.MsgCase != SyncMessage.MsgOneofCase.DeltaBatch)
|
||||
continue;
|
||||
Assert.True(msg.DeltaBatch.Entries.Count <= 5);
|
||||
@@ -312,7 +326,7 @@ public sealed class BatchBudgetTests : IDisposable
|
||||
}
|
||||
|
||||
SyncMessage ack;
|
||||
do { ack = await NextAsync(fromSession, cts.Token); }
|
||||
do { ack = await NextAsync(run, fromSession, cts.Token); }
|
||||
while (ack.MsgCase != SyncMessage.MsgOneofCase.DeltaAck);
|
||||
|
||||
// The discriminator: uncoalesced, the FIRST ack could only cover seq 1.
|
||||
@@ -348,7 +362,7 @@ public sealed class BatchBudgetTests : IDisposable
|
||||
new SyncMessage { SnapshotBegin = new SnapshotBegin { AsOfSeq = 77 } }, cts.Token);
|
||||
|
||||
SyncMessage ack;
|
||||
do { ack = await NextAsync(fromSession, cts.Token); }
|
||||
do { ack = await NextAsync(run, fromSession, cts.Token); }
|
||||
while (ack.MsgCase != SyncMessage.MsgOneofCase.DeltaAck);
|
||||
Assert.Equal(1, ack.DeltaAck.AppliedThruSeq);
|
||||
|
||||
|
||||
@@ -208,6 +208,50 @@ public sealed class SyncSessionTests : IDisposable
|
||||
Assert.Empty(await ReadOrders(b.Db));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Handshake_FaultTeardown_StillTransmitsTheLocalHandshake()
|
||||
{
|
||||
// Schema-mismatch detection is symmetric: each side computes its verdict FROM the other's
|
||||
// Handshake. So a session that faults on the peer's handshake must still flush its own —
|
||||
// otherwise the healthy peer learns nothing and waits out its receive cap (tests) or
|
||||
// misclassifies the mismatch as a generic disconnect and reconnect-loops (production).
|
||||
//
|
||||
// The race being pinned: with the peer's handshake already buffered, the path from
|
||||
// enqueueing our Handshake on the control lane to the validation throw has no yields,
|
||||
// while the writer's wakeup sits in the thread-pool queue — a teardown that cancels the
|
||||
// writer can therefore kill the Handshake before it ever transmits. Preloading makes the
|
||||
// sprint as tight as possible, and running the scenarios concurrently starves the pool
|
||||
// the way a busy test suite does.
|
||||
const int Iterations = 40;
|
||||
var sides = new List<Side>(Iterations);
|
||||
for (var i = 0; i < Iterations; i++)
|
||||
sides.Add(await NewSideAsync());
|
||||
|
||||
using var cts = new CancellationTokenSource(RunTimeout);
|
||||
var runs = new List<(Task Run, ChannelReader<SyncMessage> FromSession)>(Iterations);
|
||||
foreach (var side in sides)
|
||||
{
|
||||
var (duplex, toSession, fromSession) = ScriptedPeer();
|
||||
var mismatched = MatchingHandshake(side.Db, "peer");
|
||||
mismatched.Tables[0].Digest = "not-the-local-digest";
|
||||
await toSession.WriteAsync(new SyncMessage { Handshake = mismatched }, cts.Token);
|
||||
runs.Add((side.Session.RunAsync(duplex, cts.Token), fromSession));
|
||||
}
|
||||
|
||||
var lost = 0;
|
||||
foreach (var (run, fromSession) in runs)
|
||||
{
|
||||
await Assert.ThrowsAsync<LocalDbSchemaMismatchException>(() => run);
|
||||
var sent = false;
|
||||
while (fromSession.TryRead(out var msg))
|
||||
sent |= msg.MsgCase == SyncMessage.MsgOneofCase.Handshake;
|
||||
if (!sent)
|
||||
lost++;
|
||||
}
|
||||
|
||||
Assert.True(lost == 0, $"{lost}/{Iterations} sessions tore down without transmitting their own Handshake");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Handshake_SchemaVersionMismatch_FailsClosed()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user