Skip to content

fix(export): use one snapshot and validate local references - #833

Open
ansonnmm wants to merge 1 commit into
deeplethe:devfrom
ansonnmm:upstream-pr-export-single-snapshot
Open

ansonnmm wants to merge 1 commit into
deeplethe:devfrom
ansonnmm:upstream-pr-export-single-snapshot

Conversation

@ansonnmm

@ansonnmm ansonnmm commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

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

  • Focused store regressions: 15/15 — merged targets excluded and
    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.
  • New concurrency regression: two concurrent authenticated exports on a
    max_connections=2 pool (the supported minimum) each record their
    kb.exported audit and complete; with the audit-after-transaction
    ordering, both audits starve on a third connection that never comes.
  • cargo test -p utopia-store: all green. cargo test -p utopia-server
    mcp suite: 25/25 including authenticated export round-trips.
  • cargo fmt --all --check and cargo clippy --tests -- -D warnings
    clean for utopia-store and utopia-server.
  • Representative cost (same DB, same process, 1 warmup + 3 alternating
    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

  • Each active export holds one long-lived database connection for the
    export's duration. Audit recording completes before that transaction,
    so an export never needs a second connection while streaming.
  • Malformed ledgers may now fail with a 422 instead of producing corrupt
    output; exports of clean ledgers are unchanged.
  • Export read functions take &mut Transaction instead of &PgPool;
    the HTTP contract and the emitted output are unchanged.
  • The long-lived REPEATABLE READ snapshot also pins PostgreSQL backend_xmin for 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.

@ansonnmm

Copy link
Copy Markdown
Contributor Author

Closing pending author's final review of the PR set — will reopen once the series is finalized.

@ansonnmm ansonnmm closed this Sep 20, 2026
@ansonnmm ansonnmm changed the title An export reads one snapshot and vouches for every reference it resolves fix(export): use one snapshot and validate local references Sep 20, 2026
@ansonnmm ansonnmm reopened this Sep 21, 2026
@ansonnmm
ansonnmm force-pushed the upstream-pr-export-single-snapshot branch 2 times, most recently from 935b582 to 5891a5d Compare September 21, 2026 05:19
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>
@ansonnmm
ansonnmm force-pushed the upstream-pr-export-single-snapshot branch from 5891a5d to 2b37e1a Compare September 21, 2026 05:42
@WaylandYang

Copy link
Copy Markdown
Contributor

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:

  • Preflight before any bytes. A 422 ahead of the stream beats a 200 with a corrupt body. Once the first byte is out there is no honest way to retract it, so the boundary has to be before it.
  • Audit recording stays ahead of the snapshot transaction. Finding this with a max_connections=2 pool and two concurrent exports is the kind of thing that only shows up under the minimum supported configuration — the audit starving on a third connection that never comes is a deadlock waiting for a busy day.
  • The cost is measured rather than asserted: 273 ms to 303 ms median, with the delta attributed to the preflight and the emitted bytes identical.

Two things worth raising.

1. The edge list now exists in three places, by hand. provenance_integrity enumerates the same edges as §0 of migration 0070 — evidence.chunk, evidence.document, derivation.premise_fact, qualifier.*, fact.subject, and the rest — and the schema itself expresses them a third time as composite keys and triggers. Adding a reference column next month means remembering all three. Two of them are SQL text that nothing verifies against the catalog.

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 provenance_integrity too, not only in the schema. Otherwise the export's fail-closed guarantee quietly stops covering edges added later, which is the same failure this PR exists to prevent.

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 backend_xmin, so vacuum cannot reclaim tuples newer than it anywhere in that database while the export runs. At the measured scale this is nothing — 300 ms. Extrapolating your own figures, a base a hundred times larger is tens of seconds, and that is still fine. It is worth a sentence in the record or the ops notes rather than a change here, so that whoever first exports a very large base on a busy instance knows why the dead-tuple count moved.

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.

@ansonnmm

Copy link
Copy Markdown
Contributor Author

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 backend_xmin / vacuum implication of the long-lived snapshot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants