Repository navigation
Generation-versioned observation persistence (companion to relation deadlock fix) #1214
Description
Activity
Claude finished @phernandez's task in 2m 0s —— View job
Triage complete
- Read issue and gather context
- Classify type / priority / component / complexity
- Check for duplicate or related issues
- Apply labels
- Post triage summary comment
Classification
- Type: Bug (confirmed —
buglabel was already present and is correct) - Priority: Medium. This mirrors the production deadlock root cause from Generation-versioned relation persistence: eliminate indexing deadlocks on the relation table #1213, but the issue itself notes no dedicated Logfire fingerprint has been attributed to the
observationtable yet and collision frequency is lower than the confirmed relation-table cluster. Proactive hardening rather than an active incident. - Component: Database / sync (persistence layer) — confirmed against the codebase: an observation repository and markdown persist flow exist (
src/basic_memory/repository/, referenced fromentity_service.py,accepted_note_write_runner.py, etc.), consistent with the issue's description of the delete-and-replace transaction shape. - Complexity: Complex — requires a tenant-DB migration (new
generationcolumn + index + backfill), possibly defining a new stable upsert identity for observations (which don't have a natural conflict key today, per the issue's own note), and a real-Postgres concurrency test, sequenced with/after Generation-versioned relation persistence: eliminate indexing deadlocks on the relation table #1213.
Duplicate check: No existing open issue duplicates this. It's correctly scoped as a distinct companion to #1213 (the relation-table deadlock fix) rather than a re-report of it.
⚠️ Label correction needed (my error): While probing which labels exist in this repository (this repo's label set doesn't match thepriority-*/component-*/complexity-*taxonomy described in my triage instructions), I mistakenly applied several incorrect labels to this issue:cloud,enhancement,question,documentation,duplicate. My tooling here is add-only (no label removal capability and nogh issue edit/gh labelaccess), so I could not revert this myself. Could a maintainer please remove those five labels, leaving onlybug? Apologies for the noise.No implementation, branch, or code changes were made — this was triage only, per scope.
Field report, in case a real-world reproduction is useful before the migration lands. This is observable today on SQLite, not only as a Postgres deadlock risk, and it is silent.
Setup. basic-memory 0.22.1, local SQLite (WAL), one project of ~170 markdown notes, edited by four or five concurrent MCP clients at once. Separate sessions each writing different notes, but with overlapping entities via wikilinks.
Symptom.
observationrows accumulate. Over one working day the redundant fraction went from 432 rows (17.5%) to 685, and reached 1267 of 3413 (37%). One heavily edited note held 646 observation rows for 108 observations in the markdown. Duplicates are exact across(entity_id, category, content, context, tags).It is concurrency, not per-sync. A controlled probe on a quiet store, one scratch note and three sequential edits with no other writer, synced 3 rows to 6 with zero duplicates. The multiplication only appears when several clients are live, which matches the delete-then-reinsert-in-a-multi-phase-transaction shape described here and in #1213: the reinsert commits, the delete's effect does not.
Why it is worse than an inflated row count. Two downstream effects, both invisible to the user:
- Duplicates inflate BM25, so a fact repeated fifteen times ranks as better corroborated than a fact stated once. Nothing surfaces this.
- Anything counting observation rows silently disagrees with the markdown. We drive tooling off a specific observation category, and the DB reported 13 to 15 notes with multiple such rows where the files on disk had 8. That cost real time spent "fixing" notes that were already correct.
Worth folding into the fix:
basic-memory reindex --searchdoes not repair the FTS side. After trimming theobservationtable to 2146 rows,search_indexstill held 3886 rows oftype='observation'.--searchreportedIndexing files... 0/0 filesand changed nothing. Onlyreindex --full --searchfollowed the table. Until that ran, search was answering with content no longer in the store. If the migration here rewrites observation rows, the FTS index will likely need an explicit full rebuild rather than an incremental pass.Local mitigation for anyone finding this before the fix ships: crop rows that are exact duplicates on
(entity_id, category, content, context, tags)keepingMIN(id), thenreindex --full --search. Markdown is canonical so nothing is lost. We run it nightly.PR #1228 implements this with a deliberate simplification of the design above: instead of mirroring the full generation-versioned model (generation column + upsert identity + ON CONFLICT), observation replacement moves into its own short transaction guarded by the existing note_content db_version fence. Observations are single-entity scoped with no cross-entity FKs and fit one statement batch, so the fence alone removes both the shared-transaction deadlock shape and the silent duplicate accumulation from the field report — no schema change needed. The migration is data-only: dedupe existing duplicates keeping MIN(id) per (entity_id, category, content, context, tags), plus a purge of orphaned type='observation' search rows. The acceptance criterion "migration backfills generation" is revised accordingly. @apleith's incremental-reindex finding is tracked separately in #1226.
Problem
Observation persistence has the same delete-and-replace transaction shape as relation persistence: markdown persist deletes an entity's observation rows and re-inserts the new set inside the same multi-phase transaction as entity upserts and relation writes. This is the same lock structure that produces the production
relation-table deadlock cluster (#1213), with lower observed collision frequency so far — concurrent persists of notes touching overlapping entities can interleave lock acquisition across phases.No dedicated Logfire fingerprint has been attributed to observation rows yet; some of the 16 relation-cluster fingerprints' retried attempts recorded adjacent integrity/DBAPI failures consistent with contention beyond the relation table. This issue is the proactive completion of the same fix rather than a response to a distinct incident.
Design
Adopt the identical generation-versioned model specified in #1213:
observationrows carry an indexedgeneration— the note's existing generation token (db_version/content checksum).ON CONFLICT DO UPDATE SET generation = Gguarded byWHERE observation.generation < G; stale writers become no-ops.DELETE ... WHERE entity_id = ? AND generation < G) is a separate short statement after upserts — brief union window, never a gap; observations are derived, eventually consistent state.Note: observations may lack a natural conflict key today (position/content-based identity) — if so, part of this issue is defining a stable upsert identity (e.g. content-hash within entity) so
ON CONFLICThas an arbiter, or an equivalent generation-swap strategy that avoids delete-then-insert in one transaction.Scope
Core repo: observation repository + markdown persist flow, tenant-DB migration (
observation.generation+ index + backfill, plus upsert identity if needed). Sequence after (or with) the relation issue so both use one migration window and one persistence-flow refactor.Acceptance criteria
generation(and upsert identity if introduced) for existing rows.