Skip to content

perf(retain): store-owned backend writes facts once with entities inline - #3496

Open
nicoloboschi wants to merge 1 commit into
mainfrom
perf/store-owned-retain-single-write
Open

perf(retain): store-owned backend writes facts once with entities inline#3496
nicoloboschi wants to merge 1 commit into
mainfrom
perf/store-owned-retain-single-write

Conversation

@nicoloboschi

Copy link
Copy Markdown
Collaborator

Problem

For a memories store that owns its rows (external backend), the connection-free store phase of retain writes each memory twice: insert_facts stages it without entities, then record_unit_entities reads the just-written records back (full vectors) and re-upserts them with entity ids — because entity ids resolve onto real unit ids only after those ids exist.

On an object-store-backed engine, that reattach is a second full write per memory plus a read-back, doubling the round-trips on the slow path — which is the dominant cost of retain (measured ~2.9 s p50 / ~11 s p99 per write-group under load).

Change

Use the deferred single-write path the provider already supports:

  1. insert_facts_batch(defer_index=True) — mint unit ids, no write
  2. remap Phase-1 entities onto the ids
  3. index_facts(..., unit_entity_ids=…, txn=ext_txn)one write with entities inline, tagged with the write-group txn (commits atomically with the group; postings are now witness-covered instead of riding an uncovered seam)

Co-occurrence accumulation still runs — record_unit_entity_postings gains store_write=False (co-occurrence only, since the row is already correct).

Scope: streaming ext path only; delta path unchanged; no-op for the Postgres store.

Effect

Store-owned retain object-store round-trips per doc ~6 → ~4 (removes a full-vector GET + a re-upsert). Targets the retain write-group latency directly.

Tests

test_retain_ext_writegroup.py updated + green: the single write via index_facts is asserted connection-free and txn-tagged; the posting runs store_write=False; delta path still store_write=True.

A memories store that owns its rows (external backend) retains a document in a
connection-free store phase. That phase wrote each memory TWICE: insert_facts
staged it without entities, then record_unit_entities read the just-written
records back (full vectors) and re-upserted them with entity ids attached —
because entity ids can only be resolved onto real unit ids after they exist.

On an object-store-backed engine that reattach is a second full write per memory
plus a read-back, doubling the round-trips on the slow path (the dominant cost of
retain). It's avoidable: mint the ids without writing (insert_facts_batch with
defer_index), remap the entities, then write once via index_facts with the entity
ids already inline — tagged with the write-group txn so it commits atomically with
the group (and, as a bonus, the postings are now witness-covered instead of riding
an uncovered seam). Co-occurrence accumulation still runs (record_unit_entity_postings
gains store_write=False: co-occurrence only, since the row is already correct).

Streaming ext path only; the delta path is unchanged. Tests updated: the single
write via index_facts is asserted connection-free and txn-tagged, and the posting
runs store_write=False. No behavior change for the Postgres store (a no-op there).
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.

1 participant