Skip to content

Fix O(collection) client-side scans and missing payload indexes on the qdrant network backend - #2043

Open
jhsmith409 wants to merge 2 commits into
MemPalace:developfrom
jhsmith409:fix/shared-network-backend-scaling
Open

Fix O(collection) client-side scans and missing payload indexes on the qdrant network backend#2043
jhsmith409 wants to merge 2 commits into
MemPalace:developfrom
jhsmith409:fix/shared-network-backend-scaling

Conversation

@jhsmith409

Copy link
Copy Markdown

What & why

Fixes #2042. Setting up a shared palace across multiple machines on the qdrant network backend surfaces several operations that assume cheap local ChromaDB reads and instead do O(collection) client-side scans, or rely on payload indexes that are never created. On a shared collection of ~184k drawers these hang (mine, status, auto-save hooks) or fail (facet status 400s), which makes the network-backend / team setup impractical at scale.

Changes

All new paths are gated on the backend's supports_metadata_facets capability and fall back to the existing local-optimized path, so ChromaDB behavior is unchanged.

  • qdrant payload indexes_ensure_remote_collection now creates keyword indexes on metadata.wing, metadata.room, metadata.source_file (idempotent via the existing 400/409-swallowing create_payload_index, and backfills already-existing collections). Fixes facet_counts() HTTP 400 "No appropriate index for faceting" (which the MCP mempalace_status tool hits) and lets metadata-filtered reads use an index.
  • qdrant marker on open — write qdrant_backend.json when a populated collection is opened for write, not only on first upsert, so a read-first client can search/status a shared palace it hasn't written to yet.
  • hallways.compute_hallways_for_wing — server-side where={"wing": wing} fetch on facet-capable backends instead of scanning the whole collection and filtering client-side on every mine. Chroma keeps the client-side scan (its where binds one SQL var per matched id, overflowing on >32k wings, compute_hallways_for_wing crashes (too many SQL variables) on wings > ~32k drawers #1619).
  • miner.status — count wing/room via facet_counts on facet-capable backends instead of streaming every drawer's metadata (mirrors the MCP status tool).
  • palace.prefetch_mined_set — on facet-capable backends, resolve skip checks with lazy, cached, indexed per-source_file lookups scoped to the files being mined, instead of one full-collection scan that hangs every convo-mode mine (auto-save hooks).

Testing

  • py_compile + import of all four changed modules pass.
  • I could not run the full pytest suite in my environment (dev deps unavailable in the packaged tool venv) — CI should exercise it.
  • Validated in production: this exact logic has been running as runtime patches across a 6-machine fleet sharing one ~184k-drawer qdrant collection. Before: status and convo-mode mine hung for minutes; facet status 400'd. After: status ~0.5s, project mine ~1.2s, convo-mine ~0.5s, facets fast.

Happy to adjust naming/placement (e.g. hoist the small _backend_supports_facets helper into backends/base.py rather than the three inline copies) to match project conventions.

🤖 Generated with Claude Code

https://claude.ai/code/session_01H32fWQrjdZWPNhfY1gzCgH

@jhsmith409

Copy link
Copy Markdown
Author

For reviewers — where this fits with the existing server-side aggregation work:

All new fast paths are gated on supports_metadata_facets with a fallback to the existing local path, so ChromaDB is unchanged.

@jhsmith409

Copy link
Copy Markdown
Author

Rebased onto current develop (aa89bd8, post-#2051) — clean, no conflicts, and the diff is unchanged at 183+/18− across the same four files. The only upstream movement in files this PR touches was miner.py (#2054 / #2055 chunk_text changes), which doesn't overlap the status facet path changed here.

Could a maintainer approve the workflow run? No checks have ever reported on this branch — as a fork PR the Actions run needs approval, so Tests has never executed, and the unstable merge state reflects that rather than any failure. Happy to rebase again or split this into smaller pieces if that would make it easier to review.

@jhsmith409
jhsmith409 force-pushed the fix/shared-network-backend-scaling branch from e6b1b0a to 0a6bdad Compare August 5, 2026 00:34
@jhsmith409

Copy link
Copy Markdown
Author

Rebased onto current develop (1d113e4, post-#2148) — the branch had gone stale and was showing CONFLICTING. It's MERGEABLE again at 0a6bdad.

Conflict resolution (one hunk, miner.status()) — upstream added the #1222 HNSW-divergence preflight and moved total = col.count() down into the scan block; this branch had moved total up and inserted the facet fast path at the same spot. Kept both, ordered:

  1. HNSW divergence preflight (unchanged from develop — it has to stay ahead of count(), since that's the call it guards against the SIGSEGV class)
  2. total = col.count()
  3. facet fast path, gated on supports_metadata_facets
  4. paginated client-side scan fallback

Net effect on ChromaDB is still nil: the preflight runs first exactly as on develop, and the facet branch is skipped.

Also addressed the helper duplication I flagged in the description. _backend_supports_facets is now a single collection_supports_facets() in backends/base.py, re-exported from the backends package, replacing the three inline copies. mcp_server._supports_metadata_facets stays as a module-level name (now a one-line delegate) rather than being renamed — tests/test_mcp_server.py monkeypatches it in four places, and I'd rather not pull test churn into this PR. Happy to collapse it fully if you'd prefer.

Also fixed a missing blank line in hallways.py that ruff format --check would have failed on; squashed into the original commit.

Verification — I got a clean dev environment this time, so the suite I couldn't run before now has results:

  • pytest (full suite): 3607 passed, 31 skipped, 106 deselected
  • ruff check mempalace/: clean
  • ruff format --check mempalace/: clean

Diff is now 189+/25− across 7 files (was 184+/18− across 4), in two commits.

mergeStateStatus is still UNSTABLE, which continues to reflect that no Actions run has ever been approved on this fork branch rather than any failure — a maintainer approving the workflow would give you the real signal.

jhsmith409 and others added 2 commits August 19, 2026 11:51
…rant

Setting up a shared palace across machines on the qdrant network backend
surfaces several operations that assume cheap local ChromaDB reads and
instead scan the whole collection client-side, or rely on payload indexes
that are never created. On a large shared collection these hang or fail.

- qdrant: create keyword payload indexes on metadata.wing/room/source_file
  when ensuring the collection (idempotent, backfills existing collections),
  so facet_counts() no longer 400s ("No appropriate index for faceting") and
  metadata-filtered reads use an index instead of a full scan.
- qdrant: write the local marker when a populated collection is opened for
  write, not only on first upsert, so a read-first client can search/status a
  shared palace it hasn't written to yet.
- hallways.compute_hallways_for_wing: use a server-side where={"wing": wing}
  filter on facet-capable backends (bounded to the wing) instead of scanning
  the whole collection and filtering client-side on every mine. ChromaDB
  keeps the client-side scan (its where binds one SQL var per matched id and
  overflows on >32k wings, MemPalace#1619).
- miner.status: count wing/room via facet_counts on facet-capable backends
  instead of streaming every drawer's metadata (mirrors the MCP status tool).
- palace.prefetch_mined_set: on facet-capable backends, resolve skip checks
  with lazy, cached, indexed per-source_file lookups (scoped to the files
  being mined) instead of one full-collection scan that hangs every
  convo-mode mine (auto-save hooks).

All new paths are gated on the backend's supports_metadata_facets capability
and fall back to the existing local-optimized path, so ChromaDB is unchanged.

Fixes MemPalace#2042

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H32fWQrjdZWPNhfY1gzCgH
Replaces the three inline _backend_supports_facets copies added by this
branch with a single collection_supports_facets() in backends/base.py,
re-exported from the backends package. mcp_server keeps its module-level
_supports_metadata_facets name (now a thin delegate) so its call sites
stay monkeypatchable per-module.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jhsmith409
jhsmith409 force-pushed the fix/shared-network-backend-scaling branch from 0a6bdad to f26f5ba Compare August 19, 2026 12:04
@jhsmith409
jhsmith409 requested a review from igorls as a code owner August 19, 2026 12:04
@jhsmith409

Copy link
Copy Markdown
Author

Rebased onto current develop (517a7a0) — the branch had gone stale again (205 commits behind, CONFLICTING/DIRTY). Now f26f5ba, still two commits.

Conflicts (two hunks)

  1. palace.prefetch_mined_set — upstream mine --mode convos: file-level already-filed tracking can silently skip individual exchanges from an already-mined transcript #2183 replaced the flat mined dict with per-source_file/per-source_mtime groups and a chunk_total completeness rule. Kept upstream's scan verbatim and put the facet-capable early return ahead of it, so the fallback path is byte-identical to develop.
  2. mcp_server imports — develop added date_window to the same import block this branch touched for collection_supports_facets. Kept both.

Follow-on fix while resolving: _LazyMinedSet._lookup (the facet-backend path) predated #2183 and would have reported a mid-file partial as fully mined — exactly the stranding bug #2183 fixed, just on qdrant/pgvector instead of Chroma. It now applies the same rule as the bulk scan: group by stored mtime, count against chunk_total, treat a drawer with no chunk_total as a legacy row and trust it. Without this the rebase would have re-introduced the bug on the network backends.

Verification (rebased branch vs. develop at the same commit, clean venvs, same machine):

branch f26f5ba develop 517a7a0
pytest 4310 passed, 4 failed, 31 skipped 4310 passed, 4 failed, 31 skipped
ruff check mempalace/ clean
ruff format --check mempalace/ clean (79 files)

The 4 failures are all in tests/test_palace_graph_limits.py (TestFindTunnelRanking, TestGraphStatsDetails) and reproduce identically on unmodified develop — pre-existing, unrelated to this PR, which touches no graph code.

Diff is 206+/25− across 7 files. Still no Actions run has ever executed on this fork branch — a maintainer approving the workflow would give you real CI signal.

@jhsmith409

Copy link
Copy Markdown
Author

Any chance for a review and incorporation into your next release? I've been running this on my cluster of 5 machines for over a month now with no issues. Significantly speeds up use of MemPalace when used across more than one machine.

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.

Shared qdrant network backend doesn't scale: O(collection) client-side scans hang; required payload indexes never created

1 participant