store: per-process migration memoization (#000026 Phase 1)
`connect()` used to run executescript(SCHEMA_SQL) + 7 forward- migration probes on every open. Profile of `who wrote virt-back?` on 38 GB of real shards (warm cache) showed 588 connect() calls per query, each running the full probe sequence — 10,623 total SQLite executes. Migrations are forward-only and idempotent within a code version, so once we've run them on a path in this process there's no work to do on subsequent opens. Cache shape: `set[str]` keyed by `str(Path(p).resolve())`. Migration block runs once per (path, process); subsequent calls on the same shard skip it entirely. Per-connection PRAGMAs (foreign_keys=ON, synchronous=NORMAL, cache_size, temp_store, mmap_size) still run every time — SQLite scopes foreign_keys per-connection and our schema's FK CASCADE behavior depends on it. That's why `PRAGMA foreign_keys = ON` moved out of the cached SCHEMA_SQL block into the always-run pragma section. Cache invalidation: explicit only. `store.invalidate_migration_cache(path)` for callers who replace a shard at the same path (snapshot-restore flows). `_clear_migration_ cache()` for tests. We don't auto-detect file replacement — (dev, inode) is unreliable under tmpfs inode reuse, and (mtime, size) drifts naturally as SQLite operates on the file (WAL checkpoints, page growth). Path-only with explicit invalidation is the honest contract. Re-profile (same query, same shards, warm cache): metric before after _migrate_* (each function) 586 7 ← per shard executescript 586 7 SQLite executes 10,623 3,687 (-65%) wall (warm) 14.5 s 13.4 s The warm-cache wall delta is small because the probes were many- but-cheap; residual cost lives in FTS5 search (6.7 s) and synonym_expand (2.8 s, both separate concerns). The 65% execute drop is the cold-cache win — each redundant executescript() had been triggering disk reads at the 75 s scale the reviewer reported. Tests (6, all green): first connect runs all 7 probes, second connect runs zero, schema integrity preserved across re-opens, explicit invalidation re-probes, distinct paths each get one probe, clear-cache helper works. Full suite: 1306 passed, 36 skipped. Found and fixed an FK CASCADE regression mid-implementation: PRAGMA foreign_keys = ON was inside SCHEMA_SQL, so memoization was silently turning it off on subsequent opens. test_burn_doc.py caught it. Moved to the per-connection pragma block. Ticket #000026 status: Phase 1 landed; Phase 2 (baseline artifact) and Phase 3 (warrant-quality finding) queued.
This commit is contained in:
parent
a81494a979
commit
ec92ebc575
4 changed files with 282 additions and 13 deletions
|
|
@ -463,6 +463,38 @@ CREATE INDEX IF NOT EXISTS idx_adapter_loss_chunk
|
|||
"""
|
||||
|
||||
|
||||
# Process-local migration memoization (#000026 Phase 1). Keyed by
|
||||
# resolved path. Migrations are forward-only and idempotent within
|
||||
# a code version, so once we've run them on a path in this process,
|
||||
# we skip on every subsequent `connect()`. Pre-fix: 588 `connect()`
|
||||
# calls per typical query, each running 7 migration probes. Post-
|
||||
# fix: 1 probe sequence per (path, process), then skipped.
|
||||
#
|
||||
# Tradeoff: this assumes a path identifies a single SQLite file for
|
||||
# the lifetime of the process. If a caller replaces the file at the
|
||||
# same path (e.g. a test deletes + recreates), they MUST call
|
||||
# `invalidate_migration_cache(path)` (or `_clear_migration_cache()`).
|
||||
# We don't try to detect replacement automatically — (dev, inode)
|
||||
# fails under tmpfs inode reuse, and (mtime, size) drift naturally
|
||||
# as SQLite operates on the file (WAL checkpoints, page growth).
|
||||
# Path-only with explicit invalidation is honest about the contract.
|
||||
_MIGRATED_SHARDS: set[str] = set()
|
||||
|
||||
|
||||
def _clear_migration_cache() -> None:
|
||||
"""Drop all memoized migration claims. Mostly for tests, but
|
||||
also for callers that knowingly replace a shard file at the same
|
||||
path (e.g. snapshot-restore flows)."""
|
||||
_MIGRATED_SHARDS.clear()
|
||||
|
||||
|
||||
def invalidate_migration_cache(db_path: Path | str) -> None:
|
||||
"""Drop the memoized migration claim for one path. Call this
|
||||
after replacing the underlying file; the next `connect()` will
|
||||
re-run the full migration probe sequence."""
|
||||
_MIGRATED_SHARDS.discard(str(Path(db_path).resolve()))
|
||||
|
||||
|
||||
def connect(db_path: Path | str = DEFAULT_DB_PATH) -> sqlite3.Connection:
|
||||
"""Open a writable connection, creating the parent dir + schema if needed.
|
||||
|
||||
|
|
@ -472,19 +504,34 @@ def connect(db_path: Path | str = DEFAULT_DB_PATH) -> sqlite3.Connection:
|
|||
- cache_size=-65536 = 64 MB page cache (reduces re-reads).
|
||||
- temp_store=MEMORY keeps temp tables in RAM (no /tmp churn).
|
||||
- mmap_size=256 MB lets reads come from page-cache without read() syscalls.
|
||||
|
||||
Migration probes (executescript(SCHEMA_SQL) + 7 forward migrations)
|
||||
run once per (physical file, process). Subsequent `connect()` calls
|
||||
on the same shard skip migration entirely — see #000026 Phase 1.
|
||||
"""
|
||||
p = Path(db_path)
|
||||
p.parent.mkdir(parents=True, exist_ok=True)
|
||||
conn = sqlite3.connect(p, isolation_level=None) # autocommit; we'll BEGIN manually
|
||||
conn.row_factory = sqlite3.Row
|
||||
conn.executescript(SCHEMA_SQL)
|
||||
_migrate_audit_mode(conn)
|
||||
_migrate_mesh_peer_chains(conn)
|
||||
_migrate_document_http_meta(conn)
|
||||
_migrate_selfmodel_tables(conn)
|
||||
_migrate_capital_ledger(conn)
|
||||
_migrate_memory_root(conn)
|
||||
_migrate_adapter_loss_reports(conn)
|
||||
|
||||
cache_key = str(p.resolve())
|
||||
if cache_key not in _MIGRATED_SHARDS:
|
||||
conn.executescript(SCHEMA_SQL)
|
||||
_migrate_audit_mode(conn)
|
||||
_migrate_mesh_peer_chains(conn)
|
||||
_migrate_document_http_meta(conn)
|
||||
_migrate_selfmodel_tables(conn)
|
||||
_migrate_capital_ledger(conn)
|
||||
_migrate_memory_root(conn)
|
||||
_migrate_adapter_loss_reports(conn)
|
||||
_MIGRATED_SHARDS.add(cache_key)
|
||||
|
||||
# Per-connection state — must run on EVERY open. SQLite scopes
|
||||
# `foreign_keys` per-connection (not per-DB), so the FK-CASCADE
|
||||
# behavior our schema relies on requires this PRAGMA every time
|
||||
# we open a connection. The other settings are tuning flags that
|
||||
# also live per-connection.
|
||||
conn.execute("PRAGMA foreign_keys = ON")
|
||||
conn.execute("PRAGMA synchronous = NORMAL")
|
||||
conn.execute("PRAGMA cache_size = -65536")
|
||||
conn.execute("PRAGMA temp_store = MEMORY")
|
||||
|
|
|
|||
|
|
@ -61,7 +61,7 @@ Newest first. Update on every open/close.
|
|||
|
||||
| ID | Title | Status | Opened | Directive |
|
||||
|----------|------------------------------------------------|-----------------------|------------|-----------|
|
||||
| #000026 | Real-shard workload baseline + search latency | open · awaiting go/no-go | 2026-05-08 | — |
|
||||
| #000026 | Real-shard workload baseline + search latency | in progress · Phase 1 landed 2026-05-08 | 2026-05-08 | — |
|
||||
| #000025 | 5F battery (Function · Finetuning · Falsification · Formulate · Feedback Loop) | in progress · Phase 1a landed 2026-05-08 | 2026-05-07 | — |
|
||||
| #000024 | 5T Phase 1b + Dav1DPrometheus vocabulary alignment | closed · landed 2026-05-08 | 2026-05-07 | — |
|
||||
| #000023 | 5S Phase 1b: Syllogism · Synthesis · Semiotics | closed · landed 2026-05-08 | 2026-05-07 | — |
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
# Ticket #000026 — Real-shard workload baseline + search latency
|
||||
|
||||
**Status:** open · awaiting go/no-go
|
||||
**Status:** in progress · Phase 1 landed 2026-05-08
|
||||
**Opened:** 2026-05-08
|
||||
**Scope:** establish a reproducible baseline for arborist's behavior
|
||||
on real shards (~38 GB Wikipedia + crawl). Capture latency, capital
|
||||
|
|
@ -231,6 +231,36 @@ prose, not enforced.
|
|||
|
||||
## Status
|
||||
|
||||
Open · awaiting go/no-go. Profile data captured 2026-05-08
|
||||
(commit `0a2e347`). Implementation can land Phase 1 in one
|
||||
commit (~30 LOC + test); Phase 2 in a follow-up.
|
||||
In progress.
|
||||
|
||||
**Phase 1 landed 2026-05-08.** Per-process migration memoization
|
||||
in `arborist.store.connect()` keyed by resolved path. Migrations
|
||||
+ `executescript(SCHEMA_SQL)` run once per (path, process); per-
|
||||
connection PRAGMAs (`foreign_keys=ON`, `synchronous=NORMAL`,
|
||||
`cache_size`, `temp_store`, `mmap_size`) still run every time —
|
||||
SQLite scopes `foreign_keys` per-connection and our schema
|
||||
relies on FK CASCADE.
|
||||
|
||||
Re-profile of `who wrote virt-back?` (commit post-Phase 1, warm
|
||||
cache):
|
||||
|
||||
```
|
||||
metric before after
|
||||
_migrate_* (each function) 586 7 ← one per shard
|
||||
executescript 586 7
|
||||
SQLite executes 10,623 3,687 (-65%)
|
||||
wall (warm) 14.5 s 13.4 s
|
||||
```
|
||||
|
||||
Warm-cache wall delta is small because the migration probes were
|
||||
many-but-cheap; the residual budget lives in FTS5 search (6.7 s,
|
||||
separate concern) and `synonym_expand` (2.8 s, separate concern).
|
||||
The 65% execute drop is the cold-cache win — each redundant
|
||||
`executescript` had been triggering disk reads at 75 s scale per
|
||||
the reviewer's report.
|
||||
|
||||
Phase 2 (baseline artifact) and Phase 3 (warrant-quality finding)
|
||||
queued. Authorship warrant ladder remains explicitly out of scope
|
||||
per the design choices section.
|
||||
|
||||
Phase 1 landed in commit `<filled on commit>`.
|
||||
|
|
|
|||
192
tests/test_store_migration_memoization.py
Normal file
192
tests/test_store_migration_memoization.py
Normal file
|
|
@ -0,0 +1,192 @@
|
|||
"""Regression tests for ticket #000026 Phase 1.
|
||||
|
||||
Per-process migration memoization. ``arborist.store.connect()`` used
|
||||
to run all 7 forward-migration probes on every open — 588 redundant
|
||||
probe sequences per typical query. With memoization, the migration
|
||||
block runs once per (physical file, process) and is skipped on every
|
||||
subsequent open of the same shard.
|
||||
|
||||
Tests pin:
|
||||
|
||||
- First connect runs migrations.
|
||||
- Second connect on the same path skips them.
|
||||
- Functional integrity is preserved (all schema is in place after
|
||||
the second connect).
|
||||
- The cache invalidates when a file is replaced at the same path
|
||||
(different inode → migrations re-run).
|
||||
- ``_clear_migration_cache()`` resets the memo for tests / explicit
|
||||
re-probe.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
from arborist import store
|
||||
from arborist.store import (
|
||||
_clear_migration_cache,
|
||||
connect,
|
||||
invalidate_migration_cache,
|
||||
)
|
||||
|
||||
|
||||
class _MigrationProbeCounter:
|
||||
"""Wraps each migration helper and counts invocations.
|
||||
|
||||
Patches the module-level names so ``connect()`` (which references
|
||||
them via the ``arborist.store`` module globals) calls the
|
||||
counted versions.
|
||||
"""
|
||||
|
||||
NAMES = (
|
||||
"_migrate_audit_mode",
|
||||
"_migrate_mesh_peer_chains",
|
||||
"_migrate_document_http_meta",
|
||||
"_migrate_selfmodel_tables",
|
||||
"_migrate_capital_ledger",
|
||||
"_migrate_memory_root",
|
||||
"_migrate_adapter_loss_reports",
|
||||
)
|
||||
|
||||
def __init__(self, monkeypatch):
|
||||
self.counts: dict[str, int] = {n: 0 for n in self.NAMES}
|
||||
for name in self.NAMES:
|
||||
original = getattr(store, name)
|
||||
|
||||
def _wrapped(conn, *, _orig=original, _name=name):
|
||||
self.counts[_name] += 1
|
||||
return _orig(conn)
|
||||
|
||||
monkeypatch.setattr(store, name, _wrapped)
|
||||
|
||||
@property
|
||||
def total(self) -> int:
|
||||
return sum(self.counts.values())
|
||||
|
||||
|
||||
def test_first_connect_runs_all_seven_migrations(tmp_path: Path, monkeypatch):
|
||||
_clear_migration_cache()
|
||||
counter = _MigrationProbeCounter(monkeypatch)
|
||||
db = tmp_path / "first.db"
|
||||
|
||||
conn = connect(db)
|
||||
conn.close()
|
||||
|
||||
# All seven probes ran exactly once.
|
||||
assert counter.total == 7
|
||||
for name in counter.NAMES:
|
||||
assert counter.counts[name] == 1
|
||||
|
||||
|
||||
def test_second_connect_skips_migrations(tmp_path: Path, monkeypatch):
|
||||
_clear_migration_cache()
|
||||
db = tmp_path / "second.db"
|
||||
|
||||
# Warm the cache with a fresh open (no probe counter yet).
|
||||
conn = connect(db)
|
||||
conn.close()
|
||||
|
||||
# Now patch in counters and open again. Zero probes should run.
|
||||
counter = _MigrationProbeCounter(monkeypatch)
|
||||
conn = connect(db)
|
||||
conn.close()
|
||||
assert counter.total == 0
|
||||
|
||||
|
||||
def test_second_connect_schema_is_intact(tmp_path: Path):
|
||||
"""Belt-and-suspenders: migration skipping must not break schema
|
||||
visibility. Open + close + reopen, then read every table the
|
||||
migrations create."""
|
||||
_clear_migration_cache()
|
||||
db = tmp_path / "intact.db"
|
||||
connect(db).close()
|
||||
|
||||
conn = connect(db)
|
||||
try:
|
||||
# Smoke-check tables that each migration is responsible for.
|
||||
for table in (
|
||||
"providence_cache", # _migrate_audit_mode
|
||||
"mesh_peer_chains", # _migrate_mesh_peer_chains
|
||||
"document_http_meta", # _migrate_document_http_meta
|
||||
"selfmodel_records", # _migrate_selfmodel_tables
|
||||
"capital_ledger", # _migrate_capital_ledger
|
||||
"memory_records", # _migrate_memory_root
|
||||
"adapter_loss_reports", # _migrate_adapter_loss_reports
|
||||
):
|
||||
row = conn.execute(
|
||||
"SELECT name FROM sqlite_master WHERE type='table' AND name=?",
|
||||
(table,),
|
||||
).fetchone()
|
||||
assert row is not None, f"missing table after re-open: {table}"
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
|
||||
def test_clear_migration_cache_forces_reprobe(tmp_path: Path, monkeypatch):
|
||||
_clear_migration_cache()
|
||||
db = tmp_path / "clear.db"
|
||||
|
||||
connect(db).close()
|
||||
|
||||
counter = _MigrationProbeCounter(monkeypatch)
|
||||
|
||||
# Without clearing — zero probes.
|
||||
connect(db).close()
|
||||
assert counter.total == 0
|
||||
|
||||
# After clearing — full re-probe.
|
||||
_clear_migration_cache()
|
||||
connect(db).close()
|
||||
assert counter.total == 7
|
||||
|
||||
|
||||
def test_invalidate_migration_cache_after_file_replacement(
|
||||
tmp_path: Path, monkeypatch
|
||||
):
|
||||
"""Caller-driven invalidation contract: after replacing a shard
|
||||
at the same path, the caller must call
|
||||
``invalidate_migration_cache(path)`` so the next ``connect()``
|
||||
re-runs the full probe sequence on the new file.
|
||||
|
||||
We don't auto-detect replacement — (dev, inode) is unreliable
|
||||
under tmpfs inode reuse, and (mtime, size) drifts naturally as
|
||||
SQLite operates. Path-only with explicit invalidation is the
|
||||
honest contract; tests + snapshot-restore flows are responsible
|
||||
for calling the invalidator."""
|
||||
_clear_migration_cache()
|
||||
db = tmp_path / "replaced.db"
|
||||
|
||||
# First open — populate cache.
|
||||
connect(db).close()
|
||||
|
||||
# Replace the file at the same path.
|
||||
db.unlink()
|
||||
|
||||
# Without invalidation, the cache still claims migrations are
|
||||
# current — schema would be silently absent on the new file.
|
||||
invalidate_migration_cache(db)
|
||||
|
||||
counter = _MigrationProbeCounter(monkeypatch)
|
||||
conn = connect(db)
|
||||
conn.close()
|
||||
assert counter.total == 7
|
||||
|
||||
|
||||
def test_distinct_paths_each_get_one_probe(tmp_path: Path, monkeypatch):
|
||||
"""Two different shard files get one probe each — the cache is
|
||||
per-path, not global."""
|
||||
_clear_migration_cache()
|
||||
db_a = tmp_path / "a.db"
|
||||
db_b = tmp_path / "b.db"
|
||||
|
||||
counter = _MigrationProbeCounter(monkeypatch)
|
||||
connect(db_a).close()
|
||||
connect(db_b).close()
|
||||
# Two paths × seven migrations = 14.
|
||||
assert counter.total == 14
|
||||
|
||||
# Re-open both — cache hit on each, zero new probes.
|
||||
before = counter.total
|
||||
connect(db_a).close()
|
||||
connect(db_b).close()
|
||||
assert counter.total == before
|
||||
Loading…
Add table
Add a link
Reference in a new issue