diff --git a/openspec/changes/rebuild-graph-without-blocking-writes/.openspec.yaml b/openspec/changes/rebuild-graph-without-blocking-writes/.openspec.yaml new file mode 100644 index 00000000..8e7013b8 --- /dev/null +++ b/openspec/changes/rebuild-graph-without-blocking-writes/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-27 diff --git a/openspec/changes/rebuild-graph-without-blocking-writes/design.md b/openspec/changes/rebuild-graph-without-blocking-writes/design.md new file mode 100644 index 00000000..a8f7a1f2 --- /dev/null +++ b/openspec/changes/rebuild-graph-without-blocking-writes/design.md @@ -0,0 +1,105 @@ +## Context + +`EpistemicGraphIndex.refresh_paths` takes the vault mutation boundary and calls +`_refresh_paths_locked`, which escalates to `_rebuild_all_locked()` whenever +`available()` is false. `_rebuild_all_locked` runs a stabilization loop: +snapshot `_disk_vault_freshness`, run `_rebuild_all_pass`, and accept the result +only if freshness is unchanged; otherwise retry, up to +`REBUILD_STABILIZATION_ATTEMPTS`, then fail and mark the graph unavailable. + +Two properties of the current implementation matter here. + +`_rebuild_all_pass` mutates the **live** sidecar: it deletes every row from +`graph_edges`, `graph_nodes`, and `graph_parent_refs` before refilling. A reader +arriving mid-pass would see an empty or partial graph. That is safe today only +because the boundary excludes everyone for the whole rebuild. + +The stabilization loop is therefore currently a guard against *out-of-band* +edits — a user editing files directly in Obsidian — rather than against +concurrent Exomem writes, which the boundary already prevents. + +Moving the rebuild outside the boundary changes the status of both: the wipe +becomes visible, and the stabilization loop becomes the primary consistency +mechanism rather than a backstop. + +## Goals / Non-Goals + +**Goals:** + +- A full rebuild must not block unrelated vault mutations for its duration. +- A reader must never observe a partially rebuilt graph. +- Preserve the write-path contract: a write against an unusable sidecar returns + with the graph built and available. +- Preserve the failure contract: a rebuild that cannot observe a stable vault + marks the graph unavailable rather than publishing a bad one. + +**Non-Goals:** + +- Changing the sidecar schema or file format. +- Making incremental (`available()`-true) refresh faster. +- Removing the escalation itself. #346 established that it is load-bearing. +- Reducing how often the sidecar is invalidated. Fewer `SCHEMA_VERSION` bumps or + a registry-hash change that does not invalidate would both reduce exposure, + but they are separate questions from how a rebuild behaves. + +## Decisions + +Build into a temporary database in the sidecar's own directory, then swap. Same +directory so the publish step is an atomic rename on one filesystem. The rebuild +passes run against the temp database with no boundary held; only the swap takes +it. The boundary hold drops from the length of a full rebuild — 32 s at 2,000 +pages, 172 s at 8,000 — to a rename. + +Re-verify freshness under the boundary immediately before the swap. Outside the +boundary the vault can change during the final pass, so the pre-swap check is +what makes the published graph trustworthy. If freshness moved, the rebuild +retries within its existing attempt budget rather than publishing. + +Make rebuilds single-flight per vault. Today the boundary serialises them +implicitly: the second writer blocks, and by the time it runs the graph is +available so it takes the incremental path. Once rebuilds no longer hold the +boundary, N concurrent writers would otherwise each start a full rebuild. A +rebuild-in-progress marker keeps one running and lets the others wait for its +result. + +Keep `_mark_unavailable()` on stabilization failure. The failure contract is +unchanged; only the location of the work moves. + +Sweep abandoned temp databases in `reconcile`. A crash mid-rebuild leaves a temp +file that no longer has an owner. Reconcile already walks sidecar state and +already owns `rebuild_all`, so it is the natural place. Temp databases use a +reserved name prefix so a sweep cannot mistake one for the live sidecar. + +## Risks / Trade-offs + +- [The stabilization budget may be too small outside the boundary] → Concurrent + Exomem writes can now perturb freshness mid-pass, where previously only + out-of-band edits could. If `REBUILD_STABILIZATION_ATTEMPTS` proves + insufficient under sustained writes, a rebuild that used to succeed slowly + would fail instead. Covered by a test that writes continuously during a + rebuild; the attempt budget may need raising with the sweep. +- [Disk usage doubles transiently during a rebuild] → One extra sidecar-sized + file for the duration. Acceptable, and bounded by the sweep. +- [A waiting writer still waits] → Single-flight means a concurrent writer that + needs the graph still waits for the in-flight rebuild. It no longer blocks + *unrelated* mutations, which is the reported harm, but the first writer after + invalidation still pays the rebuild cost. Reducing that is a separate problem. +- [Atomic rename semantics on Windows] → Replacing an open SQLite file differs + from POSIX. The swap must tolerate readers holding the old file, and the + implementation must verify behaviour on the Windows service path rather than + assuming POSIX rename. + +## Migration Plan + +No data migration. The first rebuild after this change writes a temp database +and swaps it; existing sidecars are read normally until invalidated. Roll back +by restoring the in-boundary rebuild, which changes no on-disk format. + +## Open Questions + +- What should a writer do while a rebuild is in flight — block on the + single-flight result, or return deferred and let the rebuild cover its path? + Deferring keeps the write bounded but means the writer's own edit may not be + in the published graph, which the current contract does not allow. +- Does `REBUILD_STABILIZATION_ATTEMPTS` need to rise now that concurrent Exomem + writes can perturb freshness, and what is the right ceiling before failing? diff --git a/openspec/changes/rebuild-graph-without-blocking-writes/proposal.md b/openspec/changes/rebuild-graph-without-blocking-writes/proposal.md new file mode 100644 index 00000000..ddd3ff67 --- /dev/null +++ b/openspec/changes/rebuild-graph-without-blocking-writes/proposal.md @@ -0,0 +1,65 @@ +## Why + +A full epistemic graph rebuild runs inside the vault mutation boundary, so it +blocks every other vault mutation for its entire duration. Measured on a +maintainer workstation against synthetic vaults, on a single `upsert_after_write` +with no usable sidecar: + +| vault | first write | steady-state write | ratio | +|---|---|---|---| +| 500 pages | 7,728 ms | 315 ms | 24.6x | +| 2,000 pages | 32,382 ms | 1,186 ms | 27.3x | + +CI's write-latency benchmark records the same shape at larger scale: +`hold_ms=39092` over 2,000 pages and `hold_ms=172205` over 8,000. + +This is not an edge case. `_open_read_snapshot` treats the sidecar as unusable +whenever `schema_version`, `core_registry_version`, or `extension_registry_hash` +disagrees with the running build, so a full rebuild is triggered by: + +- any release that bumps `SCHEMA_VERSION` — at 7 today, bumped seven times; +- any change to the relation registry, including a user adding or editing one + extension relation; +- a sidecar that is missing, deleted, or corrupt. + +The first write after any of those stalls for tens of seconds on a +maintainer-sized vault, and every concurrent writer is blocked behind it. A +client that gives up meanwhile sees a timeout on a write that then lands. + +An earlier attempt (#346) removed the escalation so writes deferred to +`reconcile`. That was wrong and CI rejected it: +`test_refresh_missing_sidecar_routes_to_full_rebuild` requires a write against a +missing sidecar to index the whole vault and leave the graph available. The +rebuild has to happen; it must stop holding the boundary while it does. + +## What Changes + +- Build a full graph rebuild into a temporary database outside the vault + mutation boundary, then acquire the boundary only to swap it into place. +- Make concurrent rebuild requests single-flight, so N blocked writers do not + each start their own full-vault rebuild. +- Keep the existing stabilization contract: a rebuild that cannot observe a + stable vault still fails and marks the graph unavailable. +- Preserve the write-path contract that a write against an unusable sidecar + leaves the graph built and available. +- Sweep abandoned temporary rebuild databases during reconcile. + +## Capabilities + +### New Capabilities + +- `graph-rebuild-availability`: A full epistemic graph rebuild does not block + unrelated vault mutations, and never exposes a partially rebuilt graph. + +### Modified Capabilities + +None. + +## Impact + +Affected areas are `src/exomem/epistemic_graph.py`, `src/exomem/reconcile.py` +(temp sweep), and the epistemic graph freshness, boundary, and semantic unit +graph tests. + +No MCP tool schema, vault format, OAuth, or stdio behavior changes. The sidecar +file format is unchanged; only how and where it is produced changes. diff --git a/openspec/changes/rebuild-graph-without-blocking-writes/specs/graph-rebuild-availability/spec.md b/openspec/changes/rebuild-graph-without-blocking-writes/specs/graph-rebuild-availability/spec.md new file mode 100644 index 00000000..8f8b1314 --- /dev/null +++ b/openspec/changes/rebuild-graph-without-blocking-writes/specs/graph-rebuild-availability/spec.md @@ -0,0 +1,58 @@ +## ADDED Requirements + +### Requirement: A full rebuild does not hold the vault mutation boundary +The system SHALL perform full epistemic graph rebuild work without holding the +vault mutation boundary, acquiring it only to publish the result. + +#### Scenario: Unrelated mutation during a rebuild +- **WHEN** a full graph rebuild is in progress +- **AND** an unrelated vault mutation is requested +- **THEN** that mutation proceeds without waiting for the rebuild to finish + +#### Scenario: Boundary hold is bounded by the swap +- **WHEN** a full graph rebuild completes +- **THEN** the vault mutation boundary is held only for the publish step +- **AND** the hold does not scale with vault size + +### Requirement: A partially rebuilt graph is never observable +The system SHALL publish a rebuilt graph as a single atomic replacement. + +#### Scenario: Read during a rebuild +- **WHEN** a graph read occurs while a full rebuild is in progress +- **THEN** it observes either the previous graph state or the fully rebuilt one +- **AND** never an empty or partially populated graph + +#### Scenario: Crash during a rebuild +- **WHEN** the process terminates during a full rebuild +- **THEN** the live sidecar is left in its pre-rebuild state +- **AND** the abandoned temporary database is removed by the next reconcile + +### Requirement: Concurrent rebuild requests are single-flight +The system SHALL run at most one full graph rebuild per vault at a time. + +#### Scenario: Several writers arrive with an unusable sidecar +- **WHEN** multiple writes require a rebuild concurrently +- **THEN** exactly one full rebuild runs +- **AND** the remaining requests resolve from that rebuild's outcome + +### Requirement: Published graphs reflect a stable vault +The system SHALL verify vault freshness immediately before publishing a rebuilt +graph, while holding the vault mutation boundary. + +#### Scenario: Vault changes during the final pass +- **WHEN** vault freshness changes between the start of a rebuild pass and the publish step +- **THEN** the rebuild is retried rather than published + +#### Scenario: Rebuild cannot observe a stable vault +- **WHEN** a rebuild exhausts its stabilization attempts +- **THEN** it does not publish a graph +- **AND** the graph is marked unavailable + +### Requirement: The write path still yields an available graph +The system SHALL preserve the existing write-path contract when the sidecar is +unusable. + +#### Scenario: Write against a missing sidecar +- **WHEN** a write occurs and the graph sidecar is missing, schema-mismatched, or registry-invalidated +- **THEN** the graph is rebuilt across the whole vault +- **AND** the graph reports itself available once the write returns diff --git a/openspec/changes/rebuild-graph-without-blocking-writes/tasks.md b/openspec/changes/rebuild-graph-without-blocking-writes/tasks.md new file mode 100644 index 00000000..9ce05e2c --- /dev/null +++ b/openspec/changes/rebuild-graph-without-blocking-writes/tasks.md @@ -0,0 +1,70 @@ +## 1. Regression coverage + +- [ ] 1.1 Prove an unrelated mutation is not blocked while a full rebuild runs. +- [ ] 1.2 Prove a reader during a rebuild sees the old graph or the new one, never a partial one. +- [ ] 1.3 Prove concurrent rebuild requests run exactly one rebuild. +- [ ] 1.4 Prove the existing write-path contract still holds: a write against an + unusable sidecar indexes the whole vault and leaves it available + (the contract #346 broke). +- [ ] 1.5 Prove a vault change during the final pass retries rather than publishes. +- [ ] 1.6 Prove exhausted stabilization still marks the graph unavailable. + +## 2. Rebuild off the boundary + +- [ ] 2.1 Build rebuild passes into a temporary database beside the sidecar. +- [ ] 2.2 Publish by atomic replacement under a boundary hold scoped to the swap. +- [ ] 2.3 Re-verify freshness under the boundary immediately before publishing. +- [ ] 2.4 Verify the replacement behaves correctly on the Windows service path, + where replacing an open SQLite file does not follow POSIX rename semantics. + +## 3. Single-flight + +- [ ] 3.1 Serialize rebuilds per vault so concurrent writers do not each start one. +- [ ] 3.2 Resolve waiting requests from the in-flight rebuild's outcome. +- [ ] 3.3 Settle the open question: whether a waiting writer blocks or defers, + and reconcile that with the contract in 1.4. + +## 4. Housekeeping + +- [ ] 4.1 Name temporary rebuild databases with a reserved prefix. +- [ ] 4.2 Sweep abandoned temporary databases during reconcile. + +## 5. Verification + +- [ ] 5.1 Run the epistemic graph, freshness, boundary, semantic unit graph, and + reconcile suites, plus Ruff and strict OpenSpec validation. +- [ ] 5.2 Re-run the reproduction harness and record the new first-write cost at + 500 and 2,000 pages against the pre-change baseline. +- [ ] 5.3 Confirm under a concurrent-write load that the stabilization budget is + still sufficient, and raise `REBUILD_STABILIZATION_ATTEMPTS` if not. +- [ ] 5.4 Run the lean suite. + +### Baseline to beat + +Measured on a maintainer workstation against synthetic dense vaults, on main, +one `upsert_after_write` with no usable sidecar: + +| vault | first write | steady-state write | ratio | +|---|---|---|---| +| 500 pages | 7,727.8 ms | 314.5 ms | 24.6x | +| 2,000 pages | 32,381.5 ms | 1,186.2 ms | 27.3x | + +with `vault mutation boundary held too long ... +operation=epistemic_graph_refresh_paths holder_kind=graph hold_ms=32371.03`. + +CI's write-latency benchmark shows `hold_ms=39092` at 2,000 pages and +`hold_ms=172205` at 8,000. + +The target is that the *boundary hold* stops scaling with vault size. The first +writer after invalidation still pays the rebuild; what must stop is every other +mutation waiting behind it. + +### Prior art + +#346 attempted to fix this by removing the escalation so writes deferred to +reconcile. CI rejected it: +`test_refresh_missing_sidecar_routes_to_full_rebuild` requires a write against a +missing sidecar to index the whole vault and leave it available, and three other +tests depend on the same escalation — the spawned-mutator boundary contract, +forced re-resolution after a relation-registry change, and the schema-v3 sidecar +migration. Task 1.4 exists to keep that contract pinned while the rebuild moves. diff --git a/scripts/repro_graph_rebuild_hold.py b/scripts/repro_graph_rebuild_hold.py new file mode 100644 index 00000000..6ebbb224 --- /dev/null +++ b/scripts/repro_graph_rebuild_hold.py @@ -0,0 +1,67 @@ +"""Measure the mutation-boundary hold caused by one write against a missing graph sidecar. + +Reproduces on main's code path (no fix applied). Reports, per vault size: + - how long a SINGLE `upsert_after_write` takes when the sidecar is absent + - how long the same write takes once the sidecar exists (the healthy case) + +The gap between the two is the stall a user eats on the first write after the +sidecar is missing, deleted, schema-bumped, or registry-invalidated. +""" + +from __future__ import annotations + +import os +import sys +import tempfile +import time +from pathlib import Path + +ROOT = Path(__file__).resolve() +REPO = Path(sys.argv[1]) if len(sys.argv) > 1 else None +if REPO is None: + raise SystemExit("usage: repro_graph_hold.py [sizes...]") +sys.path.insert(0, str(REPO / "src")) +sys.path.insert(0, str(REPO / "scripts")) + +for name in ( + "EXOMEM_DISABLE_EMBEDDINGS", + "EXOMEM_DISABLE_CLIP", + "EXOMEM_DISABLE_MEDIA_EXTRACTION", + "EXOMEM_DISABLE_RANKING", +): + os.environ[name] = "1" + +from synth_vault import gen_dense_vault # noqa: E402 + +from exomem import epistemic_graph # noqa: E402 +from exomem.kbdir import kb_dirname # noqa: E402 + +SIZES = [int(a) for a in sys.argv[2:]] or [500, 2000] + + +def one_write(vault: Path, target: Path) -> float: + started = time.perf_counter() + epistemic_graph.upsert_after_write(vault, [target]) + return (time.perf_counter() - started) * 1000.0 + + +for size in SIZES: + with tempfile.TemporaryDirectory(prefix=f"graph-hold-{size}-") as temp: + vault = Path(temp) / "vault" + vault.mkdir(parents=True) + gen_dense_vault(vault, size, links_per_note=3) + + target = next((vault / kb_dirname()).rglob("*.md")) + sidecar = epistemic_graph.sidecar_path(vault) + assert not sidecar.exists(), "precondition: no sidecar yet" + + cold_ms = one_write(vault, target) + built = sidecar.exists() + warm_ms = one_write(vault, target) + + print( + f"pages={size:<6} first_write_missing_sidecar={cold_ms:9.1f}ms " + f"subsequent_write={warm_ms:7.1f}ms " + f"ratio={cold_ms / max(warm_ms, 0.001):7.1f}x sidecar_built={built}", + flush=True, + )