Skip to content

fix(chroma): reset chromadb System cache in ChromaBackend._client() on inode/mtime reopen - #2032

Merged
2 commits merged into
MemPalace:developfrom
colorpanda82:fix/2028-client-system-cache-reset-on-reopen
Aug 12, 2026
Merged

fix(chroma): reset chromadb System cache in ChromaBackend._client() on inode/mtime reopen#2032
2 commits merged into
MemPalace:developfrom
colorpanda82:fix/2028-client-system-cache-reset-on-reopen

Conversation

@colorpanda82

Copy link
Copy Markdown
Contributor

Fixes #2028.

What

ChromaBackend._client() reconstructs chromadb.PersistentClient(path=...) when it detects an inode/mtime change on chroma.sqlite3, but it did not first drop chromadb's process-global SharedSystemClient cache. chromadb caches its System (and the live HNSW segment) keyed by path, so the reconstructed client reused the stale in-memory segment and could persist an outdated index over a peer/rebuild's on-disk changes, the same data-loss class as #2002, reached through _client() instead of mcp_server._get_client.

This adds a SharedSystemClient.clear_system_cache() call to the existing external-change branch of _client() (the same branch that already discards the quarantine gate), mirroring the reset that mcp_server._force_chroma_cache_reset (#2026) and repair._close_chroma_handles already perform on their paths.

Why it was latent, and why fix it anyway

No shipped caller is currently exposed: the MCP server resets via _force_chroma_cache_reset (#2026), repair._post_rebuild_cleanup resets via _close_chroma_handles, the daemon holds no chroma backend, and CLI/migrate run in short-lived processes that start with an empty chromadb cache. Today the invariant is upheld by callers remembering to reset out-of-band. This moves the invariant into the backend so a future in-process open→rebuild→reopen through _client() cannot silently reintroduce #2002.

Upstream corroboration

The mechanism is documented in chromadb's own tracker:

Caveat, disclosed: chromadb annotates clear_system_cache() as intended for testing. This PR does not introduce a new dependency on it, the shipped mcp_server._force_chroma_cache_reset (#2026) and repair._close_chroma_handles already call it; this change brings _client() in line with those existing paths. If the project would rather gate this behind a different eviction primitive, happy to follow.

Scope notes

  • clear_system_cache() is process-global; chromadb exposes no per-path eviction (see #5843). It fires only on the inode/mtime-change branch, never the steady-state hot path, so the redundant-rebuild cost is bounded to genuine external-change reopens. The existing MCP and repair paths already accept this same global-clear cost.
  • Defensive import + getattr guard match the style of the existing reset helpers, so a chromadb version without clear_system_cache degrades to a logged debug rather than an error.

Test

tests/test_backends.py::test_chroma_backend_resets_system_cache_on_inode_change scripts an inode swap between two _client() calls and asserts the cache reset fires on the change reopen, strictly before the PersistentClient reconstruction, and not on first open. Modeled on the existing test_chroma_backend_requarantines_after_inode_replacement.

Relates to #2002, #2026. Distinct from the daemon/single-writer work in #1976.

…n inode/mtime reopen

_client() reconstructs PersistentClient on an inode/mtime change but did not drop chromadb's process-global SharedSystemClient cache first, so the rebuilt client reused the stale path-keyed System (and its in-memory HNSW segment) and could persist an outdated index over on-disk changes -- the same class as MemPalace#2002, reached via _client() instead of _get_client.

Add SharedSystemClient.clear_system_cache() to the external-change branch of _client(), mirroring mcp_server._force_chroma_cache_reset (MemPalace#2026) and repair._close_chroma_handles. Backend-level regression test asserts the reset fires on the change reopen, strictly before the reconstruct, and not on first open (chroma-core/chroma#2536, #5843).

Fixes MemPalace#2028.
Two monkeypatch.setattr calls were wrapped across lines that fit within
the line length; ruff format --check flagged them. Formatter-only, no
behavior change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gg6g5efZ1rNbBTHqGz2Tjw
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ChromaBackend._client() inode/mtime reopen rebuilds without a chromadb system-cache reset (parallel to #2002)

2 participants