fix(localdb): refuse cascade-delete FK parents; make one entry's row+ledger writes atomic (0.3.3, ScadaBridge arch-review P4-7)
Two halves of the same INSERT OR REPLACE hazard in the LWW apply path. Wire-compatible with every earlier peer: no proto change, no lib_schema_version bump, no on-disk schema change. 1. RegisterReplicated now REFUSES a table that another table references with a row-mutating ON DELETE action (CASCADE / SET NULL / SET DEFAULT). Applying a remote row uses INSERT OR REPLACE, whose implicit DELETE fires that referential action — while the apply path holds the capture guard raised, precisely so its own writes are not re-captured. The child rows it deletes or nulls therefore produce no oplog entry: the peer keeps them and the pair diverges with nothing reporting a fault. Registration is the only point at which the problem is still visible, so it is refused there with an actionable LocalDbRegistrationException naming the referencing table and its action. NO ACTION / RESTRICT stay allowed: they mutate no child row, so the implicit delete just fails and the applier dead-letters the entry — loud, not silent. Registration-time only; a cascading child table created LATER is not caught (SQLite offers no DDL hook). No current consumer has a foreign key on a replicated table (ScadaBridge's ten tables have none), so this is a guard against a future foot-gun, not a live fix. 2. One entry's row write and its ledger write are now atomic. They were two statements under the poison catch, so a failure landing between them committed a deleted/replaced row carrying a STALE LIVE row-version entry — exactly the shape SnapshotStreamer warns about and skips, so that key never travels again and the pair is permanently divergent for it. A per-entry SAVEPOINT makes the pair all-or-nothing while keeping the catch's per-ENTRY granularity: entries applied earlier in the same batch survive the rollback, the poison entry is still dead-lettered, and the batch still continues. The RELEASE runs outside the catch on purpose, so a failure there rolls the whole batch back rather than being swallowed as poison. Measured cost on a 100k-row backlog drain: ~7% (178.6k rows/s -> 166.1k rows/s). Tests: both hazards proven against the unfixed code first (FK parent registered silently; an injected ledger-write abort left the row deleted with a live ledger entry). Adds FK guard coverage (parent refused, child allowed, SET NULL / SET DEFAULT refused, NO ACTION / RESTRICT allowed, unrelated cascades ignored) and two applier atomicity tests. The statement-count lock-in moves 7 -> 9 for the two new savepoint statements. Version 0.3.2 -> 0.3.3; README limitations + release notes document the constraint.
This commit is contained in:
@@ -427,9 +427,12 @@ public sealed class LwwApplierTests : IDisposable
|
||||
var result = await applier.ApplyBatchAsync(entries, default);
|
||||
|
||||
Assert.Equal(202, result.Applied);
|
||||
// Four table-independent statements (applying guard, version lookup, row-version write,
|
||||
// peer state), plus an upsert per table touched and a delete for the one table tombstoned.
|
||||
Assert.Equal(7, applier.LastBatchStatementCount);
|
||||
// Six table-independent statements (applying guard, version lookup, row-version write,
|
||||
// peer state, plus the per-entry savepoint's SAVEPOINT and RELEASE), an upsert per table
|
||||
// touched, and a delete for the one table tombstoned. The savepoint's ROLLBACK TO and the
|
||||
// dead-letter insert are absent because this batch has no poison entry — statements are
|
||||
// built on first use, so the count tracks the statement KINDS the batch actually ran.
|
||||
Assert.Equal(9, applier.LastBatchStatementCount);
|
||||
|
||||
// ...and the batch is still correct, statement reuse notwithstanding.
|
||||
Assert.Null(await ReadOrder(db, 1));
|
||||
@@ -462,6 +465,72 @@ public sealed class LwwApplierTests : IDisposable
|
||||
Assert.Equal(("LAST", (long?)5), await ReadOrder(db, 5));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Apply_LedgerWriteFails_LeavesNeitherTheDeleteNorTheLedgerEntry()
|
||||
{
|
||||
// The hazard (P4-7): applying a tombstone is TWO statements — delete the row, then write the
|
||||
// ledger's tombstone version — and the poison catch commits whatever the first one already
|
||||
// did. A failure landing between them therefore committed a DELETED ROW carrying a STALE LIVE
|
||||
// ledger entry, which is exactly the shape SnapshotStreamer warns about and skips: the key
|
||||
// then never travels again and the pair is divergent for that row, permanently and silently.
|
||||
//
|
||||
// The failure is injected with a trigger that aborts the ledger write (in production the
|
||||
// realistic causes are disk-full / IO error / corruption — none of them reproducible here),
|
||||
// and RAISE(ABORT) aborts only the statement, so the applier's narrow SqliteException catch
|
||||
// treats it as per-entry poison exactly as it would a real one.
|
||||
using var db = await NewOrdersDb();
|
||||
await db.ExecuteAsync("INSERT INTO orders (id, sku, qty) VALUES (1, 'LOCAL', 5)");
|
||||
var localHlc = await RowVersionHlc(db, Pk1);
|
||||
|
||||
await db.ExecuteAsync(
|
||||
"CREATE TRIGGER poison_ledger BEFORE INSERT ON __localdb_row_version " +
|
||||
"WHEN NEW.is_tombstone = 1 BEGIN SELECT RAISE(ABORT, 'injected ledger failure'); END");
|
||||
|
||||
var result = await new LwwApplier(db).ApplyBatchAsync(
|
||||
[Entry(90, Pk1, null, localHlc + 1000, "remote-node", tombstone: true)], default);
|
||||
|
||||
// The entry is poison, so it is dead-lettered and the batch continues — unchanged semantics.
|
||||
Assert.Equal(1, result.DeadLettered);
|
||||
Assert.Equal(0, result.Applied);
|
||||
|
||||
// BOTH or NEITHER. Neither, here: the row must still be present, because its ledger entry
|
||||
// still says "live at the local version".
|
||||
Assert.Equal(("LOCAL", (long?)5), await ReadOrder(db, 1));
|
||||
var rv = await db.QueryAsync(
|
||||
"SELECT hlc, is_tombstone FROM __localdb_row_version WHERE table_name = 'orders' AND pk_json = @pk",
|
||||
x => (Hlc: x.GetInt64(0), Tomb: x.GetInt64(1)), new { pk = Pk1 });
|
||||
Assert.Single(rv);
|
||||
Assert.Equal(localHlc, rv[0].Hlc);
|
||||
Assert.Equal(0, rv[0].Tomb);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Apply_LedgerWriteFails_MidBatch_LeavesTheOtherEntriesApplied()
|
||||
{
|
||||
// The atomicity must be per-ENTRY, not per-batch: rolling back to the savepoint may not
|
||||
// discard the rows applied before the poison one, nor block the ones after it.
|
||||
using var db = await NewOrdersDb();
|
||||
await db.ExecuteAsync("INSERT INTO orders (id, sku, qty) VALUES (2, 'DOOMED', 2)");
|
||||
var doomedHlc = await RowVersionHlc(db, "{\"id\":2}");
|
||||
|
||||
await db.ExecuteAsync(
|
||||
"CREATE TRIGGER poison_ledger BEFORE INSERT ON __localdb_row_version " +
|
||||
"WHEN NEW.is_tombstone = 1 BEGIN SELECT RAISE(ABORT, 'injected ledger failure'); END");
|
||||
|
||||
var result = await new LwwApplier(db).ApplyBatchAsync(
|
||||
[
|
||||
Entry(91, Pk1, "{\"id\":1,\"sku\":\"BEFORE\",\"qty\":1}", doomedHlc + 1, "remote-node"),
|
||||
Entry(92, "{\"id\":2}", null, doomedHlc + 1000, "remote-node", tombstone: true),
|
||||
Entry(93, "{\"id\":3}", "{\"id\":3,\"sku\":\"AFTER\",\"qty\":3}", doomedHlc + 3, "remote-node"),
|
||||
], default);
|
||||
|
||||
Assert.Equal(2, result.Applied);
|
||||
Assert.Equal(1, result.DeadLettered);
|
||||
Assert.Equal(("BEFORE", (long?)1), await ReadOrder(db, 1));
|
||||
Assert.Equal(("DOOMED", (long?)2), await ReadOrder(db, 2));
|
||||
Assert.Equal(("AFTER", (long?)3), await ReadOrder(db, 3));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Apply_ReplaceExistingRow_DoesNotFireCaptureTriggers()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user