Skip to content

Commit 055386e

Browse files
committed
feat(backends): quarantine_stale_hnsw — recover from HNSW/sqlite drift
Add a helper that renames HNSW segment directories whose `data_level0.bin` is significantly older than `chroma.sqlite3`. Drift between the on-disk HNSW graph and the live embeddings table is the root cause of a segfault class where the Rust graph-walk dereferences dangling neighbor pointers for entries in the metadata segment that no longer exist in the HNSW index, crashing in a background thread on `count()` or `query()`. Issue #823 describes the same drift as a silent-staleness symptom (semantic search returns stale results after `add_drawer` because `data_level0.bin` lags the sqlite metadata under the default `sync_threshold=1000`). Under heavier load or after an interrupted write, the same drift can escalate from "silent stale results" to "SIGSEGV on next open," which is the failure mode observed at neo-cortex-mcp#2 (chromadb 1.5.5, Python 3.12) and acknowledged at chroma-core/chroma#2594. On one 135K-drawer palace where `index_metadata.pickle` claimed 137,813 elements against 135,464 rows in sqlite (2,349-entry drift), fresh Python processes crashed in `col.count()` 17/20 times; after renaming the segment dir out of the way and letting ChromaDB rebuild lazily, the same 20-run check went to 0 crashes. The recovery path #823 suggests (export / recreate / reimport) is heavy — it re-embeds every drawer. This helper is lighter: rename the segment dir so ChromaDB reopens without it, and the indexer rebuilds lazily on the next write. The original directory is renamed (not deleted) so the operator can recover if the heuristic misfires. If `chroma.sqlite3` is more than `stale_seconds` (default 3600) newer than the segment's `data_level0.bin`, the segment is considered suspect. One hour is deliberately conservative — normal HNSW flush cadence is seconds to minutes, so an hour of drift implies a crashed mid-write, not routine lag. - Additive: exposes `quarantine_stale_hnsw(palace_path, stale_seconds)` as a helper. Not wired into `_client()` / startup on this PR — the goal is to land the primitive first so operators and higher layers can opt in. A follow-up could call it automatically on palace open behind an env var or config flag. - Closes #823 by giving operators a first-class recovery path without having to install `chromadb-ops` or re-mine. Four new tests in `tests/test_backends.py`: - renames drifted segment, preserves original files under `.drift-TS` suffix - leaves fresh segments alone - no-op on missing palace path / missing `chroma.sqlite3` - skips already-quarantined (`.drift-` suffixed) directories `pytest tests/test_backends.py` → 11 passed. `ruff check` / `ruff format --check` — clean.
1 parent 2b9f17c commit 055386e

2 files changed

Lines changed: 155 additions & 1 deletion

File tree

mempalace/backends/chroma.py

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""ChromaDB-backed MemPalace storage backend (RFC 001 reference implementation)."""
22

3+
import datetime as _dt
34
import logging
45
import os
56
import sqlite3
@@ -48,6 +49,88 @@ def _validate_where(where: Optional[dict]) -> None:
4849
stack.extend(x for x in v if isinstance(x, dict))
4950

5051

52+
def quarantine_stale_hnsw(palace_path: str, stale_seconds: float = 3600.0) -> list[str]:
53+
"""Rename HNSW segment dirs whose files are stale vs. chroma.sqlite3.
54+
55+
When a ChromaDB 1.5.x PersistentClient opens a palace whose on-disk
56+
HNSW segment is significantly older than ``chroma.sqlite3``, the Rust
57+
graph-walk can dereference dangling neighbor pointers for entries that
58+
exist in the metadata segment but not in the HNSW index, and segfault
59+
in a background thread on the next ``count()`` or ``query(...)`` call.
60+
61+
This is the same failure mode reported at #823 (semantic search stale
62+
after ``add_drawer``), observed at neo-cortex-mcp#2 (SIGSEGV on
63+
``count()`` with chromadb 1.5.5), and acknowledged as by-design at
64+
chroma-core/chroma#2594. On one fork palace (135K drawers), the drift
65+
caused a 65–85% crash rate on fresh-process opens; fresh-process
66+
crash rate dropped to 0% after the segment dir was renamed out of the
67+
way and ChromaDB rebuilt lazily.
68+
69+
Heuristic: if ``chroma.sqlite3`` is more than ``stale_seconds`` newer
70+
than the segment's ``data_level0.bin``, the segment is considered
71+
suspect and renamed to ``<uuid>.drift-<timestamp>``. ChromaDB reopens
72+
cleanly without it and writes fresh index files on next use. The
73+
original directory is renamed, not deleted, so recovery remains
74+
possible if the heuristic misfires.
75+
76+
The default threshold (1h) is deliberately conservative — ChromaDB's
77+
HNSW flush cadence means legitimate drift is normally on the order of
78+
seconds to minutes. A segment that is more than an hour out of date is
79+
almost certainly in a "crashed mid-write" state.
80+
81+
Args:
82+
palace_path: path to the palace directory containing ``chroma.sqlite3``
83+
stale_seconds: minimum mtime gap to treat a segment as stale
84+
85+
Returns:
86+
List of paths that were quarantined (empty if nothing drifted).
87+
"""
88+
db_path = os.path.join(palace_path, "chroma.sqlite3")
89+
if not os.path.isfile(db_path):
90+
return []
91+
try:
92+
sqlite_mtime = os.path.getmtime(db_path)
93+
except OSError:
94+
return []
95+
96+
moved: list[str] = []
97+
try:
98+
entries = os.listdir(palace_path)
99+
except OSError:
100+
return []
101+
102+
for name in entries:
103+
if "-" not in name or name.startswith(".") or ".drift-" in name:
104+
continue
105+
seg_dir = os.path.join(palace_path, name)
106+
if not os.path.isdir(seg_dir):
107+
continue
108+
hnsw_bin = os.path.join(seg_dir, "data_level0.bin")
109+
if not os.path.isfile(hnsw_bin):
110+
continue
111+
try:
112+
hnsw_mtime = os.path.getmtime(hnsw_bin)
113+
except OSError:
114+
continue
115+
if sqlite_mtime - hnsw_mtime < stale_seconds:
116+
continue
117+
stamp = _dt.datetime.now().strftime("%Y%m%d-%H%M%S")
118+
target = f"{seg_dir}.drift-{stamp}"
119+
try:
120+
os.rename(seg_dir, target)
121+
moved.append(target)
122+
logger.warning(
123+
"Quarantined stale HNSW segment %s "
124+
"(sqlite %.0fs newer than HNSW); renamed to %s",
125+
seg_dir,
126+
sqlite_mtime - hnsw_mtime,
127+
target,
128+
)
129+
except OSError:
130+
logger.exception("Failed to quarantine stale HNSW segment %s", seg_dir)
131+
return moved
132+
133+
51134
def _fix_blob_seq_ids(palace_path: str) -> None:
52135
"""Fix ChromaDB 0.6.x -> 1.5.x migration bug: BLOB seq_ids -> INTEGER.
53136

tests/test_backends.py

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import os
12
import sqlite3
23

34
import chromadb
@@ -11,7 +12,12 @@
1112
available_backends,
1213
get_backend,
1314
)
14-
from mempalace.backends.chroma import ChromaBackend, ChromaCollection, _fix_blob_seq_ids
15+
from mempalace.backends.chroma import (
16+
ChromaBackend,
17+
ChromaCollection,
18+
_fix_blob_seq_ids,
19+
quarantine_stale_hnsw,
20+
)
1521

1622

1723
class _FakeCollection:
@@ -372,3 +378,68 @@ def test_fix_blob_seq_ids_noop_without_blobs(tmp_path):
372378
def test_fix_blob_seq_ids_noop_without_database(tmp_path):
373379
"""No error when palace has no chroma.sqlite3."""
374380
_fix_blob_seq_ids(str(tmp_path)) # should not raise
381+
382+
383+
# ── quarantine_stale_hnsw ─────────────────────────────────────────────────
384+
385+
386+
def _make_palace_with_segment(tmp_path, hnsw_mtime, sqlite_mtime):
387+
"""Helper: build a palace dir with one HNSW segment + sqlite at given mtimes."""
388+
palace = tmp_path / "palace"
389+
palace.mkdir()
390+
(palace / "chroma.sqlite3").write_text("")
391+
seg = palace / "abcd-1234-5678"
392+
seg.mkdir()
393+
(seg / "data_level0.bin").write_text("")
394+
os.utime(seg / "data_level0.bin", (hnsw_mtime, hnsw_mtime))
395+
os.utime(palace / "chroma.sqlite3", (sqlite_mtime, sqlite_mtime))
396+
return palace, seg
397+
398+
399+
def test_quarantine_stale_hnsw_renames_drifted_segment(tmp_path):
400+
"""Segment whose data_level0.bin is 2h older than sqlite gets renamed."""
401+
now = 1_700_000_000.0
402+
palace, seg = _make_palace_with_segment(tmp_path, hnsw_mtime=now - 7200, sqlite_mtime=now)
403+
moved = quarantine_stale_hnsw(str(palace), stale_seconds=3600.0)
404+
assert len(moved) == 1
405+
assert ".drift-" in moved[0]
406+
assert not seg.exists()
407+
# the renamed directory still exists and contains the original file
408+
renamed = list(palace.iterdir())
409+
drift_dirs = [p for p in renamed if ".drift-" in p.name]
410+
assert len(drift_dirs) == 1
411+
assert (drift_dirs[0] / "data_level0.bin").exists()
412+
413+
414+
def test_quarantine_stale_hnsw_leaves_fresh_segment_alone(tmp_path):
415+
"""Segment with recent mtime vs sqlite is not touched."""
416+
now = 1_700_000_000.0
417+
palace, seg = _make_palace_with_segment(tmp_path, hnsw_mtime=now - 10, sqlite_mtime=now)
418+
moved = quarantine_stale_hnsw(str(palace), stale_seconds=3600.0)
419+
assert moved == []
420+
assert seg.exists()
421+
422+
423+
def test_quarantine_stale_hnsw_no_palace(tmp_path):
424+
"""Missing palace path or chroma.sqlite3: return [] without raising."""
425+
assert quarantine_stale_hnsw(str(tmp_path / "missing")) == []
426+
empty = tmp_path / "empty"
427+
empty.mkdir()
428+
assert quarantine_stale_hnsw(str(empty)) == []
429+
430+
431+
def test_quarantine_stale_hnsw_skips_already_quarantined(tmp_path):
432+
"""Directories already named with ``.drift-`` suffix are never re-renamed."""
433+
now = 1_700_000_000.0
434+
palace = tmp_path / "palace"
435+
palace.mkdir()
436+
(palace / "chroma.sqlite3").write_text("")
437+
os.utime(palace / "chroma.sqlite3", (now, now))
438+
drift = palace / "abcd-1234.drift-20260101-000000"
439+
drift.mkdir()
440+
(drift / "data_level0.bin").write_text("")
441+
os.utime(drift / "data_level0.bin", (now - 99999, now - 99999))
442+
443+
moved = quarantine_stale_hnsw(str(palace), stale_seconds=3600.0)
444+
assert moved == []
445+
assert drift.exists()

0 commit comments

Comments
 (0)