`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.
192 lines
5.9 KiB
Python
192 lines
5.9 KiB
Python
"""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
|