Conversation
|
Closing pending author's final review of the PR set — will reopen once the series is finalized. |
935b582 to
5891a5d
Compare
An export previously took a fresh pool connection per page, so each page of a streamed export read a different moment of the ledger: a row committed after the vocabulary was sent could still appear as a reference in a later page, minting a local IRI for something the file never introduces. Read the whole export inside a single read-only REPEATABLE READ transaction: preflight, vocabulary, and every paged read share one snapshot, and an error or a client disconnect drops the transaction and returns the connection. Validate the references the export resolves against that same snapshot. Each page selects a referenced row's kb_id atomically with the row itself, and a preflight scan refuses the export before any bytes are sent when provenance is malformed: cross-KB rows, dangling pointers left behind by restores that bypassed the write-path triggers, and references to merged entities that are validly same-KB but absent from the exported set. Migration 0070 already rejects cross-KB writes; these checks remain necessary because they defend the read path against pre-existing malformed state and against references that are schema-legal yet unrepresentable in the export. Audit recording keeps its existing place before the export begins. The snapshot transaction must be the last thing the handler acquires: it lives for the whole stream, so asking the pool for a second connection while holding it can starve concurrent exports on the supported minimum pool size. Signed-off-by: Anson <310461893+ansonnmm@users.noreply.github.com>
5891a5d to
2b37e1a
Compare
|
The framing is right and it is the part I would have missed: 0070 proves ownership at write time, and a streamed export reads a moving target page by page. Those are different guarantees, and the second one is not implied by the first. The merged-entity case sharpens it — a valid same-KB row that the export filter deliberately omits, so a reference to it mints a local IRI for an identity the file never carries. That is a dangling reference no schema constraint can catch, because nothing about the row is malformed. Three things I checked and think are right:
Two things worth raising. 1. The edge list now exists in three places, by hand. This is #846 widening rather than a fault in this PR, but it widens it materially: the catalog-driven completeness test I proposed there should now assert that every covered edge appears in 2. Each export holds a REPEATABLE READ snapshot for its whole duration. You name the connection cost, which is the one that bites first. The other half is that an open snapshot pins Neither blocks. CI is green on all four checks. One convention point, same as on #832: the description should answer which record this implements or overturns. My reading is that it extends 0020's contract — the export is where these states are refused — without changing a decision, but the question should be answered rather than left for the reader. |
|
Thanks — agreed on both follow-ups. I’ll keep the catalog/preflight drift problem in #846 rather than widening this PR. I’ve updated the description to state that this extends ADR 0020 without changing its decision, and to record the |
Problem
An export spans multiple database snapshots: each paged read takes a
fresh pool connection, so successive pages of one response see different
moments of the ledger. A row committed mid-export can appear as a
reference in a later page while the row that introduced it never made it
into the file — a local IRI minted for an identity the export never
carries. There is also no same-snapshot, fail-closed read boundary for
the references the serializer resolves.
Why this is a generic Utopia bug or contract gap
Migration 0070 solved persisted schema ownership: new writes cannot
cross a knowledge base. That does not by itself prove that one streamed
export is a coherent point-in-time ledger view — schema guarantees hold
at write time, while the export reads a moving target page by page — nor
that every export-resolved identity survives export filtering in the same
snapshot: a merged entity is a valid same-KB row that is deliberately
absent from the exported entity set, and a reference to it mints a
dangling local IRI. Restores and operator repair paths can also bypass
user triggers, leaving pre-existing malformed rows that write-time
constraints never see. The export, not the schema, is where these states
must be refused.
Decision record: This extends ADR 0020 ("An auditor reads it without us"): the RDF export remains the supported external read contract, and this change makes one streamed export a coherent point-in-time view and refuses references the exported graph cannot honestly represent. It does not overturn or introduce a new decision.
Fix
One read-only REPEATABLE READ transaction across the complete export —
preflight, vocabulary, and every paged read share one snapshot — plus
the remaining same-snapshot, fail-closed reference checks. Each page
selects a referenced row's kb_id atomically with the row itself; a
preflight scan refuses the export before any bytes are sent when
provenance is malformed (cross-KB, dangling, or pointing at a row outside
the exported set). Audit recording keeps its pre-existing position before
the snapshot transaction is acquired, so an export never asks the pool
for a second connection while holding the stream's.
Regression evidence
references to them refused, replica-mode malformed rows refused by
preflight and by page reads, a mid-stream commit stays outside the
snapshot, NIL-id rows reach the first page, dangling pointers refuse.
max_connections=2pool (the supported minimum) each record theirkb.exportedaudit and complete; with the audit-after-transactionordering, both audits starve on a third connection that never comes.
cargo test -p utopia-store: all green.cargo test -p utopia-servermcp suite: 25/25 including authenticated export round-trips.
cargo fmt --all --checkandcargo clippy --tests -- -D warningsclean for utopia-store and utopia-server.
runs each; 1000 documents, 3000 chunks, 3000 facts, 3000 evidence
rows, 500 derived facts → a 3.2 MB Turtle export): median 273 ms
before vs 303 ms after (ranges 267–274 / 299–351 ms). The delta is
dominated by the one-shot preflight scan (~11–53 ms here); emitted
bytes are identical.
Compatibility / risk
export's duration. Audit recording completes before that transaction,
so an export never needs a second connection while streaming.
output; exports of clean ledgers are unchanged.
&mut Transactioninstead of&PgPool;the HTTP contract and the emitted output are unchanged.
REPEATABLE READsnapshot also pins PostgreSQLbackend_xminfor the export duration, so very large exports can temporarily delay vacuum reclamation of newer dead tuples. At the measured 3.2 MB fixture this is ~300 ms; larger exports extend that window proportionally to their runtime.