docs(rfc):memory capacity and sharding - #1387
Conversation
Teingi
left a comment
Teingi
left a comment
There was a problem hiding this comment.
Thanks for writing this up. Bounded shards and stable entry ownership are useful directions, but the RFC still has several correctness and liveness gaps that should be resolved before acceptance. Because this PR defines design rather than implementing the open bug, please also change Closes #1321 to Refs #1321, add both RFC pages to the English and Chinese navigation, and bring the Chinese text fully back into sync with the English document.
| ### Identity | ||
|
|
||
| - Memory's persistent identity remains `(family, artifact_id)`; the routing domain is only a discovery set derived from the configured root. | ||
| - Shard ID resolution first matches the exact root, then applies a strict `root.s([1-9][0-9]*)` full match; ordering uses the parsed integer, forbidding accidental prefix matches. |
There was a problem hiding this comment.
The generated names and this parser disagree: root.s0001 does not match root.s([1-9][0-9]*) under a full match (root.s1 does). The first automatic split would therefore create a shard that routing cannot rediscover. Please define one canonical suffix grammar and use it for both generation and parsing.
|
|
||
| ### Identity | ||
|
|
||
| - Memory's persistent identity remains `(family, artifact_id)`; the routing domain is only a discovery set derived from the configured root. |
There was a problem hiding this comment.
Deriving membership solely from Artifact ID spelling can absorb an existing valid Artifact. For example, memory.s0001 is already a valid Memory ID; after configuring memory as the root, it would suddenly join this routing domain even if it was created independently. Please persist explicit routing membership or ownership, including upgrade conflict handling, rather than treating the suffix as proof of ownership.
| - when the root exists, lock the root head row right after the outer transaction begins, then read all legal shard heads by ordinal; | ||
| - when the root does not exist, protect bootstrap with a database-supported `SERIALIZABLE` or equivalent predicate-conflict mechanism; a non-existent row is not a lockable object; | ||
| - every writer that changes the routing domain — explicit shard writes, split, flush, organize, and new-shard creation — first acquires the same barrier; | ||
| - head reads, candidate generation, global deduplication seeding, per-shard plans, ordinal-ordered apply, and cursor save all use the same bound connection; |
There was a problem hiding this comment.
This keeps the database barrier and connection open across model candidate generation, whose latency and retry duration are unbounded. That can block every writer in the routing domain. Please read an immutable head vector, run extraction outside the transaction, then acquire a short barrier, re-enumerate and recheck every head, and commit atomically.
|
|
||
| ## Flush: single-shard semantics extended | ||
|
|
||
| Source flush first reads all current heads of the routing domain. The candidate pipeline receives the active entries of all shards at once; its input is not truncated by this RFC, and candidate rules do not change with shard count. When the model proposes a revise, it must specify both `memory_artifact_id` and `entry_id`; an add carries no target identity. |
There was a problem hiding this comment.
Sharding bounds each Artifact, but this call still receives an unbounded domain-wide union. Once it exceeds the model context, the same Source window will fail on every retry and the cursor cannot advance. Please make selection token-budgeted and bounded, with a progress-safe fallback; global exact deduplication can remain server-side through canonical hashes and bytes.
|
|
||
| All write paths that construct a new Memory Revision apply the same final rule to the resulting manifest: first compact droppable inactive tombstones according to the threshold and capacity conditions, then check `len(final_manifest) <= memory_manifest_max_entries`. The capacity check happens before embedding/projection preparation; a failure writes no Artifact, entry, projection, or index. | ||
|
|
||
| `memory_tombstone_compaction_threshold` controls whether compaction is triggered by tombstone count, and `memory_compaction_max_drops` only bounds the internal drop batch of one Revision; it does not limit the number of remember or candidate inputs. Legitimate large batches are not implicitly truncated; only a final manifest over budget fails as a whole. |
There was a problem hiding this comment.
max_drops can prevent compaction from making progress. With capacity 200, 200 existing entries, 10 droppable tombstones, max_drops=5, and six conditional adds, compaction leaves 195 entries, the adds produce 201, and the all-or-nothing rollback restores all ten tombstones. Retrying repeats forever even though enough entries are droppable. Under pressure, please allow enough drops to fit the request or define a separately committable compaction step.
|
|
||
| ### Manifest 与版本 | ||
|
|
||
| 每个 shard 继续使用 flat-v1：manifest 只保存 `entry_id`、`entry_version_id`、`entry_content_hash` 和 `state`，正文在不可变 `MemoryEntryVersion` 中。最终 manifest 的 entry 数不得超过 `memory_manifest_max_entries`（默认 200，`ge=1`）；条目数是容量判断维度。 |
There was a problem hiding this comment.
The translations are not currently synchronized: the Chinese RFC jumps from the capacity limit directly to Candidate identity and omits the English rationale and measurements for the default of 200. Please carry the same normative rationale into both documents, and clean up the incomplete wording near the Summary (以及。) before acceptance.
2 participants
Which issue or RFC does this PR close?
Closes #1321
Rationale for this change
Currently, because the manifest persists a full snapshot on every write, storage and write costs grow super-linearly, and in extreme cases the system can run beyond its practical limits. RFC 0014 explicitly listed Memory splitting, inactive tombstone compaction, and a shared routing manifest as unresolved. To contain these extreme cases, this RFC caps unbounded growth while preserving the manifest semantics, offering a concise solution under the conditions it defines.
What changes are included in this PR?
memory_manifest_max_entries, default 200) and a routing domain derived from the configured root ID through strictroot.sNNNNsuffixes; no schema migration, no entry migration, and no change to the flat-v1 persistence shape.expected_revision, citation-driven revise/retire/reactivate, and organize retain their owner shard and return the stablememory_capacity_exceedederror when capacity is insufficient.(memory_artifact_id, entry_id)to prevent cross-shard target misidentification.dropchanges with no cooldown period.Are there any user-facing changes?
No released behavior changes.
How was this change tested?
make check
AI usage statement
Claude Code was used in this change to analyze the Memory service implementation and the related design discussions, and to assist in drafting and cross-checking the synchronized English and Chinese RFC documents. The proposal was submitted by the author for review and revision by maintainers within the RFC process.