Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions mempalace/backends/chroma.py
Original file line number Diff line number Diff line change
Expand Up @@ -1311,6 +1311,33 @@ def _close_client(client) -> None:
logger.debug("client.close() unavailable or failed", exc_info=True)


def _clear_chroma_system_cache() -> None:
"""Drop chromadb's process-global ``SharedSystemClient`` cache.

chromadb caches its ``System`` (and the live HNSW segment) keyed by path.
A bare ``chromadb.PersistentClient(path=...)`` reopen reuses that cached
System, so after a peer/rebuild has changed ``chroma.sqlite3`` on disk we
would rebuild against the stale in-memory segment and persist an outdated
index over the on-disk changes -- the same data-loss class as #2002,
reached via :meth:`ChromaBackend._client` instead of
``mcp_server._get_client``. This mirrors the reset already performed by
``mcp_server._force_chroma_cache_reset`` and ``repair._close_chroma_handles``.

The clear is process-global (it evicts every palace's cached System, not
just this path); chromadb exposes no per-path eviction. It only fires on the
inode/mtime-change branch of ``_client``, never the steady-state hot path,
so the redundant rebuild cost is bounded to genuine external-change reopens.
"""
try:
from chromadb.api.client import SharedSystemClient

clear = getattr(SharedSystemClient, "clear_system_cache", None)
if callable(clear):
clear()
except Exception:
logger.debug("Failed to clear chromadb SharedSystemClient cache", exc_info=True)


class ChromaCollection(BaseCollection):
"""Thin adapter translating ChromaDB dict returns into typed results.

Expand Down Expand Up @@ -2054,6 +2081,14 @@ def _client(self, palace_path: str):
or (mtime_appeared and palace_path in self._freshness)
):
ChromaBackend._quarantined_paths.discard(palace_path)
# #2028: the same external change means chromadb's path-keyed
# System cache is now stale. Reconstructing PersistentClient
# below would reuse the cached System (and its in-memory HNSW
# segment), so drop the shared cache first -- otherwise the
# rebuilt client persists an outdated index over the on-disk
# change. Gated on genuine external change (not first open) so
# cold opens never pay the global-evict cost.
_clear_chroma_system_cache()
ChromaBackend._prepare_palace_for_open(palace_path)
cached = chromadb.PersistentClient(path=palace_path)
self._clients[palace_path] = cached
Expand Down
61 changes: 61 additions & 0 deletions tests/test_backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -1827,6 +1827,67 @@ class DummyClient:
]


def test_chroma_backend_resets_system_cache_on_inode_change(tmp_path, monkeypatch):
"""#2028: ``_client`` must drop chromadb's path-keyed ``SharedSystemClient``
cache *before* reconstructing ``PersistentClient`` on an inode/mtime change.

chromadb caches its ``System`` (and live HNSW segment) keyed by path, so a
bare reopen reuses the stale segment and persists an outdated index over a
peer/rebuild's on-disk changes -- the #2002 data-loss class reached via
``_client`` instead of ``mcp_server._get_client``. The reset must fire only
on a genuine external change (not first open) and must precede the reopen.
"""
palace = tmp_path / "palace"
palace.mkdir()
(palace / "chroma.sqlite3").write_text("")

events = []

# Neutralize the on-disk HNSW pre-checks so the test exercises only the
# cache-reset / client-rebuild ordering.
for _name in (
"_fix_missing_collection_type",
"_fix_blob_seq_ids",
"quarantine_invalid_hnsw_metadata",
"quarantine_stale_hnsw",
):
monkeypatch.setattr(f"mempalace.backends.chroma.{_name}", lambda path, *a, **k: [])

monkeypatch.setattr(ChromaBackend, "_quarantined_paths", set())

class DummyClient:
pass

def _record_open(path):
events.append(("open", path))
return DummyClient()

monkeypatch.setattr("mempalace.backends.chroma.chromadb.PersistentClient", _record_open)

from chromadb.api.client import SharedSystemClient

def _record_clear(*args, **kwargs):
events.append(("clear", None))

monkeypatch.setattr(SharedSystemClient, "clear_system_cache", _record_clear)

backend = ChromaBackend()
# ``_db_stat`` is called twice per ``_client`` call (freshness check, then
# re-stat after reopen). Same inode on the first call (first open, no prior
# freshness -> no reset), changed inode on the second (external change).
stats = iter([(1, 1.0), (1, 1.0), (2, 2.0), (2, 2.0)])
monkeypatch.setattr(backend, "_db_stat", lambda path: next(stats))

backend._client(str(palace)) # first open: no external change -> no clear
backend._client(str(palace)) # inode 1 -> 2: clear, then reopen

assert events == [
("open", str(palace)), # first open, no cache reset
("clear", None), # #2028: reset fires on the inode change...
("open", str(palace)), # ...strictly before the PersistentClient reopen
], events


def test_explain_ef_mismatch_recognizes_chromadb_conflict():
"""When ChromaDB rejects a collection read due to an EF-name mismatch
(user changed MEMPALACE_EMBEDDING_MODEL on an existing palace), the
Expand Down