[LocalDb] Snapshot streams child rows before parents; a failed row is skipped forever (latent) #37

New Issue

2026-11-09T14:39:21-05:00

dohertj2 commented

2026-11-09 14:39:21 -05:00

From the scadaproj#15 shared-library review (code-reviews/LocalDb/findings.md, LocalDb-002), verified at 91fb2b4. Update the finding's Resolution row when fixed.


Severity	Medium
Priority	P2
Category	Correctness & logic bugs
Status	Open
Location	`ZB.MOM.WW.LocalDb/src/ZB.MOM.WW.LocalDb.Replication/Internal/SnapshotStreamer.cs:79`

Description

SnapshotStreamer.SendAsync streams tables in ordinal name order
(.OrderBy(t => t.Name, StringComparer.Ordinal), line 79–81). Within a table it pages
ORDER BY rv.pk_json (line 196), which is text order: {"id":10} sorts before {"id":2}.

The receiver applies through LwwApplier on a connection with PRAGMA foreign_keys=ON. An
immediate FK violation is a SqliteException, and LwwApplier.cs:116 treats it as per-entry
poison: dead-letter, keep going. SnapshotApplier.OnCompleteAsync then moves the watermark to
as_of_seq, and the sender clears its snapshot debt on the SnapshotAck. Nothing ever re-sends the
dead-lettered rows.

Registration explicitly allows these schemas. EnsureNoInboundActionOnDelete rejects only
CASCADE / SET NULL / SET DEFAULT, and README §Limitations recommends NO ACTION/RESTRICT. So a
replicated parent/child pair with a default FK is a supported configuration that the delta path
handles (seq order matches commit order) but the snapshot path does not.

Failure scenario

Reproduced in a scratch test:

Register z_parent(id PK) and a_child(id PK, parent_id REFERENCES z_parent(id)) on both nodes.
Insert (1) into z_parent and (10, 1) into a_child on A.
Run a full snapshot from A to an empty B, the rebuilt-peer or cap-prune path.

Result on B: parents=1 children=0, one dead-letter a_child: SQLite Error 19: 'FOREIGN KEY constraint failed', watermark advanced to 2. A self-referencing table (node(id, parent_id REFERENCES node(id)) with rows 2 and 10→2) loses row 10 the same way. The child rows are gone from
B permanently, and the pair reports itself converged.

No current consumer has an FK on a replicated table (per the 0.3.3 notes), so this is latent. The
first consumer to add one loses data on every snapshot resync.

Recommendation

Choose one:

Apply snapshot batches with PRAGMA defer_foreign_keys = ON inside the apply transaction, which
checks FKs at commit. This only helps if parent and child land in the same transaction, which
they do not today across batches. More robustly, apply the whole snapshot with FK enforcement off
(PRAGMA foreign_keys=OFF on the snapshot apply connection, set outside a transaction) and run
PRAGMA foreign_key_check at SnapshotComplete to log or dead-letter any true orphans.
Order the snapshot topologically by FK dependency, parents first. Even then a self-referencing
table still needs deferral.

Also add a snapshot test with a non-cascading FK, and consider not dead-lettering
SQLITE_CONSTRAINT_FOREIGNKEY silently on the snapshot path.

Resolution

Unresolved.

From the scadaproj#15 shared-library review (`code-reviews/LocalDb/findings.md`, **LocalDb-002**), verified at 91fb2b4. Update the finding's Resolution row when fixed. | | | |--|--| | Severity | Medium | | Priority | P2 | | Category | Correctness & logic bugs | | Status | Open | | Location | `ZB.MOM.WW.LocalDb/src/ZB.MOM.WW.LocalDb.Replication/Internal/SnapshotStreamer.cs:79` | **Description** `SnapshotStreamer.SendAsync` streams tables in ordinal name order (`.OrderBy(t => t.Name, StringComparer.Ordinal)`, line 79–81). Within a table it pages `ORDER BY rv.pk_json` (line 196), which is text order: `{"id":10}` sorts before `{"id":2}`. The receiver applies through `LwwApplier` on a connection with `PRAGMA foreign_keys=ON`. An immediate FK violation is a `SqliteException`, and `LwwApplier.cs:116` treats it as per-entry poison: dead-letter, keep going. `SnapshotApplier.OnCompleteAsync` then moves the watermark to `as_of_seq`, and the sender clears its snapshot debt on the `SnapshotAck`. Nothing ever re-sends the dead-lettered rows. Registration explicitly allows these schemas. `EnsureNoInboundActionOnDelete` rejects only CASCADE / SET NULL / SET DEFAULT, and README §Limitations recommends `NO ACTION`/`RESTRICT`. So a replicated parent/child pair with a default FK is a supported configuration that the delta path handles (seq order matches commit order) but the snapshot path does not. **Failure scenario** Reproduced in a scratch test: 1. Register `z_parent(id PK)` and `a_child(id PK, parent_id REFERENCES z_parent(id))` on both nodes. 2. Insert `(1)` into `z_parent` and `(10, 1)` into `a_child` on A. 3. Run a full snapshot from A to an empty B, the rebuilt-peer or cap-prune path. Result on B: `parents=1 children=0`, one dead-letter `a_child: SQLite Error 19: 'FOREIGN KEY constraint failed'`, watermark advanced to 2. A self-referencing table (`node(id, parent_id REFERENCES node(id))` with rows 2 and 10→2) loses row 10 the same way. The child rows are gone from B permanently, and the pair reports itself converged. No current consumer has an FK on a replicated table (per the 0.3.3 notes), so this is latent. The first consumer to add one loses data on every snapshot resync. **Recommendation** Choose one: - Apply snapshot batches with `PRAGMA defer_foreign_keys = ON` inside the apply transaction, which checks FKs at commit. This only helps if parent and child land in the same transaction, which they do not today across batches. More robustly, apply the whole snapshot with FK enforcement off (`PRAGMA foreign_keys=OFF` on the snapshot apply connection, set outside a transaction) and run `PRAGMA foreign_key_check` at `SnapshotComplete` to log or dead-letter any true orphans. - Order the snapshot topologically by FK dependency, parents first. Even then a self-referencing table still needs deferral. Also add a snapshot test with a non-cascading FK, and consider not dead-lettering `SQLITE_CONSTRAINT_FOREIGNKEY` silently on the snapshot path. **Resolution** _Unresolved._

dohertj2 added the type:review-finding priority:p2 labels 2026-11-09 14:39:21 -05:00

dohertj2 commented

2026-11-09 15:03:19 -05:00

Fixed on main 4af0ea8 (LocalDb-002 → Resolved): 2-node test reproduced both child rows dead-lettered with the watermark moving past them. Sender now streams parent tables first; the receiver holds back rows that fail only their FK check, retries them as rows arrive, and dead-letters true orphans at snapshot end. 4 new tests fail without the fix. Mixed-version note: an older receiver still loses self-referencing rows arriving child-first (documented). Evidence: LocalDb 236/236; Auth 344 passed / 78 skipped (live SQL/LDAP gated) after rebase; code-reviews --check consistent.

Fixed on main 4af0ea8 (LocalDb-002 → Resolved): 2-node test reproduced both child rows dead-lettered with the watermark moving past them. Sender now streams parent tables first; the receiver holds back rows that fail only their FK check, retries them as rows arrive, and dead-letters true orphans at snapshot end. 4 new tests fail without the fix. Mixed-version note: an older receiver still loses self-referencing rows arriving child-first (documented). Evidence: LocalDb 236/236; Auth 344 passed / 78 skipped (live SQL/LDAP gated) after rebase; code-reviews --check consistent.

dohertj2 closed this issue

2026-11-09 15:03:19 -05:00

dohertj2 referenced this issue

2026-11-09 15:03:20 -05:00

[LocalDb/Auth] Follow-ups: snapshot dead-letters uncounted, unbounded held-back rows, duplicate app sync interceptors, GC-forest sAMAccountName match #50

dohertj2 referenced this issue

2026-11-12 14:08:40 -05:00

[LocalDb] Publish 0.3.7 and adopt in ScadaBridge + OtOpcUa (tombstone fix #16) #53

Sign in to join this conversation.