Repository navigation
Conversation
This was referenced Aug 26, 2026
edwinyyyu
force-pushed
the
feat/config-registry
branch
3 times, most recently
from
August 27, 2026 17:27
10e77c4 to
4f59b07
Compare
12 of 13 tasks
edwinyyyu
force-pushed
the
feat/config-registry
branch
11 times, most recently
from
August 28, 2026 00:25
078a5b9 to
9629018
Compare
This was referenced Aug 28, 2026
edwinyyyu
force-pushed
the
feat/config-registry
branch
2 times, most recently
from
August 31, 2026 22:00
8dfd87d to
7e2ccfa
Compare
…tion Signed-off-by: Edwin Yu <[email protected]>
edwinyyyu
force-pushed
the
feat/config-registry
branch
from
August 31, 2026 22:44
7e2ccfa to
25b077d
Compare
This was referenced Sep 1, 2026
Closed
Contributor
Author
|
Superseded by a reworked stack. The premise this PR was built on has changed in four ways, each now filed separately:
The defects this PR did fix are still fixed by the replacement, and are now filed on their own so they do not get lost: #1562 (native name re-derived on open) and #1563 (stale handles resurrecting records). Closing rather than force-pushing, so the review discussion here stays attached to the design it was about. Replacement PRs to follow. |
This was referenced Sep 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose of the change
Horizontal scalability: multiple MemMachine server processes should be able to work with the same collections and partitions on one backend (#1524). Strict sharding — each collection managed by exactly one process, as the
VectorStorecontract requires today — has always worked; the restriction itself is the limitation, and what prevents lifting it is collection metadata management, not the data path. This PR introduces the enabling primitive — a cross-process-safe collection registry (CollectionRegistryABC + a SQLAlchemy implementation, in thevector_storepackage). Today several backends keep collection/partition metadata in ad-hoc per-backend bookkeeping whose create/open-or-create sequences are non-atomic read-check-write, guarded only by per-process asyncio locks. A registry backed by a store with real unique constraints makes entry creation an atomic compare-and-set across processes.A follow-up PR switches QdrantVectorStore's internal Qdrant-based
__registrybookkeeping to this registry.Description
Contract (
common/vector_store/collection_registry.py): aCollectionRegistryis a durable catalog of one vector store's logical collections — a mapping from (namespace, name) to an immutableCollectionRegistryEntry. The registry is deliberately specific to vector store collections rather than a generic key->config registry: the realistic reuse axis (other vector store backends — Milvus follows in this stack) is within the collection domain, a specific API can be widened later as an additive refactor whereas narrowing a shipped generic ABC is breaking, and speaking the domain removes key/adapter plumbing from the API entirely. Operations:register(namespace, name, entry)— atomic; raisesCollectionAlreadyRegisteredErroron an existing collection regardless of stored-entry equalityget(namespace, name)— stored entry orNoneget_or_register(namespace, name, entry)— atomic; returns(stored_entry, registered); never compares entries — config-equality policy belongs to the store, which translates conflicts into its own domain errorsderegister(namespace, name)— idempotentstartup()— idempotently prepares storageThe entry (
CollectionRegistryEntry={config, native_collection_name, partition_key}) stores resolved identity, one format shared by all backends:configis the ABC-levelVectorStoreCollectionConfig, the native name is pinned at registration (so config-serialization changes can never silently repoint collections at new empty native collections), and the partition key carries a per-registration generation (so records written through handles held across a deregistration stay invisible and are never resurrected). Evolution policy: extend by adding optional fields with defaults — no version bump, no migration; backend-specific needs use the same mechanism, not per-backend entry types.There is deliberately no update operation: entries are immutable once registered (the config hash is the native collection identity). Adding one later is purely additive.
Format evolution without standing version machinery. There is deliberately no stored format version and no version check. The evolution policy (add-optional-only, defaults reproducing prior behavior — stated on the entry model and guarded by a canned v1-row regression test) is what makes that safe: old rows classify exactly through defaults, so a future breaking change can introduce an entry format version field (default 1) at the moment it is first needed — every pre-existing row is correctly version 1 by construction — and ship its migration as a windowed migrate-then-deploy step. The accepted residual: an old binary started against migrated data fails with validation errors at first read rather than being refused at boot. (An earlier draft carried a declarations table with a startup version check and a redeclare primitive; it was removed because it insured against exactly the scenario the evolution policy already insures, at the cost of real API and documentation surface.)
SQLAlchemy implementation (
sqlalchemy_collection_registry.py): each registry owns a dedicated tablecollection_registry_<name>(keyVARCHAR(255) PK,entryJSON, JSONB on PostgreSQL), so registries sharing a database are isolated at the table level and a store can only reach its own registry (idempotentcreate_allinstartup(), matching the segment store's schema-management convention). Storage keys aref"{namespace}/{name}"— an implementation detail; "/" is outside the identifier charset, so distinct pairs can never collide ("__" would be ambiguous). The primary key is the concurrency arbiter:register= plain INSERT;IntegrityError->CollectionAlreadyRegisteredError(the patternSQLAlchemySegmentStore.create_partitionalready uses)get_or_register= SELECT fast path, then nativeINSERT .. ON CONFLICT DO NOTHING; on conflict re-SELECT the winner — no exception-driven control flowderegister= single DELETEEntries round-trip through pydantic (
dump_python(mode="json")/validate_python);get_or_registerreturns the serialization round trip of the input, so the registering store sees exactly what every later reader gets. Supported dialects: PostgreSQL and SQLite (the two relational providers the server offers). The 32-byte registry-name limit keepscollection_registry_<name>within PostgreSQL's 63-byte identifier limit. No new dependencies.Adopters in this stack: Qdrant (next PR) and Milvus (final PR). The SQLite vector stores keep their own
_CollectionRowbookkeeping — their state is process-local anyway, so the registry buys them nothing today.Design doc:
design/collection_registry.md— contributor-facing design notes live in the rootdesign/directory, outside the deployed docs site (docs/) and outside packaged sources.Fixes/Closes
Fixes #1524
Type of change
How Has This Been Tested?
server_tests/memmachine_server/common/vector_store/test_sqlalchemy_collection_registry.py, parametrized over SQLite (file) and PostgreSQL (testcontainer,integration-marked): startup idempotence (twice, and a second instance on the same engine), entry round trips, duplicate registration (same and different entry), get_or_register canonicalization and no-comparison semantics, idempotent deregister, registry isolation on a shared engine, key-ambiguity regression (("a__b","c") vs ("a","b__c")), identifier/name/engine validation, the canned v1-row readability guard, and concurrency tests (two instances over one engine,asyncio.gather): concurrent register -> exactly oneAlreadyRegistered; concurrent get_or_register with mismatched entries -> both callers observe the same stored entry, exactly oneregistered=True.Test Results: 45 passed (SQLite + PostgreSQL) locally;
ruff check,ruff format --check, andty check packagesclean (no new diagnostics).Checklist
Further comments
Design notes for the reviewer:
design/collection_registry.mdrecords the rejected generic draft and the reasoning, so the narrowing reads as deliberate rather than as a missed abstraction.create_all-on-startup.DROP TABLE.collection_registry_*tables for observability, and management operations are exactly the power stores must not have.