diff --git a/docs/sync-and-op-log/vector-clocks.md b/docs/sync-and-op-log/vector-clocks.md index fcef56bdda..ef6761edca 100644 --- a/docs/sync-and-op-log/vector-clocks.md +++ b/docs/sync-and-op-log/vector-clocks.md @@ -237,7 +237,30 @@ To make that symmetric, the `operations` row stores: The lookups in `conflict.ts` match a requested entity as the scalar `entity_id` **or** a member of `entity_ids`: -- `detectConflictForEntity` (single) — Prisma `where: { OR: [{ entityId }, { entityIds: { has: entityId } }] }`, ordered by `server_seq`. The `OR` spans the `entity_id` btree and the `entity_ids` GIN, so the planner uses a `BitmapOr` + sort bounded by the entity's stored version depth (op-log pruning keeps that small). If a real-Postgres `EXPLAIN` on a deep-history entity ever shows this hot path is a problem, the escalation is two ordered `LIMIT 1` lookups (scalar btree + `entity_ids` GIN) taking the higher `server_seq` — the array side stays small because the column is multi-entity-only. +- `detectConflictForEntity` (single) — **two separately-indexed lookups, never one combined filter.** A scalar `findFirst` on `{ userId, entityType, entityId }` ordered by `server_seq` (served end to end by the `(user_id, entity_type, entity_id, server_seq)` btree), plus a raw-SQL `MATERIALIZED` CTE taking `MAX(server_seq)` over `entity_ids @> ARRAY[id]`, with the winning row then fetched by the `(user_id, server_seq)` unique key. + + > ⚠️ **Do not "simplify" this back into one query.** It used to be + > `where: { OR: [{ entityId }, { entityIds: { has: entityId } }] }` + `orderBy: { serverSeq: 'desc' }`, + > and on 2026-07-20 that caused a total sync outage — 47 stuck backends, longest 75 minutes, + > 61/66 connections consumed. The `OR` spans two different indexes and GIN cannot supply + > `server_seq` ordering, so the planner abandons **both** index paths and walks the user's + > history. Nothing bounds that walk when the entity has no matching rows — i.e. the + > first-ever op for a new task, the most common upload there is. Op-log pruning does **not** + > bound it; that assumption is what this paragraph used to assert, and it was wrong. + > + > The obvious escalations are broken too. Two ordered `LIMIT 1` lookups still leave the + > array side unable to order on GIN. Measured under generic planning on a 40k-row seed, the + > outage query, the naive array-only `LIMIT 1`, the flat `MAX`, Prisma's `aggregate({ _max })` + > and the CTE with `MATERIALIZED` dropped **all** read the user's whole entity-type slice — + > 816 blocks and 2500 rows discarded, against 143 blocks and 0 discarded for the shipped form. + > + > Measure any change here with `SET plan_cache_mode = force_generic_plan`. Prisma sends + > parameterized prepared statements, so Postgres settles on a **generic** plan after ~5 + > executions, and `EXPLAIN` with literal constants builds a custom plan that makes every one + > of those broken shapes look perfect. See + > `packages/super-sync-server/tests/conflict-entity-lookup-plan.pglite.spec.ts` and the note + > at `detectConflictForEntity` in `packages/super-sync-server/src/sync/conflict.ts`. + - `detectConflictForEntities` / `prefetchLatestEntityOpsForBatch` (batch) — raw SQL unnesting `CASE WHEN cardinality(entity_ids) > 0 THEN entity_ids ELSE ARRAY[entity_id] END`, with a `entity_ids && ... OR entity_id = ANY(...)` prefilter so the `GIN(entity_ids)` index (migration `20260613000001`) and the existing `entity_id` btree stay usable. **Forward-only by design:** rows written before migration `20260613000000` have an empty `entity_ids` array and fall back to the scalar `entity_id` (= first entity) in the `CASE` expression / scalar branch above — there is no `UPDATE` backfill. Entities 2..n of already-stored multi-entity ops were never persisted and are unrecoverable, so they remain invisible to conflict detection until that entity gets a fresh write. This residual is bounded: client-side LWW is unaffected (the client persists the full op and `VectorClockService.getEntityFrontier()` fans each op out to **every** entity), and the server only builds an authoritative snapshot from non-encrypted ops (`replayOpsToState()` throws on encrypted ops), so the pre-fix gap could only surface a stale value to a fresh client on non-encrypted self-hosted servers. diff --git a/packages/super-sync-server/tests/conflict-entity-lookup-plan.pglite.spec.ts b/packages/super-sync-server/tests/conflict-entity-lookup-plan.pglite.spec.ts index 5b05a73008..85ce5d8535 100644 --- a/packages/super-sync-server/tests/conflict-entity-lookup-plan.pglite.spec.ts +++ b/packages/super-sync-server/tests/conflict-entity-lookup-plan.pglite.spec.ts @@ -46,11 +46,14 @@ import type { Operation } from '../src/sync/sync.types'; * inserts land in the GIN pending list (fastupdate defaults to on) which `@>` must * scan linearly: same data, same query, 140 blocks and a 14x larger index (1120 kB * vs 72 kB). VACUUM flushes the pending list and restores 2 blocks, so the real cost - * OSCILLATES between the two across the autovacuum cycle, bounded above by - * gin_pending_list_limit rather than by anything in this seed. This suite therefore - * seeds in production order and never vacuums: it measures the WORST realistic - * steady state, which is what a regression budget should sit above. Setting - * fastupdate=off removes the oscillation and pins the array branch at 2 blocks. + * OSCILLATES across the autovacuum cycle. This suite therefore seeds in production + * order and never vacuums, measuring the dirty end of that cycle rather than the + * freshly-vacuumed end — but 140 is NOT a ceiling: the pending list is bounded by + * gin_pending_list_limit (4MB default), not by anything in this seed, so a + * production write burst can be worse. The budget below is calibrated to catch the + * mis-plan, NOT to certify a maximum. Setting fastupdate=off stops new entries + * queueing — it does not flush what is already pending, which still needs a VACUUM — + * and so removes the oscillation going forward. * * REMAINING FIDELITY LIMIT: PGlite is not the production cluster — different major * version, and it reports every block as a cache hit, so these counts cannot model @@ -488,6 +491,8 @@ describe('detectConflictForEntity behaviour is unchanged by the query split (PGl const TIME_DELTA_ACTION = '[TimeTracking] Sync time spent'; const OTHER_USER_ID = 7; + /** Own tenant, so the legacy-misc row seeded for USER_ID cannot mask the gate. */ + const POST_SPLIT_USER_ID = 8; const seed = async (op: { id: string; @@ -778,7 +783,16 @@ describe('detectConflictForEntity behaviour is unchanged by the query split (PGl it('does not alias a POST-split misc write onto tasks', async () => { // The alias is gated on the fixed v1→v2 split boundary; a v2+ misc write is // disjoint from tasks and must not fabricate a conflict. + // + // MUST probe exactly 'tasks'. detectConflictForEntity enters the legacy-misc branch + // only for entityId === 'tasks', so probing any other id (this once read + // 'tasks-v2-only') never reaches the gate and passes no matter what the gate does — + // verified: breaking it to `lte` left all 915 tests green. + // + // Runs as its OWN user because the preceding test leaves a schema_version 1 misc row + // for USER_ID in the shared table, which would legitimately alias and mask this. await seed({ + userId: POST_SPLIT_USER_ID, id: 'op-modern-misc', serverSeq: 11, clientId: 'other', @@ -789,7 +803,8 @@ describe('detectConflictForEntity behaviour is unchanged by the query split (PGl }); expect( - (await detect('tasks-v2-only', { entityType: 'GLOBAL_CONFIG' })).hasConflict, + (await detect('tasks', { entityType: 'GLOBAL_CONFIG' }, POST_SPLIT_USER_ID)) + .hasConflict, ).toBe(false); }); });