Repository navigation
Conversation
marvinyu-memverge
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed at fc2565f. The drop itself is clean. Nothing reads either column; the writes this PR removes were the only references at head. delete_partition and _unregister_where delete the registry row in the same transaction, so with this change the caller's key leaves the database with the deletion. One question, on upgrades.
segment_store_gc, withpartition_keyNOT NULL, already ships in v0.4.0 (released 10-08,lateston Docker Hub). The wizard's default config puts event memory's segment store on SQLite, so a default v0.4.0 install has this table.startuponly runscreate_all, which leaves an existing table as it is. After an upgrade to a release with this change, the queue insert indelete_partitionfails theNOT NULLcheck and rolls back the whole transaction. Every partition deletion then fails until someone runs theALTERfrom the description. How will a v0.4.0 install get thatALTER? (collection_registry_gcis only onfeat/horizontal-scalingand in the 0.4.1-dev0/dev1 tags, so outside those it doesn't have this problem.)
Verified:
- No code at head reads
segment_store_gc.partition_keyorcollection_registry_gc.name. The registry's purge selectsnamespaceandconfig. - In v0.4.0,
partition_keyhas no index or constraint beyondNOT NULL, so SQLite (3.35+) can drop it with the statement in the description.
xiongzubiao
force-pushed
the
feat/horizontal-scaling
branch
from
October 15, 2026 15:31
101182c to
cf7ae15
Compare
The purge reads an entry's incarnation, enqueue stamp, and cursor. The partition key was written for forensics and read by nothing. Drop the column, so an entry holds only what the purge reads; the tests that queue an entry by hand stop setting it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A purge round locates a dead incarnation's records by its namespace and configuration. The name was kept for inspection and read by nothing. Drop the column, so a tombstone holds only what the purge reads; the tests that queue a tombstone by hand stop setting it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
edwinyyyu
force-pushed
the
fix/tombstones-without-logical-key
branch
from
October 15, 2026 21:36
fc2565f to
5ebba15
Compare
Contributor
Author
|
@marvinyu-memverge Existing databases, v0.4.0 installs included, are left to the repo-wide schema migration in #1825 and #1826 rather than special-cased here, so the description no longer carries the The branch is rebased onto the current 🤖 Written by Claude Code (Claude Opus 5.5) on behalf of @edwinyyyu. |
marvinyu-memverge
approved these changes
Oct 15, 2026
malatewang
approved these changes
Oct 15, 2026
edwinyyyu
merged commit Oct 16, 2026
a67efd1
into
MemMachine:feat/horizontal-scaling
39 checks passed
This was referenced Oct 19, 2026
[qdrant options] Let a deployment tune a Qdrant collection's HNSW, optimizers and quantization
#1618
Draft
This was referenced Oct 19, 2026
Draft
[event memory 2/6] Rename the segment store to the event memory store (port of #1659's rename)
#1686
Draft
Open
Draft
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
Both purge queues stored the deleted partition's or collection's logical key next to its incarnation:
segment_store_gc.partition_keyandcollection_registry_gc.name. Neither purge reads it.namespaceandconfig. The name was kept for inspection.A tombstone also outlives the data it tracks, and the key is caller-provided. Some data protection regulations may prohibit keeping caller-provided values, the partition key and collection name included, on a tombstone after the data is deleted.
This PR drops both columns, so a tombstone holds only what its purge reads. The deletions stop writing the columns, the tests that queue a tombstone by hand stop setting them, and the design docs describe the queues without them.
Databases created before this change are left to the repo-wide schema migration (#1825, #1826).
Stack
Directly on
feat/horizontal-scaling.Verification
At this PR's head
5ebba1554, rebased ontofeat/horizontal-scalingatcf7ae1579on 2026-10-15, with both patches unchanged fromfc2565fa9:ruff checkandruff format --checkare clean,uv lock --checkpasses, andty checkis clean as CI runs it (uv run --frozen --all-extras ty check --project packages/server).episodic_memory/event_memory/segment_storeandcommon/vector_store), unit and integration: 897 passed, 5 skipped.🤖 Generated with Claude Code