From 2d9988dd73a8ce9dbe0dc89612cf6c4d2e7c6884 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Fri, 15 May 2026 17:24:16 +0200 Subject: [PATCH] perf(sync): SuperSync server speed + correctness hardening (#7621) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * docs(sync): add super sync server perf plan * perf(sync): implement supersync server perf phases * fix(sync): bracket auth cache invalidation * fix(sync): avoid empty replay state stringify * fix(sync): harden supersync batch uploads * fix super sync review findings * fix(sync): guard payload bytes backfill rollout * perf(sync): speed up payload_bytes backfill and index its scan Raise the backfill batch size (DEFAULT 5->500, MAX 25->1000) so a 100M-row operations table backfills in minutes rather than tens of hours. Add a CONCURRENTLY partial index on (user_id, id) WHERE payload_bytes = 0: it drains to empty post-backfill so the boot-time backfill self-check and the BOOL_OR quota probe stop doing a full sequential scan to prove absence, and it makes the backfill's per-user keyset paging a true index seek. Wire the new concurrent-index migration into both deploy scripts' P3018 recovery path. Add migration-SQL guard tests for the ADD COLUMN (metadata-only fast path) and the new partial index. * fix(sync): bound auth cache invalidation map and bracket every delete The auth verification cache's invalidationVersions map grew one entry per lifetime-invalidated user with no eviction (unbounded heap on a long-lived single replica). Cap it at the same 10k LRU bound as the entries map, re-inserting the just-invalidated user at the MRU tail so the CAS race protection still holds for the only window that matters (one DB round trip). Bracket the passkey/magic-link registration cleanup deletes with pre+post invalidate to match the documented convention, and invalidate on verifyEmail so a freshly-verified user isn't denied for up to the cache TTL. * perf(sync): skip the redundant exact replay-state measurement The delta accounting is a proven over-estimate of the serialized state size, so when the running bound stays within the cap the true size is too and the final exact JSON.stringify is provably redundant. Skip it in that case (still measure-and-throw whenever the bound does not prove safety). This collapses the common small/incremental replay back to zero expensive full stringifications, matching the old per-op loop instead of regressing it. Name the entity-key JSON overhead constant and document that assertReplayStateSize's return value is load-bearing. * refactor(sync): split processOperationBatch into pipeline stages Extract the 297-line batch upload method into a thin orchestrator plus six named single-responsibility stage helpers (validate+clamp, intra- batch dedupe, classify existing duplicates, conflict-detect, reserve seq + insert, full-state clock). Behavior-preserving: every stage writes terminal rejections into the shared results array by index and the two empty-set guards short-circuit exactly as before. Also share the timestamp clamp, the duplicate-op SELECT, and the merged full-state clock persistence between the batch and legacy paths so they cannot silently diverge. * test(sync): pin batch error-code divergence and aggregate-once Strengthen the intra-batch duplicate test to assert same-id / different-content yields DUPLICATE_OPERATION (deliberate divergence from the legacy INVALID_OP_ID), and document the divergence in the plan. Replace the single-full-state aggregate test with two full-state ops + a spy asserting _aggregatePriorVectorClock runs exactly once and last-write-wins — the old test could not catch a per-op-aggregate regression. Add a makeOp fixture factory. Correct the plan's overstated replay-stringification numbers. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: johannesjo <1456265+johannesjo@users.noreply.github.com> --- ARCHITECTURE-DECISIONS.md | 46 + .../2026-05-14-super-sync-server-perf.md | 296 +++++++ packages/super-sync-server/Dockerfile | 4 +- packages/super-sync-server/README.md | 32 + packages/super-sync-server/docker-compose.yml | 4 +- .../helm/supersync/templates/configmap.yaml | 2 + .../helm/supersync/templates/deployment.yaml | 2 +- .../helm/supersync/values.yaml | 12 +- packages/super-sync-server/package.json | 4 +- .../migration.sql | 4 + .../migration.sql | 2 + .../migration.sql | 14 + .../super-sync-server/prisma/schema.prisma | 1 + packages/super-sync-server/scripts/deploy.sh | 174 ++++ .../scripts/migrate-deploy.sh | 116 +++ .../scripts/migrate-payload-bytes.ts | 157 ++++ packages/super-sync-server/src/api.ts | 6 + packages/super-sync-server/src/auth-cache.ts | 101 +++ packages/super-sync-server/src/auth.ts | 37 +- packages/super-sync-server/src/config.ts | 18 + packages/super-sync-server/src/passkey.ts | 10 + packages/super-sync-server/src/server.ts | 25 +- .../src/sync/services/snapshot.service.ts | 96 +- .../sync/services/storage-quota.service.ts | 143 ++- .../super-sync-server/src/sync/sync.const.ts | 9 +- .../super-sync-server/src/sync/sync.routes.ts | 35 +- .../src/sync/sync.service.ts | 824 ++++++++++++++++-- .../super-sync-server/src/sync/sync.types.ts | 2 + packages/super-sync-server/src/test-routes.ts | 6 + .../tests/auth-cache.spec.ts | 150 ++++ .../super-sync-server/tests/config.spec.ts | 37 + .../tests/migration-sql.spec.ts | 147 ++++ .../tests/snapshot.service.spec.ts | 72 ++ .../tests/storage-quota-cleanup.spec.ts | 39 +- .../tests/storage-quota.service.spec.ts | 191 ++-- .../tests/sync-compressed-body.routes.spec.ts | 36 +- .../tests/sync.service.spec.ts | 481 +++++++++- 37 files changed, 3071 insertions(+), 264 deletions(-) create mode 100644 docs/plans/2026-05-14-super-sync-server-perf.md create mode 100644 packages/super-sync-server/prisma/migrations/20260514000000_add_encrypted_ops_partial_index/migration.sql create mode 100644 packages/super-sync-server/prisma/migrations/20260514000001_add_operation_payload_bytes/migration.sql create mode 100644 packages/super-sync-server/prisma/migrations/20260514000002_add_payload_bytes_unbackfilled_index/migration.sql create mode 100644 packages/super-sync-server/scripts/migrate-deploy.sh create mode 100644 packages/super-sync-server/scripts/migrate-payload-bytes.ts create mode 100644 packages/super-sync-server/src/auth-cache.ts create mode 100644 packages/super-sync-server/tests/auth-cache.spec.ts diff --git a/ARCHITECTURE-DECISIONS.md b/ARCHITECTURE-DECISIONS.md index 14dd61a02f..b525a867b8 100644 --- a/ARCHITECTURE-DECISIONS.md +++ b/ARCHITECTURE-DECISIONS.md @@ -118,6 +118,52 @@ sync primitives. --- +### 4. Batch Uploads Under RepeatableRead + +**Status**: ✅ Active (since May 2026) + +**Decision**: SuperSync batch uploads derive conflict-safety from the shared +`user_sync_state.lastSeq` row write that reserves server sequence numbers, not +from PostgreSQL RepeatableRead snapshot isolation alone. + +**Rationale**: + +- PostgreSQL RepeatableRead does not provide full serializable snapshot isolation +- Two concurrent upload transactions can both pass conflict prefetch checks when + they read the same pre-insert snapshot +- Reserving sequence numbers through one `user_sync_state.lastSeq` row forces + accepted writers for the same user to serialize on that row lock +- If two batches race, the later writer blocks on the row and the transaction + retry path handles the serialization failure rather than silently accepting + conflicting operations + +**Implementation**: + +- Batch upload conflict detection runs in memory against prefetched latest + entity rows and updates that map as operations are accepted +- Accepted operations reserve one contiguous sequence range with + `INSERT ... ON CONFLICT ... DO UPDATE SET last_seq = last_seq + delta` +- The batch insert does not use `skipDuplicates`; an unexpected unique conflict + aborts the transaction and lets the request retry +- Removing or sharding the `lastSeq` write requires replacing this safety + mechanism with an equivalent per-user serialization primitive + +**Documentation**: [`docs/sync-and-op-log/diagrams/02-server-sync.md`](docs/sync-and-op-log/diagrams/02-server-sync.md) + +**Key Files**: + +- [`packages/super-sync-server/src/sync/sync.service.ts`](packages/super-sync-server/src/sync/sync.service.ts) - Upload transaction and batch primitive +- [`packages/super-sync-server/prisma/schema.prisma`](packages/super-sync-server/prisma/schema.prisma) - `user_sync_state.last_seq` + +**When to Update This Pattern**: + +- Changing upload conflict detection +- Changing server sequence assignment +- Changing transaction isolation for upload operations +- Introducing multi-writer or multi-region upload processing + +--- + ## How to Use This Document ### When Making Architectural Changes diff --git a/docs/plans/2026-05-14-super-sync-server-perf.md b/docs/plans/2026-05-14-super-sync-server-perf.md new file mode 100644 index 0000000000..f665681d26 --- /dev/null +++ b/docs/plans/2026-05-14-super-sync-server-perf.md @@ -0,0 +1,296 @@ +# SuperSync Server Performance Improvements + +Date: 2026-05-14 +Status: proposal — phases sequenced so independent low-risk wins land first while the large upload-batching work proceeds in parallel. + +Scope drawn from an audit of `packages/super-sync-server/` covering: upload processing, snapshot generation/replay, quota accounting, encrypted-op handling, auth, and deployment defaults. + +> **Revision note (post-review):** Phases 0b, 1, 2 and 4 were tightened after a subagent review surfaced design issues in the original draft. Specifically: a forgotten `userSyncState.upsert` for first-time users, intra-batch duplicate `op.id` handling, multi-entity (`entityIds[]`) op support, full-state-op aggregate-VC writes, and a `pg_column_size` vs. `computeOpStorageBytes` mismatch in the quota backfill. See each phase for the revised approach. + +--- + +## Phase 0 — Quick wins (one PR each, mostly low risk) + +### 0a. Encrypted-op partial index (Finding #4) + +- **New migration:** `prisma/migrations/_add_encrypted_ops_partial_index/migration.sql` + ```sql + DROP INDEX CONCURRENTLY IF EXISTS "operations_user_id_server_seq_encrypted_idx"; + CREATE INDEX CONCURRENTLY "operations_user_id_server_seq_encrypted_idx" + ON "operations"("user_id", "server_seq") + WHERE "is_payload_encrypted" = true; + ``` +- **Why:** `snapshot.service.ts:1083` and `:1114` `count(*) WHERE is_payload_encrypted=true` over a seq range. Today this scans the range and filters. With the partial index, the common case (no encrypted ops for the user) becomes an empty-index probe. +- **Leave alone:** existing `operations_user_id_full_state_server_seq_idx` already covers the `op_type IN (...)` filter for the `findFirst` at `snapshot.service.ts:1103`. (That `findFirst` also filters `isPayloadEncrypted: false`, which isn't in the partial-index predicate — currently a cheap index scan + flag recheck rather than a single probe. Not worth a second partial index.) +- **Verify (post-deploy on staging, NOT a CI merge gate):** `EXPLAIN ANALYZE` against a production-like distribution requires populated DB state. Run on staging after the migration applies, with 1M ops for a user holding 0-100 encrypted rows; run `ANALYZE operations` after the migration and record the expected plan. Also re-check the latest full-state `findFirst` path that filters `isPayloadEncrypted: false`; the existing full-state partial index does not include that predicate, so many encrypted full-state rows can still force scan-time rechecks. + +### 0b. Snapshot replay size-check cadence (Finding #2) + +- **File:** `packages/super-sync-server/src/sync/services/snapshot.service.ts:879-900` +- **Change:** replace `i % 1000 === 0 → JSON.stringify(state)` with delta-based accounting, with a carve-out for full-state ops: + - Before each call to `replayOpsToState` (which is called once per replay batch from `generateSnapshot`), compute `baseBytes = Buffer.byteLength(JSON.stringify(initialState), 'utf8')` once. Track `estimatedBytes = baseBytes` and `accumulatedDelta = 0`. + - During the loop, for each op add a cheap upper-bound delta = `Buffer.byteLength(JSON.stringify(payload || ''), 'utf8')` to `accumulatedDelta`. Overestimating is safe; deletes contribute 0. + - **Carve-out: when the op is `SYNC_IMPORT`, `BACKUP_IMPORT`, or `REPAIR`**, the op replaces state wholesale. The upper-bound counter would otherwise keep accumulating across the wipe and produce false "State too large" throws. After applying such an op, force a real measurement: `estimatedBytes = Buffer.byteLength(JSON.stringify(state), 'utf8')`, reset `accumulatedDelta = 0`. + - Trigger the real measurement (and reset `accumulatedDelta`) when `estimatedBytes + accumulatedDelta > 0.8 * MAX_REPLAY_STATE_SIZE_BYTES`. Throw if the real value still exceeds the cap. +- **Migration-split ops:** the inner loop at `:935-952` can fan one op into many; "delta per op = byteLength(payload)" still upper-bounds growth correctly (sum of fanned payloads ≥ state growth). No special handling needed. +- **Delete-heavy degradation:** deletes contribute 0 to the bound, but each forced real measurement re-reads the (now smaller) true size and resets `accumulatedDelta`, so the bound does not stay pinned — a delete-heavy stream after a large import triggers at most a handful of extra measurements, and only when the imported base alone is already near the cap. Effectively unreachable for normal data; signed-delta accounting is not worth the added complexity. +- **Why:** the pre-existing per-op-loop replay stringified the multi-MB state every 1000 ops, so a 100k-op replay did on the order of ~90 full stringifications inside the 60s RepeatableRead tx. After this change a 100k-op replay does ~1 per 10k-op replay batch (≈10), plus one per accepted full-state op; and because the delta bound is a proven over-estimate, the dominant case — a small/incremental replay whose bound stays under the cap — does **zero** (no regression vs the old loop, which also did zero below its 1000-op cadence). Net ≈5–10× on large replays, break-even on the common path. +- **Verify:** existing `snapshot.service.spec.ts` replay tests + add: 1500 small `CREATE`/`UPDATE` ops trigger zero full stringifications; a single `SYNC_IMPORT` triggers exactly one. + +### 0c. Snapshot blob measurement (Finding #5, snapshot half) + +- **File:** `storage-quota.service.ts:114-117` (the `findUnique({ select: { snapshotData: true } })`; `.length` is read at `:120`) +- **Change:** swap the `findUnique` for the same `octet_length(snapshot_data)` `$queryRaw` already used at `snapshot.service.ts:187-191` (`getCachedSnapshotBytes`). Don't pull a multi-MB `bytea` blob back to Node just to read `.length`. +- **Verify:** unit test: `calculateStorageUsage` agrees with `prepareSnapshotCache.bytes` (both are the gzip output length). +- **Caveat:** check how often `calculateStorageUsage` actually runs. `storage-quota.service.ts:84` describes it as "at most once per quota-cleanup event (rare per user)." If frequency is truly rare, this is a polish fix rather than a hot-path win. + +### 0d. Helm memory defaults (Finding #3, half) + +- **File:** `helm/supersync/values.yaml:178-184` +- **Current baseline:** `requests.memory: 128Mi`, `limits.memory: 256Mi`. +- **Change:** raise `limits.memory` to `1Gi`; raise `requests.memory` to `512Mi`. Add a comment naming the constants that drive the upper bound (`MAX_SNAPSHOT_DECOMPRESSED_BYTES`, `MAX_REPLAY_STATE_SIZE_BYTES`) and note the image-level `NODE_OPTIONS=--max-old-space-size=896`. +- **Why:** realistic single snapshot uploads can peak around 310-390MB (decompressed body + parsed JS object + serialized string + gzip output + baseline). Two concurrent snapshots can reach 600MB-1GB, so 512Mi remains fragile unless snapshot concurrency is separately pinned. +- **Also:** sweep `docs/sync-and-op-log/` for any documentation citing 256Mi. + +--- + +## Phase 1 — Upload batch processing (Finding #1, the big one) + +The plan-as-originally-drafted underspecified four real cases. The revised design below makes each explicit. + +### 1a. Refactor `processOperation` into a batch primitive + +**File:** `packages/super-sync-server/src/sync/sync.service.ts` — caller at `:459-467`, worker at `:634-790+`, and the upsert/counter/syncDevice tail at `:445-518`. + +**New shape, in order, inside the existing `tx`:** + +1. **Validate all ops in memory** (`validationService.validateOp`) — no DB. Produce a `decisions: Array<{ op, status: 'valid' | 'rejected', errorCode? }>`. + +2. **Dedupe by `op.id` within the batch.** If two ops in the same batch share an `id`, accept the first and reject subsequent ones as `DUPLICATE_OPERATION` — by id only, not content. This must happen before reserving sequence numbers; otherwise `lastSeq` advances for the duplicate and a server_seq gap is left when the row is silently skipped at insert time. + + **Deliberate divergence from the legacy per-op path (C4):** for an intra-batch `[A, A']` where the second op shares `A`'s id but has _different_ content, the legacy loop inserts `A` then catches `A'` at the DB and returns `INVALID_OP_ID`; the batch path returns `DUPLICATE_OPERATION`. Both are terminal rejections with no persisted row and no sequence gap, so all sync invariants hold — but the client treats them differently (`DUPLICATE_OPERATION` → marked synced silently; `INVALID_OP_ID` → hard rejection + error surfaced). This is an accepted behavior change while both paths coexist behind `SUPERSYNC_BATCH_UPLOAD`: the batch outcome (idempotent, non-noisy) is the preferred one. Pinned by the `sync.service.spec.ts` test "rejects an intra-batch same-id op as DUPLICATE_OPERATION even when its content differs". If the legacy path is retired, this divergence retires with it. + +3. **Prefetch existing op-id duplicates in one query:** + + ```ts + const existing = await tx.operation.findMany({ + where: { id: { in: validOpIds } }, + select: { + /* fields needed by isSameDuplicateOperation */ + }, + }); + ``` + + Build `Map`. For each `op` whose id is in the map, run `isSameDuplicateOperation` and audit as either `DUPLICATE_OPERATION` (idempotent retry) or `INVALID_OP_ID` (collision with a different op). + +4. **Prefetch latest-entity-op-per-(entityType, entityId) for every entity touched in the batch.** + **Multi-entity ops** carry `entityIds: string[]` (not just `entityId`) — see `sync.service.ts` `detectConflict` (lines 140-156). The prefetch set must be: + + ```ts + const entityKeys = new Set(); + for (const op of batch) { + const ids = op.entityIds ?? (op.entityId ? [op.entityId] : []); + for (const id of ids) entityKeys.add(`${op.entityType}::${id}`); + } + ``` + + Then one `DISTINCT ON` raw query (or one `findMany` with an in-app reduction) keyed on `(entityType, entityId)` ordered by `serverSeq DESC`, restricted to the touched set. Uses `@@index([userId, entityType, entityId, serverSeq])`. + +5. **Conflict detection in memory** against the prefetched map, **updating the map as each non-full-state op is accepted** so intra-batch conflicts (two ops on the same entity inside one batch) resolve in order — matches today's serial semantics. Full-state ops (`SYNC_IMPORT`/`BACKUP_IMPORT`/`REPAIR`) bypass conflict detection, as in `detectConflict` lines 129-136; they are not entity-scoped (`entityType: 'ALL'`, no `entityId`), so map invalidation is a no-op. + +6. **Reserve sequence numbers and ensure `user_sync_state` row exists in one round trip.** + The original draft proposed `tx.userSyncState.update(...)` for the increment, which throws `P2025` on a brand-new user (the row doesn't exist yet) and also leaves the existing `tx.userSyncState.upsert` at `:445-449` redundantly grabbing the same row lock. Replace **both** with one statement: + + ```sql + INSERT INTO user_sync_state (user_id, last_seq) + VALUES ($userId, $delta) + ON CONFLICT (user_id) DO UPDATE + SET last_seq = user_sync_state.last_seq + $delta + RETURNING last_seq + ``` + + `lastSeq` from the result is the new high-water mark; `accepted[i].serverSeq = lastSeq - accepted.length + i + 1`. Skip the statement entirely when `accepted.length === 0` (and skip the rest of the batch tail). + +7. **Bulk insert** with one `tx.operation.createMany({ data: rows })`. **Do NOT pass `skipDuplicates: true`.** Phase 1's correctness assumes the in-memory dedupe (step 2) and prefetch (step 3) have caught all duplicate ids; a row-level dup at insert time means our snapshot was stale and the right answer is to fail the batch with `P2002` on the operations primary key → outer 40001-style retry, not to silently drop a row whose sequence number we already reserved. + +8. **Run `_aggregatePriorVectorClock` once** if the batch contained any accepted full-state op (`isFullStateOpType(op.opType)`). This call (`sync.service.ts:600-628`) reads historical ops via `jsonb_each_text LATERAL`; it's not prefetchable. Use `beforeServerSeq = lastAcceptedFullStateOp.serverSeq` with `WHERE server_seq < beforeServerSeq`, so the aggregate includes prior history and accepted earlier-in-batch ops but excludes the full-state op itself and any later batch inserts. Persist `latestFullStateSeq` and `latestFullStateVectorClock` once. If a batch somehow contains two full-state ops, process them in batch-order — the last write wins, matching today's per-op-loop behavior. + +9. **Storage counter update** (`sync.service.ts:504-518`) — keep the `acceptedDeltaBytes` accumulation, summing `computeOpStorageBytes(op)` over accepted ops. Preserve the `isCleanSlate` SET-vs-INCREMENT branching exactly. + +10. **`syncDevice.upsert`** (`:476-495`) — per-batch already; stays as is. + +### 1b. FIX 1.5 — drop, with a recorded rationale + +Drop the per-op re-check at `sync.service.ts:786-794`. The safety it covered is delivered by the **shared `user_sync_state.lastSeq` row-write**, which forces concurrent batches to serialize: the second writer blocks on the row lock, then fails with `40001` (serialization failure) on commit. RR isolation alone does NOT provide this — PostgreSQL RR does not run full serializable snapshot isolation. The row-lock pattern is what makes the new design safe. + +**Already recorded as `ARCHITECTURE-DECISIONS.md` Decision #4** ("Batch Uploads Under RepeatableRead", line 121). Anyone proposing to remove the `lastSeq` increment from the hot path (e.g. sharded sequence assignment, distributed counters) must re-read that decision before doing so. + +### 1c. Tests + +- Extend `tests/sync.service.spec.ts`. Mock surfaces use the existing hand-rolled `vi.mock('../src/db', …)` pattern (not Prisma `$on('query')`, which isn't wired). Use `vi.spyOn(prisma.operation, 'findMany')` etc. to assert call counts. + - **25-op batch:** exactly 1 `findMany` for dup-id prefetch, 1 `findMany` (or `$queryRaw`) for entity prefetch, 1 `INSERT ... ON CONFLICT` for the sync-state row, 1 `operation.createMany`, 1 `syncDevice.upsert`, 1 `UPDATE users` counter, optionally 1 `_aggregatePriorVectorClock`. + - **Intra-batch duplicate `op.id`:** `[A, A]` — first accepted, second audited `DUPLICATE_OPERATION`, `lastSeq` advances by exactly 1, exactly one row inserted. + - **Intra-batch entity conflict:** `[op1, op2]` on the same entity — op1 wins, op2 rejected as concurrent. + - **Multi-entity op:** an op with `entityIds: [a, b, c]` correctly drives the prefetch and conflict-detection. + - **First-time user:** no `user_sync_state` row → upload succeeds; the `INSERT ... ON CONFLICT` creates the row with `last_seq = accepted.length`. + - **Full-state op in batch:** `_aggregatePriorVectorClock` runs exactly once at the end and sees only rows with `server_seq < fullState.serverSeq`. + - **Partial-acceptance batch:** 5 dups + 15 accepted → counters correct, audit log has 20 entries. + - **Concurrency:** two parallel batches on same user — outer retry on `P2034` / `40001` handles the loser. (This is unchanged in spirit but the failure mode shifts from "per-op re-check" to "shared row lock" — verify it still works.) + - **Sequence-gap invariant:** mixed-batch `[accept, reject, accept, reject, accept]` → persisted rows have contiguous `serverSeq = N, N+1, N+2`, `lastSeq` advances by exactly 3, no gaps anywhere. + - **No-double-terminal invariant:** every accepted op produces exactly zero rejection audits; every rejected op produces exactly zero persisted rows. Run across the full audit-event taxonomy (`OP_REJECTED`, `DUPLICATE_OPERATION`, `INVALID_OP_ID`, `CONFLICT_*`). + - **TIMESTAMP_CLAMPED additive case:** an op with `timestamp > now + maxClockDriftMs` produces one persisted row with the clamped timestamp **plus** one additional `TIMESTAMP_CLAMPED` audit event. The clamp is not a rejection — both outcomes coexist for the same op. +- **E2E:** `e2e/tests/sync/` (not `e2e/sync/`) — add one batch-of-50 upload test and assert latency drop vs. baseline. +- **Bench:** docker-compose Postgres, time 25-op and 100-op upload before/after. **Also measure concurrent-batch latency:** the shared row-lock means two simultaneous batches serialize hard — the per-batch latency under contention may be similar to today's per-op design. Total throughput should still win because each batch holds the lock for far less wall time. + +### 1d. Risk and rollout + +- Highest-blast-radius change in the plan. Land behind config flag `SUPERSYNC_BATCH_UPLOAD`, default `false` for one release, `true` the next. +- **Wire the flag** (shipped in `src/config.ts:172-179`): `batchUpload = (SUPERSYNC_BATCH_UPLOAD === 'true') && (SUPERSYNC_PAYLOAD_BYTES_BACKFILL_COMPLETE === 'true')`. The first condition without the second throws at startup with a message pointing operators at `npm run migrate-payload-bytes`. The DB-side complement is the startup self-check (see Cross-cutting): if `batchUpload === true` but `operations` still contains rows with `payload_bytes = 0`, the server refuses to boot. This closes the trust hole if an operator flips the env flag too early. +- **Route cap** (shipped in `sync.routes.ts:85, 601-604`): `MAX_OPS_PER_BATCH = SUPER_SYNC_MAX_OPS_PER_UPLOAD = 100` (from `packages/shared-schema/src/supersync-http-contract.ts:5`). Enforced before Zod parsing, returns HTTP 413 with `errorCode: 'PAYLOAD_TOO_LARGE'`. Same value also enforced inside the Zod schema as `.max(SUPER_SYNC_MAX_OPS_PER_UPLOAD)` so the OpenAPI contract stays in sync. +- **Invariant:** every op in the batch produces exactly one terminal-status audit (rejection) OR exactly one persisted row, plus optionally one additive `TIMESTAMP_CLAMPED` audit — never both terminal outcomes, never neither, never gapped sequence numbers. + +--- + +## Phase 2 — Quota byte accounting (Finding #5, ops half) + +### 2a. Schema change + +- New migration: add `payload_bytes BIGINT NOT NULL DEFAULT 0` to `operations`. +- Backfill — see §2b for why this can't be pure SQL. + +### 2b. Backfill must use `computeOpStorageBytes`, not `pg_column_size` + +The original draft proposed `UPDATE ... SET payload_bytes = pg_column_size(payload) + pg_column_size(vector_clock)`. **This is wrong.** + +- `pg_column_size(payload)` returns the TOAST-compressed on-disk size — typically much smaller than the uncompressed value for large JSONB. +- The write path uses `computeOpStorageBytes(op)` (in `sync.const.ts`), which returns `Buffer.byteLength(JSON.stringify(payload ?? null), 'utf8') + Buffer.byteLength(JSON.stringify(vectorClock ?? {}), 'utf8')` — i.e. the uncompressed UTF-8 length. + +These are different numbers by design. The file's own comment at `storage-quota.service.ts:88-103` already calls out this mismatch as the historical bug. Backfilling with `pg_column_size` seeds drift instead of fixing it: the SUM-query and the increment-counter will disagree on every reconcile after deployment. + +**Correct backfill:** stream `operations` rows per user in small batches from a one-time Node script, compute `computeOpStorageBytes(row)` per row, and batch the writes: + +```sql +UPDATE operations +SET payload_bytes = v.bytes::bigint +FROM (VALUES ...) AS v(id, bytes) +WHERE operations.id = v.id +``` + +This preserves correctness while avoiding one network round trip per row. Run it as a separate `migrate-payload-bytes.ts` (mirroring the existing `migrate-passkey-credentials.ts` pattern) **outside** the Prisma migration framework — Prisma migrations run synchronously at startup, and a synchronous backfill on a 100M-row `operations` table would block the server for hours. + +There is no clean SQL equivalent of `Buffer.byteLength(JSON.stringify(payload))` over JSONB. `octet_length(payload::text)` is close but reads/detoasts every row, which is the very disk-I/O DoS the file's comment warns about. + +### 2c. Write path + +- **All insert sites** populate `payload_bytes` per op using `computeOpStorageBytes` so the on-row value matches the increment-counter value the hot path is already adding. Both code paths are wired today: batch path at `sync.service.ts:1115`, legacy per-op path at `:1393`. This is the consistency `calculateStorageUsage` needs and the reason an old per-op insert deployed under `SUPERSYNC_BATCH_UPLOAD=false` does not seed drift while batch is rolled out. + +### 2d. Read path + +- Replace `storage-quota.service.ts:109-112` with the `CASE WHEN` form already shipped at `:97-120`: + ```sql + SELECT COALESCE( + SUM( + CASE + WHEN payload_bytes > 0 THEN payload_bytes + ELSE octet_length(payload::text)::bigint + + octet_length(vector_clock::text)::bigint + END + ), + 0 + ) AS total + FROM operations + WHERE user_id = $1 + ``` +- The `ELSE` branch (option (b) in the design analysis) is the only candidate on the same UTF-8 scale as `computeOpStorageBytes`. Detoasting cost is bounded — only un-backfilled rows hit it, and the set drains monotonically to zero. +- Drop `pg_column_size` entirely. No detoasting on backfilled rows, no I/O DoS. +- Snapshot side is handled in 0c. + +### 2e. Tests + +- Unit: after a 100-op upload, `calculateStorageUsage` and the cached `storage_used_bytes` counter agree to the byte. +- Test: reconcile-after-upload is idempotent (drift = 0). +- Test: a synthetic row with `payload_bytes = 0` (pre-backfill) still produces a conservative fallback SUM. Hard-cut rollout on backfill completion: `SUPERSYNC_BATCH_UPLOAD=true` must require an operator-set completion flag after `npm run migrate-payload-bytes` finishes. + +--- + +## Phase 3 — Snapshot serialization off the hot path (Finding #3, remainder) + +Pick after profiling Phase 0d in production: + +### 3a. Streaming serialize + gzip (conditional, NOT a default win) + +- Replace `prepareSnapshotCache` (`snapshot.service.ts:175-184`) with a streaming pipeline: a streaming JSON stringifier feeding `zlib.createGzip()`, collecting chunks into a final `Buffer.concat(...)`. +- **Pursue only if Phase 0d profiling shows OOM near `MAX_SNAPSHOT_DECOMPRESSED_BYTES` AND event-loop blocking — not memory pressure alone.** The 3-5× wall-clock regression vs native `JSON.stringify` makes this a memory-vs-latency trade. +- Net peak memory: saves the intermediate serialized string buffer (~100MB on large states), but the parsed JS object remains because it already exists. Expect roughly 30-40% peak reduction, not 50%+. +- **Verification gate:** `snapshotData` is only used for byte-count accounting and gunzip-then-parse round-tripping (verified — no hash or content comparison anywhere in `src/`). So byte-for-byte stability is NOT required; round-trip correctness is. Add a property-based test: random state → stream-stringify-gzip → gunzip-parse → deep-equal original. + +### 3b. Worker-thread offload (only if replay moves too) + +- Do not move only `JSON.stringify(state)` into a `worker_threads` worker. Structured-cloning a large state into the worker temporarily doubles heap and can cost more event-loop time than the stringify it avoids. Worker offload only makes sense if replay and stringify both move to the worker. + +If 0d alone is sufficient in prod (no OOMs, no event-loop-blocking signals), defer this phase indefinitely. + +--- + +## Phase 4 — Auth token-verification cache (Finding #6) + +### 4a. Cache shape + +- **New module:** `packages/super-sync-server/src/auth-cache.ts`, wired into `verifyToken` at `auth.ts:114-158`. +- LRU + TTL: `Map`. TTL 30s, max 10k entries. +- On verify: + 1. JWT decode (as today). + 2. Cache hit && not expired && `payload.tokenVersion === cached.tokenVersion` && `cached.isVerified` → return valid. + 3. Else hit DB, update cache, return. + +### 4b. Invalidation — full surface + +All `tokenVersion: { increment: 1 }` write sites plus account deletion must invalidate. Already wired in code with `// AUTH_CACHE_INVALIDATION:` comments adjacent to each write — kept here for future-PR awareness: + +- `auth.ts:77`/`:80` (`revokeAllTokens`) +- `auth.ts:96`/`:102` (`replaceToken`) +- `auth.ts:108` (post-write second invalidate after the new token version is read back) +- `passkey.ts:592`/`:616` (passkey recovery — pre- and post-write) +- `passkey.ts:278` (unverified-user delete in registration flow) +- `api.ts:210`/`:213`/`:215` (`prisma.user.delete` in account deletion — both pre- and post-delete invalidate) + +`isVerified` currently has no flip-to-zero path (`passkey.ts:277` deletes unverified users rather than flipping the flag). The assumption is already documented at `auth-cache.ts:80` — if a future code path adds verification revocation, the cache will serve stale "valid" for up to TTL. + +### 4c. Multi-instance concerns + +- `helm/supersync/values.yaml:193` caps `maxReplicas: 1`, so in-process LRU is safe. Comment explicitly so a future multi-instance rollout doesn't accidentally introduce 30s revocation lag. + +### 4d. Tests + +- Unit: revoke-and-replace invalidates cache; expired tokens still hit DB; tokenVersion mismatch falls through; user deletion invalidates cache; passkey recovery invalidates cache. +- Bench: 1000 sequential `verifyToken` calls — expect ~10× p50 latency drop on warm cache. + +--- + +## Cross-cutting + +- **Merge order:** 0a, 0b, 0c, 0d can land in any order, in parallel with Phase 1 design. Phase 2 depends on Phase 1 (same code paths). Phase 3 is conditional. Phase 4 is independent. +- **Telemetry first:** before Phase 1 lands, add structured logging of `(opsInBatch, txDurationMs, dbRoundtrips)` to `uploadOps` so we can quantify the win. Existing audit log handles per-op decisions; add a single batch-summary line. +- **Backfill-flag DB self-check:** the env-only `SUPERSYNC_PAYLOAD_BYTES_BACKFILL_COMPLETE=true` flag is operator-trusted. To prevent a too-early flip, the server runs a cheap `EXISTS (SELECT 1 FROM operations WHERE payload_bytes = 0 LIMIT 1)` probe at startup whenever `batchUpload === true` and refuses to boot if any unbackfilled rows remain. +- **Reconcile guard during backfill window:** `calculateStorageUsage` returns a `hasUnbackfilledRows` flag (computed from a `BOOL_OR(payload_bytes = 0)` over the same single scan). `updateStorageUsage` skips the `users.storage_used_bytes` write when the flag is true, so an approximate SUM-with-`octet_length`-fallback never replaces the exact incrementally-maintained counter mid-backfill. The forced-reconcile marker is preserved across the skip so the next call (after backfill completes) reconciles correctly. +- **ADR:** see `ARCHITECTURE-DECISIONS.md` Decision #4 ("Batch Uploads Under RepeatableRead"); already merged. +- **Docs:** update `docs/sync-and-op-log/operation-log-architecture-diagrams.md` §upload-path if it diagrams the per-op loop. +- **Prisma migrate dev:** document the shadow-DB workaround for migrations containing `CREATE INDEX CONCURRENTLY`; `migrate deploy` can run the production workaround, but `migrate dev` wraps migration SQL in a transaction where `CONCURRENTLY` is forbidden. +- **Server seq precision:** any raw `last_seq` read that crosses the JavaScript boundary must hard-fail if it is not a safe integer instead of blindly calling `Number(...)`. +- **Test patterns:** the codebase uses `vi.mock('../src/db', …)` with hand-rolled mocks, not Prisma `$on('query')` interceptors. Test-count assertions must use `vi.spyOn` on the mock surfaces. +- **Out of scope:** WebSocket fan-out, cleanup-job optimization, passkey paths. None flagged in the audit. + +--- + +## Estimated impact (rough order of magnitude) + +| Phase | Hot path affected | Expected win | Risk | +| ----- | ----------------------------- | ---------------------------------------------------------------------------------------------------------------------- | -------------------------- | +| 0a | Snapshot fast-path validation | Eliminates seq-range scan on **encrypted-op count**; full-state `findFirst` rechecks unchanged | very low | +| 0b | Snapshot replay | ~5–10× fewer full stringifications on large replays; zero (no regression) on the common small/incremental replay | low | +| 0c | Quota reconcile | Skips blob load (tens of MB) | very low | +| 0d | All routes (memory headroom) | Stops OOMs near snapshot cap | low (ops change) | +| 1 | Upload (every client batch) | ~5× fewer DB round trips on 25-op batch; shorter `user_sync_state` row lock; throughput-positive even under contention | medium-high | +| 2 | Quota reconcile (slow path) | Removes `pg_column_size` table scan; consistency between SUM and counter | medium (schema + backfill) | +| 3 | Snapshot upload memory | ~30-40% lower peak heap if streaming wins in profiling | medium | +| 4 | Auth on every request | ~10× p50 latency drop on warm cache | low | diff --git a/packages/super-sync-server/Dockerfile b/packages/super-sync-server/Dockerfile index 62614090f7..4271c74e2f 100644 --- a/packages/super-sync-server/Dockerfile +++ b/packages/super-sync-server/Dockerfile @@ -74,8 +74,10 @@ ENV NODE_ENV=production ENV PORT=1900 ENV DATA_DIR=/app/data ENV RUN_MIGRATIONS_ON_STARTUP=false +# Keep V8 below the recommended 1Gi container limit so GC starts before cgroup OOM. +ENV NODE_OPTIONS=--max-old-space-size=896 HEALTHCHECK --interval=30s --timeout=5s --start-period=30s --retries=3 \ CMD wget --no-verbose --tries=1 --spider http://localhost:${PORT}/health || exit 1 -CMD ["sh", "-c", "if [ \"${RUN_MIGRATIONS_ON_STARTUP:-false}\" = \"true\" ]; then npx prisma migrate deploy || exit 1; fi; exec node dist/src/index.js"] +CMD ["sh", "-c", "if [ \"${RUN_MIGRATIONS_ON_STARTUP:-false}\" = \"true\" ]; then sh scripts/migrate-deploy.sh || exit 1; fi; exec node dist/src/index.js"] diff --git a/packages/super-sync-server/README.md b/packages/super-sync-server/README.md index 1a92f5c1c5..e5418f1e2b 100644 --- a/packages/super-sync-server/README.md +++ b/packages/super-sync-server/README.md @@ -76,12 +76,44 @@ handles both cases: it resolves the failed row when needed, applies the concurrent index statements one at a time outside Prisma migrate, marks the migration applied, and retries `migrate deploy`. +For local `prisma migrate dev` shadow databases, apply migrations containing +`CREATE INDEX CONCURRENTLY` through `prisma db execute` outside the transaction +and then mark the migration applied, mirroring the production deploy workaround. + If `DATABASE_URL` points to an external PostgreSQL server, set `POSTGRES_SERVICE=` to the empty value. `deploy.sh` then starts only the app/proxy services with compose dependencies disabled so the bundled Postgres container is not required. Prisma migrations still run against the configured `DATABASE_URL`. +### Payload byte backfill and batch uploads + +The `payload_bytes` column must be fully backfilled before enabling batched +uploads in production. During a partial backfill, quota reconciles use a slower +fallback for old operation rows with `payload_bytes = 0`. + +Run the backfill to completion: + +```bash +npm run migrate-payload-bytes +``` + +In a source checkout before `npm run build`, use: + +```bash +npm run migrate-payload-bytes:dev +``` + +Only then set both rollout flags: + +```bash +SUPERSYNC_BATCH_UPLOAD=true +SUPERSYNC_PAYLOAD_BYTES_BACKFILL_COMPLETE=true +``` + +The server refuses to start with `SUPERSYNC_BATCH_UPLOAD=true` unless the +completion flag is also set. + ### Manual Setup (Development) ```bash diff --git a/packages/super-sync-server/docker-compose.yml b/packages/super-sync-server/docker-compose.yml index 26ed12e61a..2d72697ea2 100644 --- a/packages/super-sync-server/docker-compose.yml +++ b/packages/super-sync-server/docker-compose.yml @@ -38,6 +38,8 @@ services: - WEBAUTHN_ORIGIN=${WEBAUTHN_ORIGIN:-http://localhost:1900} - OLD_OPS_CLEANUP_DELETE_BATCH_SIZE=${OLD_OPS_CLEANUP_DELETE_BATCH_SIZE:-5000} - OLD_OPS_CLEANUP_MAX_DELETED_PER_RUN=${OLD_OPS_CLEANUP_MAX_DELETED_PER_RUN:-25000} + - SUPERSYNC_BATCH_UPLOAD=${SUPERSYNC_BATCH_UPLOAD:-false} + - SUPERSYNC_PAYLOAD_BYTES_BACKFILL_COMPLETE=${SUPERSYNC_PAYLOAD_BYTES_BACKFILL_COMPLETE:-false} ports: - '127.0.0.1:1900:1900' volumes: @@ -63,7 +65,7 @@ services: - no-new-privileges:true cap_drop: - ALL - mem_limit: 512m + mem_limit: 1g cpus: '1.0' networks: - internal diff --git a/packages/super-sync-server/helm/supersync/templates/configmap.yaml b/packages/super-sync-server/helm/supersync/templates/configmap.yaml index 7f77569bf9..995271866f 100644 --- a/packages/super-sync-server/helm/supersync/templates/configmap.yaml +++ b/packages/super-sync-server/helm/supersync/templates/configmap.yaml @@ -13,6 +13,8 @@ data: CORS_ORIGINS: {{ .Values.supersync.cors.origins | quote }} OLD_OPS_CLEANUP_DELETE_BATCH_SIZE: {{ .Values.supersync.cleanup.oldOpsDeleteBatchSize | int | quote }} OLD_OPS_CLEANUP_MAX_DELETED_PER_RUN: {{ .Values.supersync.cleanup.oldOpsMaxDeletedPerRun | int | quote }} + SUPERSYNC_BATCH_UPLOAD: {{ .Values.supersync.batchUpload.enabled | quote }} + SUPERSYNC_PAYLOAD_BYTES_BACKFILL_COMPLETE: {{ .Values.supersync.batchUpload.payloadBytesBackfillComplete | quote }} {{- if .Values.supersync.webauthn.rpName }} WEBAUTHN_RP_NAME: {{ .Values.supersync.webauthn.rpName | quote }} {{- end }} diff --git a/packages/super-sync-server/helm/supersync/templates/deployment.yaml b/packages/super-sync-server/helm/supersync/templates/deployment.yaml index e6d79923d4..d4d01259f8 100644 --- a/packages/super-sync-server/helm/supersync/templates/deployment.yaml +++ b/packages/super-sync-server/helm/supersync/templates/deployment.yaml @@ -43,7 +43,7 @@ spec: - name: migrate-db image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}" imagePullPolicy: {{ .Values.image.pullPolicy }} - command: ['sh', '-c', 'npx prisma@5.22.0 migrate deploy'] + command: ['sh', '-c', 'sh scripts/migrate-deploy.sh'] env: {{- if .Values.postgresql.enabled }} - name: POSTGRES_USER diff --git a/packages/super-sync-server/helm/supersync/values.yaml b/packages/super-sync-server/helm/supersync/values.yaml index 8c0d7a1fda..2f6ddd8ced 100644 --- a/packages/super-sync-server/helm/supersync/values.yaml +++ b/packages/super-sync-server/helm/supersync/values.yaml @@ -43,6 +43,11 @@ supersync: oldOpsDeleteBatchSize: 5000 # -- Max old operations deleted across all users in one cleanup pass. oldOpsMaxDeletedPerRun: 25000 + batchUpload: + # -- Enable the batched upload implementation. Requires payloadBytesBackfillComplete=true. + enabled: false + # -- Set only after running `npm run migrate-payload-bytes` to completion. + payloadBytesBackfillComplete: false webauthn: # -- WebAuthn relying party display name rpName: "Super Productivity Sync" @@ -176,12 +181,15 @@ service: port: 80 # -- Resource requests and limits for the SuperSync container +# Snapshot upload/replay can hold decompressed request data plus replay state +# near MAX_SNAPSHOT_DECOMPRESSED_BYTES and MAX_REPLAY_STATE_SIZE_BYTES; the +# image sets NODE_OPTIONS=--max-old-space-size=896 for a 1Gi memory limit. resources: requests: cpu: 100m - memory: 128Mi + memory: 512Mi limits: - memory: 256Mi + memory: 1Gi # -- Horizontal Pod Autoscaler configuration autoscaling: diff --git a/packages/super-sync-server/package.json b/packages/super-sync-server/package.json index 0cbb862b71..20ebebb7fa 100644 --- a/packages/super-sync-server/package.json +++ b/packages/super-sync-server/package.json @@ -27,7 +27,9 @@ "monitor:all": "node dist/scripts/run-all-monitoring.js", "monitor:all:dev": "tsx scripts/run-all-monitoring.ts", "monitor:all:quick": "node dist/scripts/run-all-monitoring.js --quick", - "monitor:all:save": "node dist/scripts/run-all-monitoring.js --save" + "monitor:all:save": "node dist/scripts/run-all-monitoring.js --save", + "migrate-payload-bytes": "node dist/scripts/migrate-payload-bytes.js", + "migrate-payload-bytes:dev": "ts-node scripts/migrate-payload-bytes.ts" }, "dependencies": { "@fastify/cors": "^11.1.0", diff --git a/packages/super-sync-server/prisma/migrations/20260514000000_add_encrypted_ops_partial_index/migration.sql b/packages/super-sync-server/prisma/migrations/20260514000000_add_encrypted_ops_partial_index/migration.sql new file mode 100644 index 0000000000..5e18a560fe --- /dev/null +++ b/packages/super-sync-server/prisma/migrations/20260514000000_add_encrypted_ops_partial_index/migration.sql @@ -0,0 +1,4 @@ +DROP INDEX CONCURRENTLY IF EXISTS "operations_user_id_server_seq_encrypted_idx"; +CREATE INDEX CONCURRENTLY "operations_user_id_server_seq_encrypted_idx" + ON "operations"("user_id", "server_seq") + WHERE "is_payload_encrypted" = true; diff --git a/packages/super-sync-server/prisma/migrations/20260514000001_add_operation_payload_bytes/migration.sql b/packages/super-sync-server/prisma/migrations/20260514000001_add_operation_payload_bytes/migration.sql new file mode 100644 index 0000000000..356763f626 --- /dev/null +++ b/packages/super-sync-server/prisma/migrations/20260514000001_add_operation_payload_bytes/migration.sql @@ -0,0 +1,2 @@ +ALTER TABLE "operations" + ADD COLUMN "payload_bytes" BIGINT NOT NULL DEFAULT 0; diff --git a/packages/super-sync-server/prisma/migrations/20260514000002_add_payload_bytes_unbackfilled_index/migration.sql b/packages/super-sync-server/prisma/migrations/20260514000002_add_payload_bytes_unbackfilled_index/migration.sql new file mode 100644 index 0000000000..4bca498801 --- /dev/null +++ b/packages/super-sync-server/prisma/migrations/20260514000002_add_payload_bytes_unbackfilled_index/migration.sql @@ -0,0 +1,14 @@ +-- Partial index over the rows the payload_bytes backfill still has to touch. +-- It only contains rows WHERE payload_bytes = 0, so as migrate-payload-bytes.ts +-- sets every row > 0 the index physically drains to empty. Post-backfill the +-- boot-time `EXISTS (SELECT 1 FROM operations WHERE payload_bytes = 0 LIMIT 1)` +-- self-check and the `BOOL_OR(payload_bytes = 0)` quota probe resolve via an +-- empty index instead of a full sequential scan that has to visit every heap +-- tuple to prove absence. Keyed (user_id, id) so the backfill's per-user +-- keyset paging (WHERE user_id = $ AND payload_bytes = 0 AND id > $lastId +-- ORDER BY id) is a true index seek rather than a re-scan from the start of +-- each user's range on every batch. Concurrent build to avoid locking writes. +DROP INDEX CONCURRENTLY IF EXISTS "operations_payload_bytes_unbackfilled_idx"; +CREATE INDEX CONCURRENTLY "operations_payload_bytes_unbackfilled_idx" + ON "operations"("user_id", "id") + WHERE "payload_bytes" = 0; diff --git a/packages/super-sync-server/prisma/schema.prisma b/packages/super-sync-server/prisma/schema.prisma index d6d626ee12..d2bbc7e361 100644 --- a/packages/super-sync-server/prisma/schema.prisma +++ b/packages/super-sync-server/prisma/schema.prisma @@ -70,6 +70,7 @@ model Operation { entityType String @map("entity_type") entityId String? @map("entity_id") payload Json + payloadBytes BigInt @default(0) @map("payload_bytes") vectorClock Json @map("vector_clock") schemaVersion Int @map("schema_version") clientTimestamp BigInt @map("client_timestamp") diff --git a/packages/super-sync-server/scripts/deploy.sh b/packages/super-sync-server/scripts/deploy.sh index ecb9bd631e..87b10e5942 100755 --- a/packages/super-sync-server/scripts/deploy.sh +++ b/packages/super-sync-server/scripts/deploy.sh @@ -149,6 +149,8 @@ echo "" MIGRATION_TIMEOUT="${MIGRATION_TIMEOUT:-900}" MIGRATOR_RUN="docker compose $COMPOSE_FILES run --rm --no-deps --interactive=false -T supersync" FULL_STATE_INDEX_MIGRATION="20260512000000_add_full_state_sequence_index_drop_redundant_indexes" +ENCRYPTED_OPS_INDEX_MIGRATION="20260514000000_add_encrypted_ops_partial_index" +PAYLOAD_BYTES_INDEX_MIGRATION="20260514000002_add_payload_bytes_unbackfilled_index" echo "==> Verifying database connectivity from the supersync image..." set +e timeout "$POSTGRES_WAIT_TIMEOUT" \ @@ -196,6 +198,32 @@ is_full_state_index_transaction_block_failure() { grep -q 'cannot run inside a transaction block' "$MIGRATE_LOG" } +is_recoverable_encrypted_ops_index_migration_failure() { + [ -n "$MIGRATE_LOG" ] && + grep -q 'P3009' "$MIGRATE_LOG" && + grep -q "$ENCRYPTED_OPS_INDEX_MIGRATION" "$MIGRATE_LOG" +} + +is_encrypted_ops_index_transaction_block_failure() { + [ -n "$MIGRATE_LOG" ] && + grep -q 'P3018' "$MIGRATE_LOG" && + grep -q "$ENCRYPTED_OPS_INDEX_MIGRATION" "$MIGRATE_LOG" && + grep -q 'cannot run inside a transaction block' "$MIGRATE_LOG" +} + +is_recoverable_payload_bytes_index_migration_failure() { + [ -n "$MIGRATE_LOG" ] && + grep -q 'P3009' "$MIGRATE_LOG" && + grep -q "$PAYLOAD_BYTES_INDEX_MIGRATION" "$MIGRATE_LOG" +} + +is_payload_bytes_index_transaction_block_failure() { + [ -n "$MIGRATE_LOG" ] && + grep -q 'P3018' "$MIGRATE_LOG" && + grep -q "$PAYLOAD_BYTES_INDEX_MIGRATION" "$MIGRATE_LOG" && + grep -q 'cannot run inside a transaction block' "$MIGRATE_LOG" +} + run_concurrent_index_sql() { local sql="$1" local execute_status @@ -234,6 +262,24 @@ apply_full_state_index_migration_outside_prisma() { run_concurrent_index_sql 'DROP INDEX CONCURRENTLY IF EXISTS "operations_user_id_server_seq_idx";' } +apply_encrypted_ops_index_migration_outside_prisma() { + echo "" + echo "==> Applying $ENCRYPTED_OPS_INDEX_MIGRATION outside Prisma migrate..." + echo " Prisma cannot run CREATE INDEX CONCURRENTLY in a transaction block." + + run_concurrent_index_sql 'DROP INDEX CONCURRENTLY IF EXISTS "operations_user_id_server_seq_encrypted_idx";' + run_concurrent_index_sql 'CREATE INDEX CONCURRENTLY "operations_user_id_server_seq_encrypted_idx" ON "operations"("user_id", "server_seq") WHERE "is_payload_encrypted" = true;' +} + +apply_payload_bytes_index_migration_outside_prisma() { + echo "" + echo "==> Applying $PAYLOAD_BYTES_INDEX_MIGRATION outside Prisma migrate..." + echo " Prisma cannot run CREATE INDEX CONCURRENTLY in a transaction block." + + run_concurrent_index_sql 'DROP INDEX CONCURRENTLY IF EXISTS "operations_payload_bytes_unbackfilled_idx";' + run_concurrent_index_sql 'CREATE INDEX CONCURRENTLY "operations_payload_bytes_unbackfilled_idx" ON "operations"("user_id", "id") WHERE "payload_bytes" = 0;' +} + resolve_failed_full_state_index_migration() { local resolve_status @@ -258,6 +304,30 @@ resolve_failed_full_state_index_migration() { fi } +resolve_failed_encrypted_ops_index_migration() { + local resolve_status + + echo "" + echo "==> Resolving failed index-only migration $ENCRYPTED_OPS_INDEX_MIGRATION..." + echo " Marking it rolled back so Prisma can retry the migration SQL." + set +e + timeout "$POSTGRES_WAIT_TIMEOUT" \ + $MIGRATOR_RUN npx prisma migrate resolve --rolled-back "$ENCRYPTED_OPS_INDEX_MIGRATION" + resolve_status=$? + set -e + + if [ "$resolve_status" -eq 124 ]; then + echo "" + echo "ERROR: prisma migrate resolve timed out after ${POSTGRES_WAIT_TIMEOUT}s." + exit 1 + fi + if [ "$resolve_status" -ne 0 ]; then + echo "" + echo "ERROR: prisma migrate resolve failed (exit $resolve_status)." + exit "$resolve_status" + fi +} + resolve_applied_full_state_index_migration() { local resolve_status @@ -281,6 +351,76 @@ resolve_applied_full_state_index_migration() { fi } +resolve_applied_encrypted_ops_index_migration() { + local resolve_status + + echo "" + echo "==> Marking $ENCRYPTED_OPS_INDEX_MIGRATION as applied after out-of-band SQL..." + set +e + timeout "$POSTGRES_WAIT_TIMEOUT" \ + $MIGRATOR_RUN npx prisma migrate resolve --applied "$ENCRYPTED_OPS_INDEX_MIGRATION" + resolve_status=$? + set -e + + if [ "$resolve_status" -eq 124 ]; then + echo "" + echo "ERROR: prisma migrate resolve --applied timed out after ${POSTGRES_WAIT_TIMEOUT}s." + exit 1 + fi + if [ "$resolve_status" -ne 0 ]; then + echo "" + echo "ERROR: prisma migrate resolve --applied failed (exit $resolve_status)." + exit "$resolve_status" + fi +} + +resolve_failed_payload_bytes_index_migration() { + local resolve_status + + echo "" + echo "==> Resolving failed index-only migration $PAYLOAD_BYTES_INDEX_MIGRATION..." + echo " Marking it rolled back so Prisma can retry the migration SQL." + set +e + timeout "$POSTGRES_WAIT_TIMEOUT" \ + $MIGRATOR_RUN npx prisma migrate resolve --rolled-back "$PAYLOAD_BYTES_INDEX_MIGRATION" + resolve_status=$? + set -e + + if [ "$resolve_status" -eq 124 ]; then + echo "" + echo "ERROR: prisma migrate resolve timed out after ${POSTGRES_WAIT_TIMEOUT}s." + exit 1 + fi + if [ "$resolve_status" -ne 0 ]; then + echo "" + echo "ERROR: prisma migrate resolve failed (exit $resolve_status)." + exit "$resolve_status" + fi +} + +resolve_applied_payload_bytes_index_migration() { + local resolve_status + + echo "" + echo "==> Marking $PAYLOAD_BYTES_INDEX_MIGRATION as applied after out-of-band SQL..." + set +e + timeout "$POSTGRES_WAIT_TIMEOUT" \ + $MIGRATOR_RUN npx prisma migrate resolve --applied "$PAYLOAD_BYTES_INDEX_MIGRATION" + resolve_status=$? + set -e + + if [ "$resolve_status" -eq 124 ]; then + echo "" + echo "ERROR: prisma migrate resolve --applied timed out after ${POSTGRES_WAIT_TIMEOUT}s." + exit 1 + fi + if [ "$resolve_status" -ne 0 ]; then + echo "" + echo "ERROR: prisma migrate resolve --applied failed (exit $resolve_status)." + exit "$resolve_status" + fi +} + run_migrate_deploy if [ "$MIGRATE_STATUS" -ne 0 ] && [ "$MIGRATE_STATUS" -ne 124 ] && is_recoverable_full_state_index_migration_failure; then @@ -297,6 +437,40 @@ if [ "$MIGRATE_STATUS" -ne 0 ] && [ "$MIGRATE_STATUS" -ne 124 ] && echo "==> Retrying database migrations after applying $FULL_STATE_INDEX_MIGRATION..." run_migrate_deploy fi +if [ "$MIGRATE_STATUS" -ne 0 ] && [ "$MIGRATE_STATUS" -ne 124 ] && + is_recoverable_encrypted_ops_index_migration_failure; then + resolve_failed_encrypted_ops_index_migration + apply_encrypted_ops_index_migration_outside_prisma + resolve_applied_encrypted_ops_index_migration + echo "" + echo "==> Retrying database migrations after applying $ENCRYPTED_OPS_INDEX_MIGRATION..." + run_migrate_deploy +fi +if [ "$MIGRATE_STATUS" -ne 0 ] && [ "$MIGRATE_STATUS" -ne 124 ] && + is_encrypted_ops_index_transaction_block_failure; then + apply_encrypted_ops_index_migration_outside_prisma + resolve_applied_encrypted_ops_index_migration + echo "" + echo "==> Retrying database migrations after applying $ENCRYPTED_OPS_INDEX_MIGRATION..." + run_migrate_deploy +fi +if [ "$MIGRATE_STATUS" -ne 0 ] && [ "$MIGRATE_STATUS" -ne 124 ] && + is_recoverable_payload_bytes_index_migration_failure; then + resolve_failed_payload_bytes_index_migration + apply_payload_bytes_index_migration_outside_prisma + resolve_applied_payload_bytes_index_migration + echo "" + echo "==> Retrying database migrations after applying $PAYLOAD_BYTES_INDEX_MIGRATION..." + run_migrate_deploy +fi +if [ "$MIGRATE_STATUS" -ne 0 ] && [ "$MIGRATE_STATUS" -ne 124 ] && + is_payload_bytes_index_transaction_block_failure; then + apply_payload_bytes_index_migration_outside_prisma + resolve_applied_payload_bytes_index_migration + echo "" + echo "==> Retrying database migrations after applying $PAYLOAD_BYTES_INDEX_MIGRATION..." + run_migrate_deploy +fi if [ "$MIGRATE_STATUS" -eq 124 ]; then echo "" diff --git a/packages/super-sync-server/scripts/migrate-deploy.sh b/packages/super-sync-server/scripts/migrate-deploy.sh new file mode 100644 index 0000000000..3372ab2c44 --- /dev/null +++ b/packages/super-sync-server/scripts/migrate-deploy.sh @@ -0,0 +1,116 @@ +#!/bin/sh +set -eu + +FULL_STATE_INDEX_MIGRATION="20260512000000_add_full_state_sequence_index_drop_redundant_indexes" +ENCRYPTED_OPS_INDEX_MIGRATION="20260514000000_add_encrypted_ops_partial_index" +PAYLOAD_BYTES_INDEX_MIGRATION="20260514000002_add_payload_bytes_unbackfilled_index" + +MIGRATE_LOG="" +MIGRATE_STATUS=0 + +run_migrate_deploy() { + MIGRATE_LOG="$(mktemp "${TMPDIR:-/tmp}/supersync-migrate.XXXXXX")" + set +e + npx prisma migrate deploy >"$MIGRATE_LOG" 2>&1 + MIGRATE_STATUS=$? + set -e + cat "$MIGRATE_LOG" +} + +log_mentions() { + grep -q "$1" "$MIGRATE_LOG" +} + +is_recoverable_migration_failure() { + [ -n "$MIGRATE_LOG" ] && + grep -Eq 'P3009|P3018' "$MIGRATE_LOG" && + log_mentions "$1" +} + +run_sql() { + printf '%s\n' "$1" | npx prisma db execute --schema prisma/schema.prisma --stdin +} + +resolve_rolled_back() { + npx prisma migrate resolve --rolled-back "$1" +} + +resolve_applied() { + npx prisma migrate resolve --applied "$1" +} + +apply_full_state_index_migration() { + echo "Applying $FULL_STATE_INDEX_MIGRATION outside Prisma migrate..." + run_sql 'DROP INDEX CONCURRENTLY IF EXISTS "operations_user_id_full_state_server_seq_idx";' + run_sql 'CREATE INDEX CONCURRENTLY "operations_user_id_full_state_server_seq_idx" ON "operations"("user_id", "server_seq") WHERE "op_type" IN ('\''SYNC_IMPORT'\'', '\''BACKUP_IMPORT'\'', '\''REPAIR'\'');' + run_sql 'DROP INDEX CONCURRENTLY IF EXISTS "operations_user_id_op_type_idx";' + run_sql 'DROP INDEX CONCURRENTLY IF EXISTS "operations_user_id_entity_type_entity_id_idx";' + run_sql 'DROP INDEX CONCURRENTLY IF EXISTS "operations_user_id_server_seq_idx";' +} + +apply_encrypted_ops_index_migration() { + echo "Applying $ENCRYPTED_OPS_INDEX_MIGRATION outside Prisma migrate..." + run_sql 'DROP INDEX CONCURRENTLY IF EXISTS "operations_user_id_server_seq_encrypted_idx";' + run_sql 'CREATE INDEX CONCURRENTLY "operations_user_id_server_seq_encrypted_idx" ON "operations"("user_id", "server_seq") WHERE "is_payload_encrypted" = true;' +} + +apply_payload_bytes_index_migration() { + echo "Applying $PAYLOAD_BYTES_INDEX_MIGRATION outside Prisma migrate..." + run_sql 'DROP INDEX CONCURRENTLY IF EXISTS "operations_payload_bytes_unbackfilled_idx";' + run_sql 'CREATE INDEX CONCURRENTLY "operations_payload_bytes_unbackfilled_idx" ON "operations"("user_id", "id") WHERE "payload_bytes" = 0;' +} + +recover_index_migration() { + migration="$1" + + set +e + resolve_rolled_back "$migration" + resolve_status=$? + set -e + if [ "$resolve_status" -ne 0 ]; then + echo "Migration $migration was not in a failed state; continuing with out-of-band SQL." + fi + + case "$migration" in + "$FULL_STATE_INDEX_MIGRATION") + apply_full_state_index_migration + ;; + "$ENCRYPTED_OPS_INDEX_MIGRATION") + apply_encrypted_ops_index_migration + ;; + "$PAYLOAD_BYTES_INDEX_MIGRATION") + apply_payload_bytes_index_migration + ;; + *) + echo "Unknown migration: $migration" + exit 1 + ;; + esac + + resolve_applied "$migration" +} + +run_migrate_deploy + +if [ "$MIGRATE_STATUS" -ne 0 ] && + is_recoverable_migration_failure "$FULL_STATE_INDEX_MIGRATION"; then + recover_index_migration "$FULL_STATE_INDEX_MIGRATION" + run_migrate_deploy +fi + +if [ "$MIGRATE_STATUS" -ne 0 ] && + is_recoverable_migration_failure "$ENCRYPTED_OPS_INDEX_MIGRATION"; then + recover_index_migration "$ENCRYPTED_OPS_INDEX_MIGRATION" + run_migrate_deploy +fi + +if [ "$MIGRATE_STATUS" -ne 0 ] && + is_recoverable_migration_failure "$PAYLOAD_BYTES_INDEX_MIGRATION"; then + recover_index_migration "$PAYLOAD_BYTES_INDEX_MIGRATION" + run_migrate_deploy +fi + +if [ "$MIGRATE_STATUS" -ne 0 ]; then + echo "prisma migrate deploy failed (exit $MIGRATE_STATUS)." + exit "$MIGRATE_STATUS" +fi diff --git a/packages/super-sync-server/scripts/migrate-payload-bytes.ts b/packages/super-sync-server/scripts/migrate-payload-bytes.ts new file mode 100644 index 0000000000..148d1d9bf6 --- /dev/null +++ b/packages/super-sync-server/scripts/migrate-payload-bytes.ts @@ -0,0 +1,157 @@ +import { Prisma, PrismaClient } from '@prisma/client'; +import { computeOpStorageBytes } from '../src/sync/sync.const'; + +// One backfill iteration is 2 round trips (findMany + UPDATE ... FROM (VALUES ...)) +// per BATCH_SIZE rows. The UPDATE is N primary-key lookups joined to a small VALUES +// list, so it only takes short per-row locks and never a table lock; VALUES lists of +// a few thousand short tuples are cheap. Keeping these tiny made a 100M-row backfill +// take tens of hours, which in turn keeps the slow octet_length() quota fallback and +// the boot-time backfill self-check on the un-indexed scan path far longer than +// necessary. Size for throughput; the MAX cap still bounds the VALUES string so a +// fat-fingered override cannot OOM the Node process. +const DEFAULT_BATCH_SIZE = 500; +const MAX_BATCH_SIZE = 1000; +const USER_PAGE_SIZE = 1000; + +const prisma = new PrismaClient(); + +const parseBatchSize = (): number => { + const raw = process.env.PAYLOAD_BYTES_MIGRATION_BATCH_SIZE; + if (!raw) return DEFAULT_BATCH_SIZE; + + const parsed = Number.parseInt(raw, 10); + if (!Number.isInteger(parsed) || parsed <= 0) { + throw new Error( + `Invalid PAYLOAD_BYTES_MIGRATION_BATCH_SIZE: ${raw}. Must be a positive integer.`, + ); + } + return Math.min(parsed, MAX_BATCH_SIZE); +}; + +const fetchUserIdsWithUnbackfilledRows = async ( + afterUserId: number | undefined, +): Promise => { + const rows = await prisma.$queryRaw>` + SELECT DISTINCT user_id + FROM operations + WHERE payload_bytes = 0 + ${afterUserId === undefined ? Prisma.empty : Prisma.sql`AND user_id > ${afterUserId}`} + ORDER BY user_id ASC + LIMIT ${USER_PAGE_SIZE} + `; + + return rows.map((row) => row.user_id); +}; + +const updatePayloadBytesBatch = async ( + updates: Array<{ id: string; bytes: number }>, +): Promise => { + if (updates.length === 0) return; + + const values = Prisma.join( + updates.map( + (update) => Prisma.sql`(${update.id}::text, ${BigInt(update.bytes)}::bigint)`, + ), + ); + + await prisma.$executeRaw` + UPDATE operations + SET payload_bytes = v.bytes + FROM (VALUES ${values}) AS v(id, bytes) + WHERE operations.id = v.id + `; +}; + +const backfillUser = async (userId: number, batchSize: number): Promise => { + let updated = 0; + let lastId: string | undefined; + + for (;;) { + const rows = await prisma.operation.findMany({ + where: { + userId, + payloadBytes: BigInt(0), + ...(lastId ? { id: { gt: lastId } } : {}), + }, + orderBy: { id: 'asc' }, + take: batchSize, + select: { + id: true, + payload: true, + vectorClock: true, + }, + }); + + if (rows.length === 0) break; + + await updatePayloadBytesBatch( + rows.map((row) => ({ + id: row.id, + bytes: computeOpStorageBytes({ + payload: row.payload, + vectorClock: row.vectorClock, + }).bytes, + })), + ); + + updated += rows.length; + lastId = rows[rows.length - 1].id; + console.log( + `Updated ${updated} operation payload byte counters for user ${userId}...`, + ); + } + + return updated; +}; + +const reconcileUserStorageUsage = async (userId: number): Promise => { + await prisma.$executeRaw` + UPDATE users + SET storage_used_bytes = usage.total_bytes + FROM ( + SELECT + ${userId}::integer AS user_id, + ( + SELECT COALESCE(SUM(payload_bytes), 0) + FROM operations + WHERE user_id = ${userId} + ) + + COALESCE(( + SELECT octet_length(snapshot_data)::bigint + FROM user_sync_state + WHERE user_id = ${userId} + ), 0) AS total_bytes + ) AS usage + WHERE users.id = usage.user_id + `; +}; + +const run = async (): Promise => { + const batchSize = parseBatchSize(); + let updated = 0; + let lastUserId: number | undefined; + + for (;;) { + const userIds = await fetchUserIdsWithUnbackfilledRows(lastUserId); + if (userIds.length === 0) break; + + for (const userId of userIds) { + updated += await backfillUser(userId, batchSize); + await reconcileUserStorageUsage(userId); + lastUserId = userId; + } + console.log(`Updated ${updated} operation payload byte counters total...`); + } + + console.log(`Payload byte migration complete. Updated ${updated} operations.`); +}; + +run() + .catch((err: unknown) => { + const message = err instanceof Error ? err.message : String(err); + console.error(`Payload byte migration failed: ${message}`); + process.exitCode = 1; + }) + .finally(async () => { + await prisma.$disconnect(); + }); diff --git a/packages/super-sync-server/src/api.ts b/packages/super-sync-server/src/api.ts index 70a771b757..f51749be63 100644 --- a/packages/super-sync-server/src/api.ts +++ b/packages/super-sync-server/src/api.ts @@ -23,6 +23,7 @@ import { import { authenticate, getAuthUser } from './middleware'; import { Logger } from './logger'; import { prisma } from './db'; +import { authCache } from './auth-cache'; // Zod Schemas const VerifyEmailSchema = z.object({ @@ -205,8 +206,13 @@ export const apiRoutes = async (fastify: FastifyInstance): Promise => { Logger.info(`[user:${userId}] DELETE ACCOUNT requested`); + // AUTH_CACHE_INVALIDATION: account deletion must not leave a ghost-token window. + authCache.invalidate(userId); + // Cascade delete handles: operations, syncState, devices (via Prisma schema) await prisma.user.delete({ where: { id: userId } }); + // AUTH_CACHE_INVALIDATION: account deletion must not leave a ghost-token window. + authCache.invalidate(userId); Logger.audit({ event: 'USER_ACCOUNT_DELETED', userId }); diff --git a/packages/super-sync-server/src/auth-cache.ts b/packages/super-sync-server/src/auth-cache.ts new file mode 100644 index 0000000000..d89f218d19 --- /dev/null +++ b/packages/super-sync-server/src/auth-cache.ts @@ -0,0 +1,101 @@ +interface AuthCacheEntry { + tokenVersion: number; + isVerified: boolean; + expiresAt: number; +} + +const AUTH_CACHE_TTL_MS = 30 * 1000; +const AUTH_CACHE_MAX_ENTRIES = 10_000; + +class AuthCache { + private entries = new Map(); + private invalidationVersions = new Map(); + + get(userId: number): AuthCacheEntry | null { + const entry = this.entries.get(userId); + if (!entry) return null; + + if (entry.expiresAt <= Date.now()) { + this.entries.delete(userId); + return null; + } + + this.entries.delete(userId); + this.entries.set(userId, entry); + return entry; + } + + getInvalidationVersion(userId: number): number { + return this.invalidationVersions.get(userId) ?? 0; + } + + setIfCurrent( + userId: number, + tokenVersion: number, + isVerified: boolean, + expectedInvalidationVersion: number, + ): boolean { + if (this.getInvalidationVersion(userId) !== expectedInvalidationVersion) { + return false; + } + + this.entries.delete(userId); + this.entries.set(userId, { + tokenVersion, + isVerified, + expiresAt: Date.now() + AUTH_CACHE_TTL_MS, + }); + + while (this.entries.size > AUTH_CACHE_MAX_ENTRIES) { + const oldestKey = this.entries.keys().next().value; + if (oldestKey === undefined) break; + this.entries.delete(oldestKey); + } + return true; + } + + set(userId: number, tokenVersion: number, isVerified: boolean): void { + this.setIfCurrent( + userId, + tokenVersion, + isVerified, + this.getInvalidationVersion(userId), + ); + } + + invalidate(userId: number): void { + const nextVersion = this.getInvalidationVersion(userId) + 1; + // Re-insert at the tail so the just-invalidated user is the MOST recently + // used. invalidationVersions must persist after entries.delete() so a + // verifyToken whose DB read raced this invalidate fails its setIfCurrent CAS + // and does not cache stale-valid data. Bounding the map is required (it + // otherwise grows one entry per lifetime-invalidated user, unbounded on a + // long-lived single replica). Evicting the OLDEST invalidations is safe: an + // invalidation only needs to survive until the racing in-flight read's + // setIfCurrent (bounded by one DB round trip). A freshly-invalidated user + // sits at the MRU tail, so it can only be evicted after every other of the + // 10k tracked invalidations is newer than it — far beyond any read window. + this.invalidationVersions.delete(userId); + this.invalidationVersions.set(userId, nextVersion); + this.entries.delete(userId); + + while (this.invalidationVersions.size > AUTH_CACHE_MAX_ENTRIES) { + const oldestKey = this.invalidationVersions.keys().next().value; + if (oldestKey === undefined) break; + this.invalidationVersions.delete(oldestKey); + } + } + + clear(): void { + this.entries.clear(); + this.invalidationVersions.clear(); + } +} + +// Safe while Helm caps SuperSync at one replica. A future multi-instance rollout +// needs shared invalidation or a lower revocation-lag design. +// +// `isVerified` currently has no verified -> unverified transition; unverified +// passkey registrations are deleted on failure. If verification revocation is +// added later, invalidate this cache beside that write. +export const authCache = new AuthCache(); diff --git a/packages/super-sync-server/src/auth.ts b/packages/super-sync-server/src/auth.ts index 9923c0a48a..a1f1507fb7 100644 --- a/packages/super-sync-server/src/auth.ts +++ b/packages/super-sync-server/src/auth.ts @@ -6,6 +6,7 @@ import { randomBytes } from 'crypto'; import { sendLoginMagicLinkEmail, sendVerificationEmail } from './email'; import { loadConfigFromEnv } from './config'; import { Prisma } from '@prisma/client'; +import { authCache } from './auth-cache'; // Auth constants const MIN_JWT_SECRET_LENGTH = 32; @@ -63,6 +64,11 @@ export const verifyEmail = async (token: string): Promise => { }, }); + // AUTH_CACHE_INVALIDATION: drop any negative (isVerified:false) entry so the + // now-verified user isn't denied for up to the cache TTL, and so the cache + // stays correct if a verified -> unverified path is ever added. + authCache.invalidate(user.id); + Logger.info(`User verified (ID: ${user.id})`); return true; }; @@ -72,10 +78,14 @@ export const verifyEmail = async (token: string): Promise => { * Call this when the user explicitly logs out all devices. */ export const revokeAllTokens = async (userId: number): Promise => { + // AUTH_CACHE_INVALIDATION: keep adjacent to tokenVersion writes. + authCache.invalidate(userId); await prisma.user.update({ where: { id: userId }, data: { tokenVersion: { increment: 1 } }, }); + // AUTH_CACHE_INVALIDATION: keep adjacent to tokenVersion writes. + authCache.invalidate(userId); Logger.info(`All tokens revoked for user ${userId}`); }; @@ -87,6 +97,8 @@ export const replaceToken = async ( userId: number, email: string, ): Promise<{ token: string; user: { id: number; email: string } }> => { + // AUTH_CACHE_INVALIDATION: keep adjacent to tokenVersion writes. + authCache.invalidate(userId); // Use transaction to ensure atomicity of version increment and read const newTokenVersion = await prisma.$transaction(async (tx) => { // Increment token version to invalidate all existing tokens @@ -97,6 +109,8 @@ export const replaceToken = async ( }); return user.tokenVersion; }); + // AUTH_CACHE_INVALIDATION: keep adjacent to tokenVersion writes. + authCache.invalidate(userId); const token = jwt.sign({ userId, email, tokenVersion: newTokenVersion }, JWT_SECRET, { expiresIn: JWT_EXPIRY, @@ -124,6 +138,13 @@ export const verifyToken = async (token: string): Promise { await tx.passkey.deleteMany({ where: { userId: user.id } }); @@ -612,6 +620,8 @@ export const completePasskeyRecovery = async ( }, }); }); + // AUTH_CACHE_INVALIDATION: keep adjacent to tokenVersion writes. + authCache.invalidate(user.id); Logger.info(`Passkey recovery completed (ID: ${user.id})`); diff --git a/packages/super-sync-server/src/server.ts b/packages/super-sync-server/src/server.ts index eb9284001d..6bf6ea406d 100644 --- a/packages/super-sync-server/src/server.ts +++ b/packages/super-sync-server/src/server.ts @@ -11,7 +11,13 @@ import { prisma, disconnectDb } from './db'; import websocket from '@fastify/websocket'; import { apiRoutes } from './api'; import { pageRoutes } from './pages'; -import { syncRoutes, startCleanupJobs, stopCleanupJobs } from './sync'; +import { + syncRoutes, + startCleanupJobs, + stopCleanupJobs, + initSyncService, + getSyncService, +} from './sync'; import { wsRoutes } from './sync/websocket.routes'; import { getWsConnectionService, @@ -109,6 +115,7 @@ export const createServer = ( stop: () => Promise; } => { const fullConfig = loadConfigFromEnv(config); + initSyncService({ batchUpload: fullConfig.batchUpload }); // Ensure data directory exists if (!fs.existsSync(fullConfig.dataDir)) { @@ -220,6 +227,22 @@ export const createServer = ( options: { maxPayload: 1024 }, }); + // Backfill self-check: paired with the env-flag enforcement in + // loadConfigFromEnv. The env flag (SUPERSYNC_PAYLOAD_BYTES_BACKFILL_COMPLETE) + // is operator-set; if it is flipped to true before the migrate-payload-bytes + // script finishes, batch-upload deltas are still correct but the SUM-based + // reconcile in calculateStorageUsage would mix exact bytes with the + // CASE-WHEN fallback for legacy rows. One indexed probe at startup closes + // the trust hole: refuse to boot if any row still has payload_bytes = 0. + if (fullConfig.batchUpload) { + try { + await getSyncService().assertPayloadBytesBackfillComplete(); + } catch (err) { + Logger.error('Startup self-check failed', err); + throw err; + } + } + // Health Check - verifies database connectivity // Exempt from rate limiting (Kubernetes probes hit this every 5-15s) fastifyServer.get('/health', { config: { rateLimit: false } }, async (_, reply) => { diff --git a/packages/super-sync-server/src/sync/services/snapshot.service.ts b/packages/super-sync-server/src/sync/services/snapshot.service.ts index 3609993d84..e25a793648 100644 --- a/packages/super-sync-server/src/sync/services/snapshot.service.ts +++ b/packages/super-sync-server/src/sync/services/snapshot.service.ts @@ -46,10 +46,7 @@ const MAX_SNAPSHOT_DECOMPRESSED_BYTES = 100 * 1024 * 1024; */ const MAX_REPLAY_STATE_SIZE_BYTES = 100 * 1024 * 1024; -/** - * How often to check state size during replay (every N operations). - */ -const REPLAY_SIZE_CHECK_INTERVAL = 1000; +const REPLAY_SIZE_CHECK_THRESHOLD_BYTES = MAX_REPLAY_STATE_SIZE_BYTES * 0.8; /** * Reject these as property keys when applying user-supplied ids to the @@ -60,6 +57,50 @@ const REPLAY_SIZE_CHECK_INTERVAL = 1000; const isUnsafeEntityKey = (key: string): boolean => key === '__proto__' || key === 'constructor' || key === 'prototype'; +const isReplayFullStateOpType = (opType: string): boolean => + opType === 'SYNC_IMPORT' || opType === 'BACKUP_IMPORT' || opType === 'REPAIR'; + +/** + * Generous upper bound on the JSON structural overhead of inserting one new + * `state[entityType][entityId] = {...}` slot: the id key's quotes + colon + + * comma (~4 bytes) plus, when the entity-type map is new, a `"type":{}` wrapper + * (~6 bytes). Padded to 32 so the running delta accounting can NEVER + * under-count — the replay size guard's correctness (and the decision to skip + * the final exact measurement when the upper bound proves safety) depends on + * this being an over-estimate. + */ +const REPLAY_ENTITY_KEY_JSON_OVERHEAD_BYTES = 32; + +const getReplayPayloadDeltaBytes = (opType: string, payload: unknown): number => { + if (opType === 'DEL') return 0; + return Buffer.byteLength(JSON.stringify(payload ?? ''), 'utf8'); +}; + +const getReplayEntityKeyDeltaBytes = ( + entityType: string, + entityId: string | null, +): number => + Buffer.byteLength(entityType, 'utf8') + + (entityId ? Buffer.byteLength(entityId, 'utf8') : 0) + + REPLAY_ENTITY_KEY_JSON_OVERHEAD_BYTES; + +/** + * Throws if the serialized state exceeds the replay cap. The return value is + * load-bearing, NOT incidental: callers assign it back to `estimatedBytes` to + * reset the running delta-accounting baseline. Do not "simplify" this to + * `void`. + */ +const assertReplayStateSize = (state: Record): number => { + const stateBytes = Buffer.byteLength(JSON.stringify(state), 'utf8'); + if (stateBytes > MAX_REPLAY_STATE_SIZE_BYTES) { + throw new Error( + `State too large during replay: ${Math.round(stateBytes / 1024 / 1024)}MB ` + + `(max: ${Math.round(MAX_REPLAY_STATE_SIZE_BYTES / 1024 / 1024)}MB)`, + ); + } + return stateBytes; +}; + const encryptedOpsNotSupportedMessage = (encryptedOpCount: number): string => `ENCRYPTED_OPS_NOT_SUPPORTED: Cannot generate snapshot - ${encryptedOpCount} operations have encrypted payloads. ` + `Server-side restore is not available when E2E encryption is enabled. ` + @@ -875,6 +916,9 @@ export class SnapshotService { initialState: Record = {}, ): Record { const state = { ...(initialState as Record>) }; + let estimatedBytes = + Object.keys(state).length === 0 ? 2 : assertReplayStateSize(state); + let accumulatedDeltaBytes = 0; for (let i = 0; i < ops.length; i++) { const row = ops[i]; @@ -885,24 +929,14 @@ export class SnapshotService { throw new EncryptedOpsNotSupportedError(1); } - // Periodically check state size to prevent memory exhaustion. - // Measure in UTF-8 bytes (Buffer.byteLength) so the limit matches the - // constant's documented unit — JSON.stringify(state).length counts - // UTF-16 code units, which under-counts up to 4x for non-ASCII content. - if (i > 0 && i % REPLAY_SIZE_CHECK_INTERVAL === 0) { - const estimatedSize = Buffer.byteLength(JSON.stringify(state), 'utf8'); - if (estimatedSize > MAX_REPLAY_STATE_SIZE_BYTES) { - throw new Error( - `State too large during replay: ${Math.round(estimatedSize / 1024 / 1024)}MB ` + - `(max: ${Math.round(MAX_REPLAY_STATE_SIZE_BYTES / 1024 / 1024)}MB)`, - ); - } - } - let opType = row.opType as Operation['opType']; let entityType = row.entityType; let entityId = row.entityId; let payload = row.payload; + accumulatedDeltaBytes += + getReplayPayloadDeltaBytes(opType, payload) + + getReplayEntityKeyDeltaBytes(entityType, entityId); + let forceStateSizeMeasurement = false; const opSchemaVersion = row.schemaVersion ?? 1; @@ -966,11 +1000,7 @@ export class SnapshotService { // otherwise stale entity types from a prior state survive a "reset" // and `_resolveExpectedFirstSeq`'s leading-gap acceptance becomes // incorrect (the gap is only safe if the full-state op truly resets). - if ( - processOpType === 'SYNC_IMPORT' || - processOpType === 'BACKUP_IMPORT' || - processOpType === 'REPAIR' - ) { + if (isReplayFullStateOpType(processOpType)) { const fullState = processPayload && typeof processPayload === 'object' && @@ -999,6 +1029,7 @@ export class SnapshotService { if (isUnsafeEntityKey(key)) continue; state[key] = fullStateRecord[key] as Record; } + forceStateSizeMeasurement = true; continue; } @@ -1068,6 +1099,25 @@ export class SnapshotService { break; } } + + if ( + forceStateSizeMeasurement || + estimatedBytes + accumulatedDeltaBytes > REPLAY_SIZE_CHECK_THRESHOLD_BYTES + ) { + estimatedBytes = assertReplayStateSize(state); + accumulatedDeltaBytes = 0; + } + } + // `estimatedBytes + accumulatedDeltaBytes` is a proven over-estimate of the + // true serialized size (payload byteLength upper-bounds merged growth, DEL + // contributes 0, entity-key overhead is padded). When it is within the cap + // the true size is too, so the exact final measurement is provably + // redundant — skipping it keeps the common small/incremental replay at zero + // expensive full stringifications. The exact check still runs (and throws) + // whenever the bound does not prove safety; any state that truly exceeds + // the cap pushes the over-estimate past it and trips this. + if (estimatedBytes + accumulatedDeltaBytes > MAX_REPLAY_STATE_SIZE_BYTES) { + assertReplayStateSize(state); } return state; } diff --git a/packages/super-sync-server/src/sync/services/storage-quota.service.ts b/packages/super-sync-server/src/sync/services/storage-quota.service.ts index 10a1b9bedb..417d31819e 100644 --- a/packages/super-sync-server/src/sync/services/storage-quota.service.ts +++ b/packages/super-sync-server/src/sync/services/storage-quota.service.ts @@ -9,6 +9,7 @@ */ import { AsyncLocalStorage } from 'node:async_hooks'; import { prisma } from '../../db'; +import { Logger } from '../../logger'; /** * Default storage quota per user in bytes (100MB). @@ -30,9 +31,8 @@ export class StorageQuotaService { /** * Per-user in-flight reconcile promises. When multiple concurrent requests * for the same user hit the quota cache-miss path, only the first triggers - * the slow SUM(pg_column_size) scan; the rest await the same promise. Cap - * the number of duplicate full-table scans under a retry storm. Sequential - * calls are unaffected (entry is deleted in `finally` before resolve). + * the exact SUM(payload_bytes) reconcile; the rest await the same promise. + * Sequential calls are unaffected (entry is deleted in `finally` before resolve). */ private inflightReconciles: Map> = new Map(); @@ -74,56 +74,83 @@ export class StorageQuotaService { } /** - * Calculate actual storage usage for a user by summing on-disk payload sizes. + * Calculate actual storage usage for a user by summing the write-time byte + * counters on operation rows plus the cached snapshot blob length. * - * SLOW PATH — DO NOT CALL PER REQUEST. SUM(pg_column_size(payload)) forces - * PostgreSQL to detoast every payload for the user (TOAST table reads), - * which on active users takes minutes and saturates disk I/O. Reserved for: + * SLOW PATH — DO NOT CALL PER REQUEST. Even without detoasting JSONB payloads, + * this still scans one user's operation rows and is reserved for: * 1. Quota-cache reconciliation, run at most once per quota-cleanup event * (rare per user) — see SyncService.freeStorageForUpload. * 2. Offline / admin reconciliation scripts. * Hot-path tracking uses incrementStorageUsage / decrementStorageUsage with * deltas computed locally on the Node side. * - * KNOWN BYTE-COUNTING MISMATCH (tracked for follow-up): - * `pg_column_size` returns the TOAST-compressed on-disk size, while the - * hot-path delta (sync.routes.ts `computeOpsStorageBytes`) uses - * `Buffer.byteLength(JSON.stringify(...))` — the uncompressed UTF-8 length. - * For large compressible JSONB payloads (~2KB+) the compressed size can be - * substantially smaller, so a reconcile can shrink a counter that was - * incremented with the larger uncompressed numbers, making the quota - * artificially generous after each reconcile. + * Rows with payload_bytes=0 are pre-backfill rows. They must not be counted + * as zero bytes: that would let a reconcile lower the cached counter below + * actual usage. The CASE WHEN fallback only touches unbackfilled rows, so + * once the one-time backfill completes this remains a cheap SUM. * - * A no-DoS fix is to add a `payload_bytes` column populated at insert time - * with the uncompressed length and SUM that column here. That is a schema - * migration and is outside the scope of this service-only file; switching - * to `octet_length(payload::text)` in-place would re-detoast every row and - * resurrect the original disk-I/O DoS that pg_column_size also caused (see - * sync.const.ts `APPROX_BYTES_PER_OP`). + * `hasUnbackfilledRows` is computed in the same single scan via BOOL_OR. + * Callers (notably `updateStorageUsage`) treat the SUM as approximate when + * this flag is true, because the fallback's UTF-8 length differs by single + * bytes from the JS-side `computeOpStorageBytes` value used by the hot-path + * counter. Skipping the `users.storage_used_bytes` write while unbackfilled + * rows exist preserves the exact incremental counter. */ async calculateStorageUsage(userId: number): Promise<{ operationsBytes: number; snapshotBytes: number; totalBytes: number; + hasUnbackfilledRows: boolean; }> { - const opsResult = await prisma.$queryRaw<[{ total: bigint | null }]>` - SELECT COALESCE(SUM(pg_column_size(payload) + pg_column_size(vector_clock)), 0) as total - FROM operations WHERE user_id = ${userId} + const usageResult = await prisma.$queryRaw< + [ + { + operations_bytes: bigint | null; + snapshot_bytes: number | bigint | null; + has_unbackfilled?: boolean | null; + }, + ] + >` + SELECT + ops.operations_bytes, + ops.has_unbackfilled, + COALESCE( + ( + SELECT octet_length(snapshot_data) + FROM user_sync_state + WHERE user_id = ${userId} + ), + 0 + )::bigint AS snapshot_bytes + FROM ( + SELECT + COALESCE( + SUM( + CASE + WHEN payload_bytes > 0 THEN payload_bytes + ELSE octet_length(payload::text)::bigint + + octet_length(vector_clock::text)::bigint + END + ), + 0 + )::bigint AS operations_bytes, + COALESCE(BOOL_OR(payload_bytes = 0), false) AS has_unbackfilled + FROM operations + WHERE user_id = ${userId} + ) AS ops `; - const snapshotResult = await prisma.userSyncState.findUnique({ - where: { userId }, - select: { snapshotData: true }, - }); - - const operationsBytes = Number(opsResult[0]?.total ?? 0); - const snapshotBytes = snapshotResult?.snapshotData?.length ?? 0; + const operationsBytes = Number(usageResult[0]?.operations_bytes ?? 0); + const snapshotBytes = Number(usageResult[0]?.snapshot_bytes ?? 0); const totalBytes = operationsBytes + snapshotBytes; + const hasUnbackfilledRows = Boolean(usageResult[0]?.has_unbackfilled ?? false); return { operationsBytes, snapshotBytes, totalBytes, + hasUnbackfilledRows, }; } @@ -225,7 +252,19 @@ export class StorageQuotaService { // that is itself queued behind our own lock → deadlock. const inLock = this.storageUsageLockContext.getStore()?.has(userId); if (inLock) { - const { totalBytes } = await this.calculateStorageUsage(userId); + const { totalBytes, hasUnbackfilledRows } = + await this.calculateStorageUsage(userId); + if (hasUnbackfilledRows) { + // Pre-backfill rows make the SUM approximate (CASE-WHEN fallback uses + // postgres-side text length, not JS-side computeOpStorageBytes). Writing + // an approximate value here would replace the exact incrementally + // maintained counter — drift in either direction. Leave the forced + // reconcile marker so a post-backfill call self-heals. + Logger.warn( + `[user:${userId}] Skipping storage usage reconcile: payload_bytes backfill incomplete for this user.`, + ); + return; + } await prisma.user.update({ where: { id: userId }, data: { storageUsedBytes: BigInt(totalBytes) }, @@ -238,7 +277,14 @@ export class StorageQuotaService { if (existing) return existing; const promise = this.runWithStorageUsageLock(userId, async () => { - const { totalBytes } = await this.calculateStorageUsage(userId); + const { totalBytes, hasUnbackfilledRows } = + await this.calculateStorageUsage(userId); + if (hasUnbackfilledRows) { + Logger.warn( + `[user:${userId}] Skipping storage usage reconcile: payload_bytes backfill incomplete for this user.`, + ); + return; + } await prisma.user.update({ where: { id: userId }, data: { storageUsedBytes: BigInt(totalBytes) }, @@ -291,6 +337,37 @@ export class StorageQuotaService { this.forcedReconciles.delete(userId); } + /** + * Backfill self-check. When SUPERSYNC_BATCH_UPLOAD=true the operator is + * trusted to have set SUPERSYNC_PAYLOAD_BYTES_BACKFILL_COMPLETE=true only + * after `npm run migrate-payload-bytes` finished. If the flag was flipped + * too early, batch uploads still write `payload_bytes` correctly but the + * SUM-based reconcile in `calculateStorageUsage` would mix exact bytes with + * the CASE-WHEN fallback for legacy rows — small drift, but unnecessary + * given the env-flag's whole purpose. + * + * One indexed-probe at startup closes the trust hole. The query relies on a + * full table scan-with-LIMIT-1; for a fully backfilled table that is one + * row visit on the first encountered row (cheap), and for a partially + * backfilled table it returns immediately. Worst case (zero rows in + * `operations`, e.g. fresh deployment) is also one round-trip. + */ + async assertPayloadBytesBackfillComplete(): Promise { + const result = await prisma.$queryRaw<[{ exists: boolean }]>` + SELECT EXISTS ( + SELECT 1 FROM operations WHERE payload_bytes = 0 LIMIT 1 + ) AS "exists" + `; + if (result[0]?.exists) { + throw new Error( + 'SUPERSYNC_BATCH_UPLOAD is enabled but the operations table still ' + + 'contains rows with payload_bytes = 0. Run ' + + '`npm run migrate-payload-bytes` to complete the backfill before ' + + 'setting SUPERSYNC_PAYLOAD_BYTES_BACKFILL_COMPLETE=true.', + ); + } + } + /** * Get storage quota and usage for a user. * Used by status endpoint. diff --git a/packages/super-sync-server/src/sync/sync.const.ts b/packages/super-sync-server/src/sync/sync.const.ts index 814e3abdf4..08c93afc8d 100644 --- a/packages/super-sync-server/src/sync/sync.const.ts +++ b/packages/super-sync-server/src/sync/sync.const.ts @@ -15,9 +15,8 @@ export { * payloads can be up to 20MB, so 1024 undercounts by ~20000x and the cached * counter ends up permanently low if reconcile fails. `deleteOldestRestorePointAndOps` * measures the exact `pg_column_size(payload)` for those 1-2 rows BEFORE - * deleting; the per-row scan there is bounded to the restore-point fan-out - * (very small) and does not reintroduce the SUM(pg_column_size) DoS that - * scanning every delta op caused. + * deleting; the persisted payload_bytes value avoids reintroducing the + * SUM(pg_column_size) DoS that scanning every delta op caused. */ export const APPROX_BYTES_PER_OP = 1024; @@ -26,8 +25,8 @@ export const APPROX_BYTES_PER_OP = 1024; * vector clock will occupy on disk. Used by both the route layer (for quota * gating and post-commit counter deltas) and the service layer (for the atomic * counter write inside the upload transaction). Keeping a single - * implementation guarantees the gate and the increment cannot disagree about - * what "size" means. + * implementation guarantees the gate, the operation payload_bytes column, and + * the increment cannot disagree about what "size" means. * * Robust against malformed payloads: if JSON.stringify throws (e.g. BigInt, * circular ref), the op is charged APPROX_BYTES_PER_OP so the counter cannot diff --git a/packages/super-sync-server/src/sync/sync.routes.ts b/packages/super-sync-server/src/sync/sync.routes.ts index 58381ffe8f..27084f913e 100644 --- a/packages/super-sync-server/src/sync/sync.routes.ts +++ b/packages/super-sync-server/src/sync/sync.routes.ts @@ -9,6 +9,7 @@ import { Transform, type TransformCallback } from 'node:stream'; import { z } from 'zod'; import { uuidv7 } from 'uuidv7'; import { + SUPER_SYNC_MAX_OPS_PER_UPLOAD, SuperSyncDownloadOpsQuerySchema, SuperSyncUploadOpsRequestSchema, SuperSyncUploadSnapshotRequestSchema, @@ -79,6 +80,9 @@ const MAX_COMPRESSED_SIZE_SNAPSHOT = 30 * 1024 * 1024; // 30MB for /snapshot (ma // for the JSON-vs-compressed ratio. const MAX_DECOMPRESSED_SIZE_OPS = 30 * 1024 * 1024; // 30MB for /ops const MAX_DECOMPRESSED_SIZE_SNAPSHOT = 60 * 1024 * 1024; // 60MB for /snapshot +// Route-level guard that mirrors the shared contract but runs before Zod's +// per-op validation and before SyncService can build large prefetch queries. +const MAX_OPS_PER_BATCH = SUPER_SYNC_MAX_OPS_PER_UPLOAD; // Fastify's route bodyLimit runs before our parser can decode Android's // Content-Transfer-Encoding: base64 wrapper. Use the raw base64 envelope limit @@ -215,6 +219,27 @@ const computeJsonStorageBytes = (value: unknown, fallback: unknown): number => { } }; +const getRawOpsCount = (body: unknown): number | null => { + if (typeof body !== 'object' || body === null) return null; + const ops = (body as { ops?: unknown }).ops; + return Array.isArray(ops) ? ops.length : null; +}; + +const sendOpsBatchTooLargeReply = ( + reply: FastifyReply, + userId: number, + opsCount: number, +): FastifyReply => { + Logger.warn( + `[user:${userId}] Upload rejected: ${opsCount} ops exceeds max batch size ${MAX_OPS_PER_BATCH}`, + ); + return reply.status(413).send({ + error: `Too many operations in upload batch. Maximum is ${MAX_OPS_PER_BATCH}.`, + errorCode: SYNC_ERROR_CODES.PAYLOAD_TOO_LARGE, + maxOpsPerBatch: MAX_OPS_PER_BATCH, + }); +}; + const applyStorageUsageDelta = async ( userId: number, deltaBytes: number, @@ -573,6 +598,11 @@ export const syncRoutes = async (fastify: FastifyInstance): Promise => { ); } + const rawOpsCount = getRawOpsCount(body); + if (rawOpsCount !== null && rawOpsCount > MAX_OPS_PER_BATCH) { + return sendOpsBatchTooLargeReply(reply, userId, rawOpsCount); + } + // Validate request body const parseResult = SuperSyncUploadOpsRequestSchema.safeParse(body); if (!parseResult.success) { @@ -996,6 +1026,9 @@ export const syncRoutes = async (fastify: FastifyInstance): Promise => { .send(createValidationErrorResponse(parseResult.error.issues)); } + const snapshotRequest = parseResult.data as typeof parseResult.data & { + requestId?: string; + }; const { state, clientId, @@ -1008,7 +1041,7 @@ export const syncRoutes = async (fastify: FastifyInstance): Promise => { snapshotOpType, syncImportReason, requestId, - } = parseResult.data; + } = snapshotRequest; const syncService = getSyncService(); if (requestId) { diff --git a/packages/super-sync-server/src/sync/sync.service.ts b/packages/super-sync-server/src/sync/sync.service.ts index bf0fd5fad4..c2084f58f3 100644 --- a/packages/super-sync-server/src/sync/sync.service.ts +++ b/packages/super-sync-server/src/sync/sync.service.ts @@ -30,6 +30,7 @@ import { } from './services'; interface DuplicateOperationCandidate { + id: string; userId: number; clientId: string; actionType: string; @@ -45,12 +46,51 @@ interface DuplicateOperationCandidate { syncImportReason: string | null; } +/** + * The exact column set `isSameDuplicateOperation` needs to compare an incoming + * op against a stored one. Shared by every duplicate-detection query (batch + * prefetch + both legacy per-op checks) so a field added here can never be + * silently missed at one of the call sites. + */ +const DUPLICATE_OP_SELECT = { + id: true, + userId: true, + clientId: true, + actionType: true, + opType: true, + entityType: true, + entityId: true, + payload: true, + vectorClock: true, + schemaVersion: true, + clientTimestamp: true, + receivedAt: true, + isPayloadEncrypted: true, + syncImportReason: true, +} satisfies Prisma.OperationSelect; + interface LatestEntityOperationRow { entityId: string; clientId: string; vectorClock: unknown; } +interface LatestBatchEntityOperationRow extends LatestEntityOperationRow { + entityType: string; +} + +interface BatchUploadCandidate { + op: Operation; + resultIndex: number; + originalTimestamp: number; + fullStateVectorClock?: VectorClock; +} + +interface AcceptedBatchOperation extends BatchUploadCandidate { + serverSeq: number; + storageBytes: number; +} + // Conservative enough to avoid planner-heavy BitmapOr + Sort plans on large // histories while still replacing up to 100 per-entity round trips with one query. const CONFLICT_DETECTION_ENTITY_BATCH_SIZE = 100; @@ -67,6 +107,61 @@ const OLD_OPS_CLEANUP_MAX_DELETED_PER_RUN = 25_000; const OLD_OPS_CLEANUP_DELETE_BATCH_SIZE_MAX = 50_000; const OLD_OPS_CLEANUP_MAX_DELETED_PER_RUN_MAX = 1_000_000; +const toSafeServerSeq = (value: number | bigint | undefined, userId: number): number => { + if (typeof value === 'bigint') { + if (value < BigInt(0) || value > BigInt(Number.MAX_SAFE_INTEGER)) { + throw new Error(`Unsafe last_seq value returned for user ${userId}`); + } + const asNumber = Number(value); + if (BigInt(asNumber) !== value) { + throw new Error(`Precision-losing last_seq value returned for user ${userId}`); + } + return asNumber; + } + + if (typeof value === 'number' && Number.isSafeInteger(value) && value >= 0) { + return value; + } + + throw new Error(`Invalid last_seq value returned for user ${userId}`); +}; + +const getPrismaP2002TargetTokens = ( + err: Prisma.PrismaClientKnownRequestError, +): string[] => { + const target = err.meta?.target; + if (Array.isArray(target)) return target.map(String); + if (typeof target === 'string') return [target]; + return []; +}; + +const isRetryableOperationUniqueViolation = (err: unknown): boolean => { + if (!(err instanceof Prisma.PrismaClientKnownRequestError) || err.code !== 'P2002') { + return false; + } + + const targetTokens = getPrismaP2002TargetTokens(err); + if (targetTokens.length === 0) return true; + + const normalizedTargets = targetTokens.map((target) => + target.toLowerCase().replace(/"/g, ''), + ); + const targetSet = new Set(normalizedTargets); + + return ( + normalizedTargets.some( + (target) => + target.includes('operations_pkey') || + target.includes('operation_pkey') || + target.includes('operations_id') || + target.includes('operation_id'), + ) || + targetSet.has('id') || + (targetSet.has('user_id') && targetSet.has('server_seq')) || + (targetSet.has('userid') && targetSet.has('serverseq')) + ); +}; + const getOldOpsCleanupDeleteBatchSize = (): number => parsePositiveIntegerEnv( 'OLD_OPS_CLEANUP_DELETE_BATCH_SIZE', @@ -387,6 +482,144 @@ export class SyncService { return value; } + private clampFutureTimestamp( + userId: number, + clientId: string, + op: Operation, + now: number, + ): number { + const originalTimestamp = op.timestamp; + const maxAllowedTimestamp = now + this.config.maxClockDriftMs; + if (op.timestamp > maxAllowedTimestamp) { + op.timestamp = maxAllowedTimestamp; + Logger.audit({ + event: 'TIMESTAMP_CLAMPED', + userId, + clientId, + opId: op.id, + entityType: op.entityType, + originalTimestamp, + clampedTo: maxAllowedTimestamp, + driftMs: originalTimestamp - now, + }); + } + return originalTimestamp; + } + + private rejectedUploadResult( + userId: number, + clientId: string, + op: Operation, + error: string | undefined, + errorCode: UploadResult['errorCode'], + existingClock?: VectorClock, + ): UploadResult { + Logger.audit({ + event: 'OP_REJECTED', + userId, + clientId, + opId: op.id, + entityType: op.entityType, + entityId: op.entityId, + errorCode, + reason: error, + opType: op.opType, + }); + + return { + opId: op.id, + accepted: false, + error, + errorCode, + existingClock, + }; + } + + private getConflictEntityIds(op: Operation): string[] { + const rawEntityIds = op.entityIds?.length + ? op.entityIds + : op.entityId + ? [op.entityId] + : []; + return Array.from(new Set(rawEntityIds)); + } + + private getEntityConflictKey(entityType: string, entityId: string): string { + return `${entityType}\u0000${entityId}`; + } + + private getBatchConflictEntityPairs( + candidates: BatchUploadCandidate[], + ): { entityType: string; entityId: string }[] { + const entityPairs = new Map(); + + for (const candidate of candidates) { + if (isFullStateOpType(candidate.op.opType)) continue; + + for (const entityId of this.getConflictEntityIds(candidate.op)) { + entityPairs.set(this.getEntityConflictKey(candidate.op.entityType, entityId), { + entityType: candidate.op.entityType, + entityId, + }); + } + } + + return Array.from(entityPairs.values()); + } + + private async prefetchLatestEntityOpsForBatch( + userId: number, + entityPairs: { entityType: string; entityId: string }[], + tx: Prisma.TransactionClient, + ): Promise> { + const latestByEntity = new Map(); + if (entityPairs.length === 0) return latestByEntity; + + for ( + let start = 0; + start < entityPairs.length; + start += CONFLICT_DETECTION_ENTITY_BATCH_SIZE + ) { + const touchedRows = entityPairs + .slice(start, start + CONFLICT_DETECTION_ENTITY_BATCH_SIZE) + .map(({ entityType, entityId }) => Prisma.sql`(${entityType}, ${entityId})`); + + const latestOps = await tx.$queryRaw` + SELECT DISTINCT ON (o.entity_type, o.entity_id) + o.entity_type AS "entityType", + o.entity_id AS "entityId", + o.client_id AS "clientId", + o.vector_clock AS "vectorClock" + FROM operations o + JOIN (VALUES ${Prisma.join(touchedRows)}) AS touched(entity_type, entity_id) + ON touched.entity_type = o.entity_type + AND touched.entity_id = o.entity_id + WHERE o.user_id = ${userId} + ORDER BY o.entity_type, o.entity_id, o.server_seq DESC + `; + + for (const latestOp of latestOps) { + latestByEntity.set( + this.getEntityConflictKey(latestOp.entityType, latestOp.entityId), + latestOp, + ); + } + } + + return latestByEntity; + } + + private pruneVectorClockForStorage(op: Operation): void { + const beforeSize = Object.keys(op.vectorClock).length; + op.vectorClock = limitVectorClockSize(op.vectorClock, [op.clientId]); + const afterSize = Object.keys(op.vectorClock).length; + if (afterSize < beforeSize) { + Logger.debug( + `[client:${op.clientId}] Vector clock pruned from ${beforeSize} to ${afterSize} before storage`, + ); + } + } + // === Upload Operations === async uploadOps( @@ -397,6 +630,8 @@ export class SyncService { ): Promise { const results: UploadResult[] = []; const now = Date.now(); + const txStartedAt = Date.now(); + let uploadDbRoundtrips = 0; try { // Use transaction to acquire write lock and ensure atomicity @@ -438,16 +673,6 @@ export class SyncService { Logger.info(`[user:${userId}] Clean slate completed - all data deleted`); } - // Ensure user has sync state row (init if needed) - // We assume user exists in `users` table because of foreign key, - // but if `uploadOps` is called, authentication should have verified user existence. - // However, `user_sync_state` might not exist yet. - await tx.userSyncState.upsert({ - where: { userId }, - create: { userId, lastSeq: 0 }, - update: {}, // No-op update to ensure it exists - }); - // Track the delta-bytes for accepted ops so we can write // `users.storage_used_bytes` atomically in the same transaction as the // op inserts. Doing the counter write outside this transaction (as @@ -456,13 +681,39 @@ export class SyncService { // `markStorageNeedsReconcile` marker is lost too. let acceptedDeltaBytes = 0; let unserializableAccepted = 0; - for (const op of ops) { - const result = await this.processOperation(userId, clientId, op, now, tx); - results.push(result); - if (result.accepted) { - const sized = computeOpStorageBytes(op); - acceptedDeltaBytes += sized.bytes; - if (sized.fallback) unserializableAccepted += 1; + + if (this.config.batchUpload) { + const batchResult = await this.processOperationBatch( + userId, + clientId, + ops, + now, + tx, + ); + results.push(...batchResult.results); + acceptedDeltaBytes = batchResult.acceptedDeltaBytes; + unserializableAccepted = batchResult.unserializableAccepted; + uploadDbRoundtrips += batchResult.dbRoundtrips; + } else { + // Ensure user has sync state row (init if needed) + // We assume user exists in `users` table because of foreign key, + // but if `uploadOps` is called, authentication should have verified user existence. + // However, `user_sync_state` might not exist yet. + await tx.userSyncState.upsert({ + where: { userId }, + create: { userId, lastSeq: 0 }, + update: {}, // No-op update to ensure it exists + }); + uploadDbRoundtrips++; + + for (const op of ops) { + const result = await this.processOperation(userId, clientId, op, now, tx); + results.push(result); + if (result.accepted) { + const sized = computeOpStorageBytes(op); + acceptedDeltaBytes += sized.bytes; + if (sized.fallback) unserializableAccepted += 1; + } } } if (unserializableAccepted > 0) { @@ -493,6 +744,9 @@ export class SyncService { lastSeenAt: BigInt(now), }, }); + uploadDbRoundtrips++; + + if (this.config.batchUpload && acceptedDeltaBytes === 0) return; // W1: write the storage counter as the LAST statement before COMMIT // so the row-level write lock on `users` is held for only the @@ -508,6 +762,7 @@ export class SyncService { SET storage_used_bytes = GREATEST(storage_used_bytes + ${delta}::bigint, 0::bigint) WHERE id = ${userId} `; + uploadDbRoundtrips++; } else if (acceptedDeltaBytes > 0 && isCleanSlate) { const delta = BigInt(Math.floor(acceptedDeltaBytes)); await tx.$executeRaw` @@ -515,6 +770,7 @@ export class SyncService { SET storage_used_bytes = ${delta}::bigint WHERE id = ${userId} `; + uploadDbRoundtrips++; } }, { @@ -522,9 +778,9 @@ export class SyncService { // Default Prisma timeout (5s) is too short for these. Use 60s to match generateSnapshot. timeout: 60000, // FIX 1.6: Set explicit isolation level for strict consistency. - // REPEATABLE_READ prevents phantom reads and ensures consistent conflict detection. - // Combined with the FIX 1.5 re-check after sequence allocation, this prevents - // race conditions where two concurrent requests both pass conflict detection. + // The serial path also performs the legacy post-sequence conflict re-check. + // The batch path serializes accepted writers through the shared + // user_sync_state.last_seq row update; see ARCHITECTURE-DECISIONS.md. isolationLevel: Prisma.TransactionIsolationLevel.RepeatableRead, }, ); @@ -538,6 +794,17 @@ export class SyncService { this.storageQuotaService.clearForUser(userId); this.requestDeduplicationService.clearForUser(userId); } + + const accepted = results.filter((result) => result.accepted).length; + Logger.info('UPLOAD_BATCH_SUMMARY', { + userId, + opsInBatch: ops.length, + accepted, + rejected: results.length - accepted, + txDurationMs: Date.now() - txStartedAt, + dbRoundtrips: uploadDbRoundtrips, + batchUpload: this.config.batchUpload, + }); } catch (err) { // Transaction failed - all operations were rolled back const errorMessage = (err as Error).message || 'Unknown error'; @@ -547,6 +814,7 @@ export class SyncService { // PostgreSQL uses 40001 (serialization_failure) and 40P01 (deadlock_detected) const isSerializationFailure = (err instanceof Prisma.PrismaClientKnownRequestError && err.code === 'P2034') || + (this.config.batchUpload && isRetryableOperationUniqueViolation(err)) || errorMessage.includes('40001') || errorMessage.includes('40P01') || errorMessage.toLowerCase().includes('serialization') || @@ -627,6 +895,439 @@ export class SyncService { return out; } + /** + * Aggregate the prior vector clock, merge the full-state op's clock into it + * (max per client) and persist it as the user's latest-full-state marker. + * Shared by the batch and legacy per-op paths so the aggregate-and-merge + * semantics cannot drift between them. Costs 2 DB round trips (the aggregate + * scan + the userSyncState update). + */ + private async persistMergedFullStateClock( + tx: Prisma.TransactionClient, + userId: number, + serverSeq: number, + opClock: VectorClock, + ): Promise { + const priorAggregate = await this._aggregatePriorVectorClock(tx, userId, serverSeq); + const mergedClock: VectorClock = { ...priorAggregate }; + for (const [clientId, counter] of Object.entries(opClock)) { + mergedClock[clientId] = Math.max(mergedClock[clientId] ?? 0, counter); + } + await tx.userSyncState.update({ + where: { userId }, + data: { + latestFullStateSeq: serverSeq, + latestFullStateVectorClock: mergedClock as Prisma.InputJsonValue, + }, + }); + } + + private async processOperationBatch( + userId: number, + clientId: string, + ops: Operation[], + now: number, + tx: Prisma.TransactionClient, + ): Promise<{ + results: UploadResult[]; + acceptedDeltaBytes: number; + unserializableAccepted: number; + dbRoundtrips: number; + }> { + const results = new Array(ops.length); + let dbRoundtrips = 0; + + // Pipeline: validate+clamp -> intra-batch dedupe -> classify existing + // duplicates -> conflict-detect -> reserve seq + insert -> full-state + // clock. Each stage writes terminal rejections into `results` by index + // and passes the surviving candidates forward; the two empty-set guards + // short-circuit exactly as the original single function did. + const validatedCandidates = this.validateAndClampBatch( + userId, + clientId, + ops, + now, + results, + ); + + const uniqueCandidates = this.rejectIntraBatchDuplicates( + userId, + clientId, + validatedCandidates, + results, + ); + + if (uniqueCandidates.length === 0) { + return { + results: results as UploadResult[], + acceptedDeltaBytes: 0, + unserializableAccepted: 0, + dbRoundtrips, + }; + } + + const classified = await this.classifyExistingDuplicates( + userId, + clientId, + uniqueCandidates, + tx, + results, + ); + dbRoundtrips += classified.dbRoundtrips; + const duplicateFreeCandidates = classified.duplicateFreeCandidates; + + const conflictOutcome = await this.detectBatchConflicts( + userId, + clientId, + duplicateFreeCandidates, + tx, + results, + ); + dbRoundtrips += conflictOutcome.dbRoundtrips; + const { accepted, acceptedDeltaBytes, unserializableAccepted } = conflictOutcome; + + if (accepted.length === 0) { + return { + results: results as UploadResult[], + acceptedDeltaBytes: 0, + unserializableAccepted: 0, + dbRoundtrips, + }; + } + + dbRoundtrips += await this.reserveSeqAndInsert(userId, clientId, accepted, now, tx); + + dbRoundtrips += await this.persistBatchFullStateClock(userId, accepted, tx); + + for (const acceptedOp of accepted) { + results[acceptedOp.resultIndex] = { + opId: acceptedOp.op.id, + accepted: true, + serverSeq: acceptedOp.serverSeq, + }; + } + + return { + results: results as UploadResult[], + acceptedDeltaBytes, + unserializableAccepted, + dbRoundtrips, + }; + } + + /** + * Stage 1: clamp future timestamps and validate every op in memory (no DB). + * Invalid ops get a terminal rejection written into `results` by index. + */ + private validateAndClampBatch( + userId: number, + clientId: string, + ops: Operation[], + now: number, + results: UploadResult[], + ): BatchUploadCandidate[] { + const validatedCandidates: BatchUploadCandidate[] = []; + for (let i = 0; i < ops.length; i++) { + const op = ops[i]; + const originalTimestamp = this.clampFutureTimestamp(userId, clientId, op, now); + const validation = this.validationService.validateOp(op, clientId); + + if (!validation.valid) { + results[i] = this.rejectedUploadResult( + userId, + clientId, + op, + validation.error, + validation.errorCode, + ); + continue; + } + + validatedCandidates.push({ + op, + resultIndex: i, + originalTimestamp, + fullStateVectorClock: isFullStateOpType(op.opType) + ? { ...op.vectorClock } + : undefined, + }); + } + return validatedCandidates; + } + + /** + * Stage 2: within a single batch, accept the first op for an id and reject + * every later op sharing that id as DUPLICATE_OPERATION (by id, not content + * — see plan §1a step 2 / the C4 divergence note). Must run before sequence + * reservation so a duplicate never consumes a server_seq. + */ + private rejectIntraBatchDuplicates( + userId: number, + clientId: string, + validatedCandidates: BatchUploadCandidate[], + results: UploadResult[], + ): BatchUploadCandidate[] { + const seenOpIds = new Set(); + const uniqueCandidates: BatchUploadCandidate[] = []; + for (const candidate of validatedCandidates) { + if (seenOpIds.has(candidate.op.id)) { + results[candidate.resultIndex] = this.rejectedUploadResult( + userId, + clientId, + candidate.op, + 'Duplicate operation ID', + SYNC_ERROR_CODES.DUPLICATE_OPERATION, + ); + continue; + } + seenOpIds.add(candidate.op.id); + uniqueCandidates.push(candidate); + } + return uniqueCandidates; + } + + /** + * Stage 3: prefetch any already-persisted ops sharing an incoming id (one + * query) and classify each as an idempotent retry (DUPLICATE_OPERATION) or + * an id collision with different content (INVALID_OP_ID). Survivors are + * returned for conflict detection. + */ + private async classifyExistingDuplicates( + userId: number, + clientId: string, + uniqueCandidates: BatchUploadCandidate[], + tx: Prisma.TransactionClient, + results: UploadResult[], + ): Promise<{ + duplicateFreeCandidates: BatchUploadCandidate[]; + dbRoundtrips: number; + }> { + const existingOps = await tx.operation.findMany({ + where: { id: { in: uniqueCandidates.map((candidate) => candidate.op.id) } }, + select: DUPLICATE_OP_SELECT, + }); + const existingOpById = new Map( + existingOps.map((existingOp) => [existingOp.id, existingOp]), + ); + + const duplicateFreeCandidates: BatchUploadCandidate[] = []; + for (const candidate of uniqueCandidates) { + const existingOp = existingOpById.get(candidate.op.id); + if (!existingOp) { + duplicateFreeCandidates.push(candidate); + continue; + } + + if ( + !this.isSameDuplicateOperation( + existingOp, + userId, + candidate.op, + candidate.originalTimestamp, + ) + ) { + results[candidate.resultIndex] = this.rejectedUploadResult( + userId, + clientId, + candidate.op, + 'Operation ID already belongs to a different operation', + SYNC_ERROR_CODES.INVALID_OP_ID, + ); + continue; + } + + results[candidate.resultIndex] = this.rejectedUploadResult( + userId, + clientId, + candidate.op, + 'Duplicate operation ID', + SYNC_ERROR_CODES.DUPLICATE_OPERATION, + ); + } + return { duplicateFreeCandidates, dbRoundtrips: 1 }; + } + + /** + * Stage 4: prefetch the latest op per touched entity and run conflict + * detection in memory. The prefetched map is updated as each non-full-state + * op is accepted so intra-batch conflicts on the same entity resolve in + * serial order — this must stay co-located with the conflict loop. Accepted + * ops are sized for the storage counter here. + */ + private async detectBatchConflicts( + userId: number, + clientId: string, + duplicateFreeCandidates: BatchUploadCandidate[], + tx: Prisma.TransactionClient, + results: UploadResult[], + ): Promise<{ + accepted: AcceptedBatchOperation[]; + acceptedDeltaBytes: number; + unserializableAccepted: number; + dbRoundtrips: number; + }> { + const entityPairs = this.getBatchConflictEntityPairs(duplicateFreeCandidates); + const dbRoundtrips = Math.ceil( + entityPairs.length / CONFLICT_DETECTION_ENTITY_BATCH_SIZE, + ); + const latestOpByEntity = await this.prefetchLatestEntityOpsForBatch( + userId, + entityPairs, + tx, + ); + + const accepted: AcceptedBatchOperation[] = []; + let acceptedDeltaBytes = 0; + let unserializableAccepted = 0; + + for (const candidate of duplicateFreeCandidates) { + const { op } = candidate; + if (!isFullStateOpType(op.opType)) { + let conflict: ConflictResult | null = null; + for (const entityId of this.getConflictEntityIds(op)) { + const existingOp = latestOpByEntity.get( + this.getEntityConflictKey(op.entityType, entityId), + ); + if (!existingOp) continue; + + const conflictResult = this.resolveConflictForExistingOp( + op, + entityId, + existingOp, + ); + if (conflictResult.hasConflict) { + conflict = conflictResult; + break; + } + } + + if (conflict) { + const errorCode = + conflict.conflictType === 'concurrent' || + conflict.conflictType === 'equal_different_client' + ? SYNC_ERROR_CODES.CONFLICT_CONCURRENT + : SYNC_ERROR_CODES.CONFLICT_SUPERSEDED; + results[candidate.resultIndex] = this.rejectedUploadResult( + userId, + clientId, + op, + conflict.reason, + errorCode, + conflict.existingClock, + ); + continue; + } + } + + this.pruneVectorClockForStorage(op); + const sized = computeOpStorageBytes(op); + acceptedDeltaBytes += sized.bytes; + if (sized.fallback) unserializableAccepted++; + + const acceptedOperation: AcceptedBatchOperation = { + ...candidate, + serverSeq: 0, + storageBytes: sized.bytes, + }; + accepted.push(acceptedOperation); + + if (!isFullStateOpType(op.opType)) { + for (const entityId of this.getConflictEntityIds(op)) { + latestOpByEntity.set(this.getEntityConflictKey(op.entityType, entityId), { + entityType: op.entityType, + entityId, + clientId: op.clientId, + vectorClock: op.vectorClock, + }); + } + } + } + + return { accepted, acceptedDeltaBytes, unserializableAccepted, dbRoundtrips }; + } + + /** + * Stage 5: reserve a contiguous server_seq range for the accepted ops in one + * INSERT ... ON CONFLICT (which also serializes concurrent batches via the + * user_sync_state row lock), assign each op its seq, and bulk-insert. Returns + * the DB round trips consumed (2). + */ + private async reserveSeqAndInsert( + userId: number, + clientId: string, + accepted: AcceptedBatchOperation[], + now: number, + tx: Prisma.TransactionClient, + ): Promise { + const syncStateRows = await tx.$queryRaw>` + INSERT INTO user_sync_state (user_id, last_seq) + VALUES (${userId}, ${accepted.length}) + ON CONFLICT (user_id) DO UPDATE + SET last_seq = user_sync_state.last_seq + EXCLUDED.last_seq + RETURNING last_seq AS "lastSeq" + `; + const lastSeq = toSafeServerSeq(syncStateRows[0]?.lastSeq, userId); + if (lastSeq < accepted.length) { + throw new Error(`Failed to reserve server sequence range for user ${userId}`); + } + const firstSeq = lastSeq - accepted.length + 1; + for (let i = 0; i < accepted.length; i++) { + accepted[i].serverSeq = firstSeq + i; + } + + await tx.operation.createMany({ + data: accepted.map((candidate) => ({ + id: candidate.op.id, + userId, + clientId, + serverSeq: candidate.serverSeq, + actionType: candidate.op.actionType, + opType: candidate.op.opType, + entityType: candidate.op.entityType, + entityId: candidate.op.entityId ?? null, + payload: candidate.op.payload as Prisma.InputJsonValue, + payloadBytes: BigInt(candidate.storageBytes), + vectorClock: candidate.op.vectorClock as Prisma.InputJsonValue, + schemaVersion: candidate.op.schemaVersion, + clientTimestamp: BigInt(candidate.op.timestamp), + receivedAt: BigInt(now), + isPayloadEncrypted: candidate.op.isPayloadEncrypted ?? false, + syncImportReason: candidate.op.syncImportReason ?? null, + })), + }); + return 2; + } + + /** + * Stage 6: if the batch accepted any full-state op, persist the merged + * latest-full-state vector clock once for the last such op (last write + * wins). Returns the DB round trips consumed (2 if a full-state op was + * accepted, else 0). + */ + private async persistBatchFullStateClock( + userId: number, + accepted: AcceptedBatchOperation[], + tx: Prisma.TransactionClient, + ): Promise { + let lastFullStateOp: AcceptedBatchOperation | null = null; + for (const acceptedOp of accepted) { + if (acceptedOp.fullStateVectorClock) { + lastFullStateOp = acceptedOp; + } + } + + if (lastFullStateOp?.fullStateVectorClock) { + await this.persistMergedFullStateClock( + tx, + userId, + lastFullStateOp.serverSeq, + lastFullStateOp.fullStateVectorClock, + ); + return 2; + } + return 0; + } + /** * Process a single operation within a transaction. * Handles validation, conflict detection, and persistence. @@ -638,22 +1339,9 @@ export class SyncService { now: number, tx: Prisma.TransactionClient, ): Promise { - // Clamp future timestamps instead of rejecting them (prevents silent data loss) - const originalTimestamp = op.timestamp; - const maxAllowedTimestamp = now + this.config.maxClockDriftMs; - if (op.timestamp > maxAllowedTimestamp) { - op.timestamp = maxAllowedTimestamp; - Logger.audit({ - event: 'TIMESTAMP_CLAMPED', - userId, - clientId, - opId: op.id, - entityType: op.entityType, - originalTimestamp, - clampedTo: maxAllowedTimestamp, - driftMs: originalTimestamp - now, - }); - } + // Clamp future timestamps instead of rejecting them (prevents silent data + // loss). Shares the exact clamp + audit with the batch path. + const originalTimestamp = this.clampFutureTimestamp(userId, clientId, op, now); // Validate operation (including clientId match) const validation = this.validationService.validateOp(op, clientId); @@ -690,22 +1378,7 @@ export class SyncService { // from advancing lastSeq. const existingOp = await tx.operation.findUnique({ where: { id: op.id }, - select: { - id: true, - userId: true, - clientId: true, - actionType: true, - opType: true, - entityType: true, - entityId: true, - payload: true, - vectorClock: true, - schemaVersion: true, - clientTimestamp: true, - receivedAt: true, - isPayloadEncrypted: true, - syncImportReason: true, - }, + select: DUPLICATE_OP_SELECT, }); if (existingOp) { @@ -847,6 +1520,7 @@ export class SyncService { entityType: op.entityType, entityId: op.entityId ?? null, payload: op.payload as Prisma.InputJsonValue, + payloadBytes: BigInt(computeOpStorageBytes(op).bytes), vectorClock: op.vectorClock as Prisma.InputJsonValue, schemaVersion: op.schemaVersion, clientTimestamp: BigInt(op.timestamp), @@ -864,22 +1538,7 @@ export class SyncService { if (createResult.count === 0) { const duplicateOp = await tx.operation.findUnique({ where: { id: op.id }, - select: { - id: true, - userId: true, - clientId: true, - actionType: true, - opType: true, - entityType: true, - entityId: true, - payload: true, - vectorClock: true, - schemaVersion: true, - clientTimestamp: true, - receivedAt: true, - isPayloadEncrypted: true, - syncImportReason: true, - }, + select: DUPLICATE_OP_SELECT, }); if (!duplicateOp) { @@ -943,18 +1602,7 @@ export class SyncService { // to per-snapshot — full-state ops are rare so the upload-time scan is // strictly cheaper overall. Stored unpruned; the download path applies // `limitVectorClockSize` with `preserveClientIds` known to that read. - const priorAggregate = await this._aggregatePriorVectorClock(tx, userId, serverSeq); - const mergedClock: VectorClock = { ...priorAggregate }; - for (const [clientId, counter] of Object.entries(fullStateVectorClock)) { - mergedClock[clientId] = Math.max(mergedClock[clientId] ?? 0, counter); - } - await tx.userSyncState.update({ - where: { userId }, - data: { - latestFullStateSeq: serverSeq, - latestFullStateVectorClock: mergedClock as Prisma.InputJsonValue, - }, - }); + await this.persistMergedFullStateClock(tx, userId, serverSeq, fullStateVectorClock); } return { @@ -1153,10 +1801,15 @@ export class SyncService { operationsBytes: number; snapshotBytes: number; totalBytes: number; + hasUnbackfilledRows: boolean; }> { return this.storageQuotaService.calculateStorageUsage(userId); } + async assertPayloadBytesBackfillComplete(): Promise { + return this.storageQuotaService.assertPayloadBytesBackfillComplete(); + } + async checkStorageQuota( userId: number, additionalBytes: number, @@ -1369,17 +2022,14 @@ export class SyncService { // Full-state ops (SYNC_IMPORT/BACKUP_IMPORT/REPAIR) can be up to 20MB each, // so the APPROX_BYTES_PER_OP=1024 fallback used for delta ops would undercount // by ~20000x and leave the cached counter permanently low if a reconcile - // failure later rolls back to that figure. Measure the exact bytes for the - // restore-point rows in the deletion window via pg_column_size BEFORE we - // delete them — `deleteOldestRestorePointAndOps` deletes at most ONE - // restore point per call (or zero, when it keeps the single remaining one), - // so this is a bounded 0-1 row scan that does not reintroduce the DoS the - // earlier SUM(pg_column_size) over every delta op caused. + // failure later rolls back to that figure. Use the write-time payload_bytes + // value so cleanup accounting matches quota reconciliation without + // detoasting JSONB payloads. const fullStateRows = await prisma.$queryRaw< Array<{ exact_bytes: bigint | null; full_state_count: bigint }> >` SELECT - COALESCE(SUM(pg_column_size(payload) + pg_column_size(vector_clock)), 0) AS exact_bytes, + COALESCE(SUM(payload_bytes), 0) AS exact_bytes, COUNT(*)::bigint AS full_state_count FROM operations WHERE user_id = ${userId} diff --git a/packages/super-sync-server/src/sync/sync.types.ts b/packages/super-sync-server/src/sync/sync.types.ts index 14f39a843c..d84a2e8f36 100644 --- a/packages/super-sync-server/src/sync/sync.types.ts +++ b/packages/super-sync-server/src/sync/sync.types.ts @@ -382,6 +382,7 @@ export interface SyncConfig { downloadRateLimit: { max: number; windowMs: number }; retentionMs: number; // Unified retention period for ops, devices, and validation maxClockDriftMs: number; + batchUpload: boolean; } // Time constants (in milliseconds) @@ -404,4 +405,5 @@ export const DEFAULT_SYNC_CONFIG: SyncConfig = { downloadRateLimit: { max: 200, windowMs: MS_PER_MINUTE }, retentionMs: RETENTION_MS, // 45 days - used for ops, devices, and validation maxClockDriftMs: MS_PER_MINUTE, // 60 seconds + batchUpload: false, }; diff --git a/packages/super-sync-server/src/test-routes.ts b/packages/super-sync-server/src/test-routes.ts index 56a48d1584..0fce58efe5 100644 --- a/packages/super-sync-server/src/test-routes.ts +++ b/packages/super-sync-server/src/test-routes.ts @@ -10,6 +10,7 @@ import * as jwt from 'jsonwebtoken'; import { prisma } from './db'; import { Logger } from './logger'; import { getJwtSecret, JWT_EXPIRY } from './auth'; +import { authCache } from './auth-cache'; const BCRYPT_ROUNDS = 12; @@ -129,6 +130,7 @@ export const testRoutes = async (fastify: FastifyInstance): Promise => { prisma.userSyncState.deleteMany(), prisma.user.deleteMany(), ]); + authCache.clear(); Logger.info('[TEST] All test data cleaned up'); @@ -171,8 +173,12 @@ export const testRoutes = async (fastify: FastifyInstance): Promise => { } try { + // AUTH_CACHE_INVALIDATION: test deletion should mirror production account deletion. + authCache.invalidate(userId); // CASCADE delete handles: operations, syncState, devices (via Prisma schema) await prisma.user.delete({ where: { id: userId } }); + // AUTH_CACHE_INVALIDATION: clear any token cached while the delete was in flight. + authCache.invalidate(userId); Logger.info(`[TEST] Deleted test user ID: ${userId}`); return reply.send({ deleted: true, userId }); } catch (err: unknown) { diff --git a/packages/super-sync-server/tests/auth-cache.spec.ts b/packages/super-sync-server/tests/auth-cache.spec.ts new file mode 100644 index 0000000000..229da23cb2 --- /dev/null +++ b/packages/super-sync-server/tests/auth-cache.spec.ts @@ -0,0 +1,150 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import * as jwt from 'jsonwebtoken'; + +const jwtSecret = vi.hoisted(() => { + const secret = 'a'.repeat(32); + process.env.JWT_SECRET = secret; + return secret; +}); + +vi.mock('../src/auth', async (importOriginal) => { + return await importOriginal(); +}); + +vi.mock('../src/logger', () => ({ + Logger: { + info: vi.fn(), + warn: vi.fn(), + debug: vi.fn(), + error: vi.fn(), + }, +})); + +import { verifyToken, revokeAllTokens } from '../src/auth'; +import { authCache } from '../src/auth-cache'; +import { prisma } from '../src/db'; + +const createToken = (tokenVersion: number = 0): string => + jwt.sign({ userId: 1, email: 'user@example.com', tokenVersion }, jwtSecret, { + expiresIn: '1h', + }); + +describe('auth verification cache', () => { + beforeEach(() => { + vi.clearAllMocks(); + authCache.clear(); + }); + + it('should reuse a warm verified-token cache entry', async () => { + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + id: 1, + tokenVersion: 0, + isVerified: 1, + } as any); + + const token = createToken(); + + await expect(verifyToken(token)).resolves.toEqual({ + valid: true, + userId: 1, + email: 'user@example.com', + }); + await expect(verifyToken(token)).resolves.toEqual({ + valid: true, + userId: 1, + email: 'user@example.com', + }); + + expect(prisma.user.findUnique).toHaveBeenCalledTimes(1); + }); + + it('should fall through to the database on tokenVersion mismatch', async () => { + authCache.set(1, 1, true); + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + id: 1, + tokenVersion: 0, + isVerified: 1, + } as any); + + await expect(verifyToken(createToken(0))).resolves.toEqual( + expect.objectContaining({ valid: true }), + ); + + expect(prisma.user.findUnique).toHaveBeenCalledTimes(1); + }); + + it('should invalidate the cache when revoking all tokens', async () => { + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + id: 1, + tokenVersion: 0, + isVerified: 1, + } as any); + const token = createToken(); + + await verifyToken(token); + expect(prisma.user.findUnique).toHaveBeenCalledTimes(1); + + vi.mocked(prisma.user.update).mockResolvedValue({} as any); + await revokeAllTokens(1); + + vi.mocked(prisma.user.findUnique).mockClear(); + await verifyToken(token); + + expect(prisma.user.findUnique).toHaveBeenCalledTimes(1); + }); + + it('should not re-cache a token when invalidation happens during verification', async () => { + const token = createToken(0); + let resolveFindUnique!: (value: { + id: number; + tokenVersion: number; + isVerified: number; + }) => void; + + vi.mocked(prisma.user.findUnique) + .mockReturnValueOnce( + new Promise((resolve) => { + resolveFindUnique = resolve; + }) as ReturnType, + ) + .mockResolvedValueOnce({ + id: 1, + tokenVersion: 1, + isVerified: 1, + } as any); + vi.mocked(prisma.user.update).mockResolvedValue({} as any); + + const inFlightVerification = verifyToken(token); + await Promise.resolve(); + + await revokeAllTokens(1); + resolveFindUnique({ id: 1, tokenVersion: 0, isVerified: 1 }); + + await expect(inFlightVerification).resolves.toEqual( + expect.objectContaining({ valid: true }), + ); + + await expect(verifyToken(token)).resolves.toEqual({ + valid: false, + reason: 'Token was revoked. Please log in again to get a new token.', + }); + expect(prisma.user.findUnique).toHaveBeenCalledTimes(2); + }); + + it('bounds invalidationVersions and keeps recent invalidations newest', () => { + // A long-ago invalidation must not pin heap forever. + authCache.invalidate(1); + expect(authCache.getInvalidationVersion(1)).toBe(1); + + // Push >10k distinct invalidations so user 1 (the oldest) is evicted. + for (let userId = 2; userId <= 10_002; userId++) { + authCache.invalidate(userId); + } + + // Evicted -> reverts to the default (0). Memory is bounded. + expect(authCache.getInvalidationVersion(1)).toBe(0); + // A freshly-invalidated user sits at the MRU tail and is retained, so the + // CAS race protection still holds for the window that matters. + expect(authCache.getInvalidationVersion(10_002)).toBe(1); + }); +}); diff --git a/packages/super-sync-server/tests/config.spec.ts b/packages/super-sync-server/tests/config.spec.ts index 3d92c37693..23f0e792da 100644 --- a/packages/super-sync-server/tests/config.spec.ts +++ b/packages/super-sync-server/tests/config.spec.ts @@ -228,6 +228,43 @@ describe('loadConfigFromEnv - CORS_ORIGINS parsing', () => { }); }); +describe('loadConfigFromEnv - SuperSync rollout flags', () => { + beforeEach(() => { + resetEnv(); + }); + + afterEach(() => { + resetEnv(); + }); + + it('should keep batch uploads disabled by default', async () => { + const { loadConfigFromEnv } = await importConfig(); + const config = loadConfigFromEnv(); + + expect(config.batchUpload).toBe(false); + }); + + it('should enable batch uploads only after the payload byte backfill completed', async () => { + process.env.SUPERSYNC_BATCH_UPLOAD = 'true'; + process.env.SUPERSYNC_PAYLOAD_BYTES_BACKFILL_COMPLETE = 'true'; + + const { loadConfigFromEnv } = await importConfig(); + const config = loadConfigFromEnv(); + + expect(config.batchUpload).toBe(true); + }); + + it('should reject batch uploads before the payload byte backfill completed', async () => { + process.env.SUPERSYNC_BATCH_UPLOAD = 'true'; + + const { loadConfigFromEnv } = await importConfig(); + + expect(() => loadConfigFromEnv()).toThrow( + 'SUPERSYNC_BATCH_UPLOAD=true requires SUPERSYNC_PAYLOAD_BYTES_BACKFILL_COMPLETE=true', + ); + }); +}); + describe('DEFAULT_CORS_ORIGINS', () => { beforeEach(() => { resetEnv(); diff --git a/packages/super-sync-server/tests/migration-sql.spec.ts b/packages/super-sync-server/tests/migration-sql.spec.ts index ffb1316a30..d2fbc0bd9b 100644 --- a/packages/super-sync-server/tests/migration-sql.spec.ts +++ b/packages/super-sync-server/tests/migration-sql.spec.ts @@ -57,10 +57,84 @@ describe('performance migrations', () => { expect(migrationSql).not.toMatch(/\bBEGIN\b|\bCOMMIT\b/i); }); + it('adds partial encrypted-op sequence index concurrently', () => { + const migrationSql = readFileSync( + join( + currentDir, + '../prisma/migrations/20260514000000_add_encrypted_ops_partial_index/migration.sql', + ), + 'utf8', + ); + + expect(migrationSql).toContain('CREATE INDEX CONCURRENTLY'); + expect(migrationSql).toContain( + 'DROP INDEX CONCURRENTLY IF EXISTS "operations_user_id_server_seq_encrypted_idx"', + ); + expect(migrationSql).toContain('"operations_user_id_server_seq_encrypted_idx"'); + expect(migrationSql).toContain('ON "operations"("user_id", "server_seq")'); + expect(migrationSql).toContain('WHERE "is_payload_encrypted" = true'); + expect(migrationSql).not.toMatch(/\bDROP\s+TABLE\b/i); + expect(migrationSql).not.toMatch(/\bBEGIN\b|\bCOMMIT\b/i); + }); + + it('adds operation payload_bytes as a metadata-only column (no table rewrite)', () => { + const migrationSql = readFileSync( + join( + currentDir, + '../prisma/migrations/20260514000001_add_operation_payload_bytes/migration.sql', + ), + 'utf8', + ); + + // ADD COLUMN ... NOT NULL DEFAULT is a metadata-only operation on + // PostgreSQL 11+ (the default is stored in pg_attribute, no table rewrite). + // These guards lock in the fast path: a future edit to a volatile/expression + // default or a separate UPDATE backfill would rewrite/lock a 100M-row table. + expect(migrationSql).toMatch( + /ALTER TABLE "operations"\s+ADD COLUMN "payload_bytes" BIGINT NOT NULL DEFAULT 0/i, + ); + expect(migrationSql).not.toMatch(/\bUPDATE\b/i); + expect(migrationSql).not.toMatch(/\bUSING\b/i); + expect(migrationSql).not.toMatch(/DEFAULT\s+(?!0\b)/i); + expect(migrationSql).not.toMatch(/\bDROP\s+TABLE\b/i); + expect(migrationSql).not.toMatch(/\bBEGIN\b|\bCOMMIT\b/i); + }); + + it('adds the payload_bytes unbackfilled partial index concurrently', () => { + const migrationSql = readFileSync( + join( + currentDir, + '../prisma/migrations/20260514000002_add_payload_bytes_unbackfilled_index/migration.sql', + ), + 'utf8', + ); + + expect(migrationSql).toContain('CREATE INDEX CONCURRENTLY'); + expect(migrationSql).toContain( + 'DROP INDEX CONCURRENTLY IF EXISTS "operations_payload_bytes_unbackfilled_idx"', + ); + expect(migrationSql).toContain('"operations_payload_bytes_unbackfilled_idx"'); + expect(migrationSql).toContain('ON "operations"("user_id", "id")'); + // Partial predicate must match the boot self-check / quota probe + // (payload_bytes = 0) so the index drains to empty post-backfill. + expect(migrationSql).toContain('WHERE "payload_bytes" = 0'); + expect(migrationSql).not.toMatch(/\bDROP\s+TABLE\b/i); + expect(migrationSql).not.toMatch(/\bALTER\s+TABLE\b/i); + expect(migrationSql).not.toMatch(/\bBEGIN\b|\bCOMMIT\b/i); + }); + it('runs migrations before replacing the app during compose deploys', () => { const deployScript = readFileSync(join(currentDir, '../scripts/deploy.sh'), 'utf8'); + const runtimeMigrateScript = readFileSync( + join(currentDir, '../scripts/migrate-deploy.sh'), + 'utf8', + ); const dockerfile = readFileSync(join(currentDir, '../Dockerfile'), 'utf8'); const composeFile = readFileSync(join(currentDir, '../docker-compose.yml'), 'utf8'); + const helmDeployment = readFileSync( + join(currentDir, '../helm/supersync/templates/deployment.yaml'), + 'utf8', + ); const migrationCommand = 'npx prisma migrate deploy'; const startCommand = 'up -d --wait --wait-timeout "$WAIT_TIMEOUT"'; const externalDbStartCommand = @@ -78,7 +152,16 @@ describe('performance migrations', () => { expect(deployScript).toContain( 'FULL_STATE_INDEX_MIGRATION="20260512000000_add_full_state_sequence_index_drop_redundant_indexes"', ); + expect(deployScript).toContain( + 'ENCRYPTED_OPS_INDEX_MIGRATION="20260514000000_add_encrypted_ops_partial_index"', + ); + expect(deployScript).toContain( + 'PAYLOAD_BYTES_INDEX_MIGRATION="20260514000002_add_payload_bytes_unbackfilled_index"', + ); expect(deployScript).toContain('is_recoverable_full_state_index_migration_failure'); + expect(deployScript).toContain( + 'is_recoverable_encrypted_ops_index_migration_failure', + ); expect(deployScript).toContain("grep -q 'P3009'"); expect(deployScript).toContain('is_full_state_index_transaction_block_failure'); expect(deployScript).toContain("grep -q 'P3018'"); @@ -92,6 +175,9 @@ describe('performance migrations', () => { expect(deployScript).toContain( 'CREATE INDEX CONCURRENTLY \\"operations_user_id_full_state_server_seq_idx\\"', ); + expect(deployScript).toContain( + 'CREATE INDEX CONCURRENTLY "operations_user_id_server_seq_encrypted_idx"', + ); expect(deployScript).toContain( 'migrate resolve --rolled-back "$FULL_STATE_INDEX_MIGRATION"', ); @@ -104,19 +190,80 @@ describe('performance migrations', () => { expect(deployScript).toContain( 'Retrying database migrations after applying $FULL_STATE_INDEX_MIGRATION', ); + expect(deployScript).toContain( + 'Retrying database migrations after applying $ENCRYPTED_OPS_INDEX_MIGRATION', + ); + expect(deployScript).toContain( + 'is_recoverable_payload_bytes_index_migration_failure', + ); + expect(deployScript).toContain('is_payload_bytes_index_transaction_block_failure'); + expect(deployScript).toContain( + 'CREATE INDEX CONCURRENTLY "operations_payload_bytes_unbackfilled_idx"', + ); + expect(deployScript).toContain( + 'Retrying database migrations after applying $PAYLOAD_BYTES_INDEX_MIGRATION', + ); expect(deployScript).toContain(externalDbStartCommand); expect(deployScript).toContain('RUN_MIGRATIONS_ON_STARTUP'); expect(deployScript.indexOf(migrationCommand)).toBeLessThan( deployScript.indexOf(startCommand), ); expect(dockerfile).toContain('RUN_MIGRATIONS_ON_STARTUP'); + expect(dockerfile).toContain('sh scripts/migrate-deploy.sh'); + expect(dockerfile).toContain('NODE_OPTIONS=--max-old-space-size=896'); + expect(helmDeployment).toContain('sh scripts/migrate-deploy.sh'); + expect(runtimeMigrateScript).toContain('npx prisma migrate deploy'); + expect(runtimeMigrateScript).toContain('npx prisma db execute'); + expect(runtimeMigrateScript).toContain( + 'DROP INDEX CONCURRENTLY IF EXISTS "operations_user_id_server_seq_encrypted_idx"', + ); + expect(runtimeMigrateScript).toContain( + 'CREATE INDEX CONCURRENTLY "operations_user_id_server_seq_encrypted_idx"', + ); + expect(runtimeMigrateScript).toContain( + 'DROP INDEX CONCURRENTLY IF EXISTS "operations_payload_bytes_unbackfilled_idx"', + ); + expect(runtimeMigrateScript).toContain( + 'CREATE INDEX CONCURRENTLY "operations_payload_bytes_unbackfilled_idx"', + ); expect(composeFile).toContain( 'RUN_MIGRATIONS_ON_STARTUP=${RUN_MIGRATIONS_ON_STARTUP:-false}', ); + expect(composeFile).toContain( + 'SUPERSYNC_PAYLOAD_BYTES_BACKFILL_COMPLETE=${SUPERSYNC_PAYLOAD_BYTES_BACKFILL_COMPLETE:-false}', + ); expect(composeFile).toContain( 'psql -U "$$POSTGRES_USER" -d "$$POSTGRES_DB" -c "SELECT 1"', ); expect(composeFile).toContain('aliases:'); expect(composeFile).toContain('- db'); }); + + it('backfills operation payload bytes with per-user batched updates', () => { + const script = readFileSync( + join(currentDir, '../scripts/migrate-payload-bytes.ts'), + 'utf8', + ); + const packageJson = readFileSync(join(currentDir, '../package.json'), 'utf8'); + + expect(script).toContain('SELECT DISTINCT user_id'); + // Batch size sized for throughput: a tiny batch made a 100M-row backfill take + // tens of hours, prolonging the slow octet_length() quota fallback window. + expect(script).toContain('const DEFAULT_BATCH_SIZE = 500'); + expect(script).toContain('const MAX_BATCH_SIZE = 1000'); + // The override is still clamped so a fat-fingered value cannot OOM the + // Node process building the VALUES string. + expect(script).toContain('Math.min(parsed, MAX_BATCH_SIZE)'); + expect(script).toContain('userId,'); + expect(script).toContain('FROM (VALUES ${values}) AS v(id, bytes)'); + expect(script).toContain('SET payload_bytes = v.bytes'); + expect(script).toContain('storage_used_bytes = usage.total_bytes'); + expect(packageJson).toContain( + '"migrate-payload-bytes": "node dist/scripts/migrate-payload-bytes.js"', + ); + expect(packageJson).toContain( + '"migrate-payload-bytes:dev": "ts-node scripts/migrate-payload-bytes.ts"', + ); + expect(script).not.toContain('prisma.operation.update({'); + }); }); diff --git a/packages/super-sync-server/tests/snapshot.service.spec.ts b/packages/super-sync-server/tests/snapshot.service.spec.ts index b14d2a3ec6..f3a371324c 100644 --- a/packages/super-sync-server/tests/snapshot.service.spec.ts +++ b/packages/super-sync-server/tests/snapshot.service.spec.ts @@ -1574,6 +1574,78 @@ describe('SnapshotService', () => { expect(result.PROJECT).toEqual({ 'proj-1': { id: 'proj-1' } }); }); + it('should never stringify the full replay state for small delta ops', () => { + // Delta accounting is a proven over-estimate, so when the running bound + // stays well under the cap the exact measurement is provably redundant + // and skipped entirely. This matches the pre-existing per-op-loop replay + // (which did zero full stringifications below its 1000-op cadence) — the + // earlier "exactly 1" expectation encoded a regression on the dominant + // small/incremental-replay path. + const stringifySpy = vi.spyOn(JSON, 'stringify'); + const ops = Array.from({ length: 1500 }, (_, index) => ({ + id: `op-${index}`, + opType: 'CRT', + entityType: 'TASK', + entityId: `task-${index}`, + payload: { id: `task-${index}`, title: `Task ${index}` }, + isPayloadEncrypted: false, + serverSeq: index + 1, + schemaVersion: 1, + })); + + try { + service.replayOpsToState(ops as any); + + const fullStateStringifications = stringifySpy.mock.calls.filter(([value]) => { + return ( + value !== null && + typeof value === 'object' && + !Array.isArray(value) && + Object.prototype.hasOwnProperty.call(value, 'TASK') + ); + }); + expect(fullStateStringifications).toHaveLength(0); + } finally { + stringifySpy.mockRestore(); + } + }); + + it('should measure replay state immediately after a full-state op', () => { + const stringifySpy = vi.spyOn(JSON, 'stringify'); + const ops = [ + { + id: 'op-1', + opType: 'SYNC_IMPORT', + entityType: 'FULL_STATE', + entityId: null, + payload: { + appDataComplete: { + TASK: { 'task-1': { id: 'task-1' } }, + }, + }, + isPayloadEncrypted: false, + serverSeq: 1, + schemaVersion: 1, + }, + ]; + + try { + service.replayOpsToState(ops as any); + + const fullStateStringifications = stringifySpy.mock.calls.filter(([value]) => { + return ( + value !== null && + typeof value === 'object' && + !Array.isArray(value) && + Object.prototype.hasOwnProperty.call(value, 'TASK') + ); + }); + expect(fullStateStringifications.length).toBeGreaterThanOrEqual(1); + } finally { + stringifySpy.mockRestore(); + } + }); + it('SYNC_IMPORT replaces the entire state (does NOT merge into stale cache)', () => { // Regression test: previously the full-state ops used Object.assign which // merged into existing state, so stale entity types from a cached base diff --git a/packages/super-sync-server/tests/storage-quota-cleanup.spec.ts b/packages/super-sync-server/tests/storage-quota-cleanup.spec.ts index ada9a9686f..cc6d2e1a52 100644 --- a/packages/super-sync-server/tests/storage-quota-cleanup.spec.ts +++ b/packages/super-sync-server/tests/storage-quota-cleanup.spec.ts @@ -197,9 +197,9 @@ vi.mock('../src/db', () => { .fn() .mockImplementation(async (query, userIdArg, deleteUpToSeqArg) => { // The same mock serves two SQL shapes: - // 1. calculateStorageUsage's full-table SUM(pg_column_size) - // scan (slow path) — returns `[{ total }]`. - // 2. deleteOldestRestorePointAndOps's BOUNDED full-state scan + // 1. calculateStorageUsage's SUM(payload_bytes) reconcile + // — returns `[{ operations_bytes, snapshot_bytes, has_unbackfilled }]`. + // 2. deleteOldestRestorePointAndOps's bounded full-state sum // filtered by `op_type IN (SYNC_IMPORT, BACKUP_IMPORT, REPAIR)` // — returns `[{ exact_bytes, full_state_count }]`. // Detect by inspecting the SQL template fragments. @@ -232,7 +232,13 @@ vi.mock('../src/db', () => { if (isFullStateScan) { return [{ exact_bytes: totalBytes, full_state_count: fullStateCount }]; } - return [{ total: totalBytes }]; + return [ + { + operations_bytes: totalBytes, + snapshot_bytes: BigInt(0), + has_unbackfilled: false, + }, + ]; }), // decrementStorageUsage uses $executeRaw with a clamped UPDATE. // Simulate by mutating the matching user's storageUsedBytes in-place. @@ -393,14 +399,11 @@ describe('Storage Quota Cleanup', () => { expect(testOperations.size).toBe(4); // ops 4, 5, 6, 7 remain }); - it('should not run pg_column_size over the full delta-op range during cleanup-delete', async () => { - // The cleanup path used to SUM(pg_column_size(payload)) over the FULL - // range being deleted, which forced PostgreSQL to detoast every payload - // and caused the production disk-I/O DoS. Counter is decremented via an - // approximate count*const decrement for delta ops; the only allowed - // pg_column_size scan is over the bounded restore-point rows - // (op_type IN (SYNC_IMPORT, BACKUP_IMPORT, REPAIR)) so the 20MB-ish - // full-state payloads do not undercount. + it('should not run pg_column_size during cleanup-delete', async () => { + // The cleanup path used to SUM(pg_column_size(payload)) over deleted + // ranges, which forced PostgreSQL to detoast payloads and caused the + // production disk-I/O DoS. Exact cleanup accounting now uses the + // write-time payload_bytes column. const { initSyncService, getSyncService } = await import('../src/sync/sync.service'); const { prisma } = await import('../src/db'); @@ -418,9 +421,7 @@ describe('Storage Quota Cleanup', () => { const offendingCalls = vi.mocked(prisma.$queryRaw).mock.calls.filter((call) => { const queryParts = call[0] as unknown as TemplateStringsArray; const sql = Array.from(queryParts).join(''); - // Only block the unbounded variant; the bounded full-state scan is - // expected and gated by an `op_type IN (...)` filter. - return sql.includes('pg_column_size') && !sql.includes('op_type IN'); + return sql.includes('pg_column_size'); }); expect(offendingCalls).toHaveLength(0); }); @@ -730,12 +731,12 @@ describe('Storage Quota Cleanup', () => { storageQuotaBytes: BigInt(quota), }); - // Use a larger payload so the exact-pg_column_size measurement for the + // Use a larger payload so the exact payload_bytes measurement for the // restore point produces a freedBytes value comparable to the realistic // 20MB-scale payloads this code path was tuned for. With tiny payloads - // (`{}`), the new exact accounting would mean each iteration barely - // dents the counter and the success-then-reconcile-disproves flow this - // test exercises never fires. + // (`{}`), exact accounting would mean each iteration barely dents the + // counter and the success-then-reconcile-disproves flow this test + // exercises never fires. const restorePayload = { state: 'x'.repeat(2000) }; createOp(clientId, userId, { opType: 'SYNC_IMPORT', payload: restorePayload }); // seq 1 - first approximate delete createOp(clientId, userId); // seq 2 diff --git a/packages/super-sync-server/tests/storage-quota.service.spec.ts b/packages/super-sync-server/tests/storage-quota.service.spec.ts index eb492b474c..20a0da9b39 100644 --- a/packages/super-sync-server/tests/storage-quota.service.spec.ts +++ b/packages/super-sync-server/tests/storage-quota.service.spec.ts @@ -33,10 +33,13 @@ describe('StorageQuotaService', () => { describe('calculateStorageUsage', () => { it('should calculate storage from operations and snapshot', async () => { - vi.mocked(prisma.$queryRaw).mockResolvedValue([{ total: BigInt(5000) }]); - vi.mocked(prisma.userSyncState.findUnique).mockResolvedValue({ - snapshotData: Buffer.alloc(3000), - } as any); + vi.mocked(prisma.$queryRaw).mockResolvedValue([ + { + operations_bytes: BigInt(5000), + snapshot_bytes: BigInt(3000), + has_unbackfilled: false, + }, + ]); const result = await service.calculateStorageUsage(1); @@ -44,12 +47,14 @@ describe('StorageQuotaService', () => { operationsBytes: 5000, snapshotBytes: 3000, totalBytes: 8000, + hasUnbackfilledRows: false, }); }); it('should handle null operation total', async () => { - vi.mocked(prisma.$queryRaw).mockResolvedValue([{ total: null }]); - vi.mocked(prisma.userSyncState.findUnique).mockResolvedValue(null); + vi.mocked(prisma.$queryRaw).mockResolvedValue([ + { operations_bytes: null, snapshot_bytes: null, has_unbackfilled: false }, + ]); const result = await service.calculateStorageUsage(1); @@ -57,14 +62,18 @@ describe('StorageQuotaService', () => { operationsBytes: 0, snapshotBytes: 0, totalBytes: 0, + hasUnbackfilledRows: false, }); }); it('should handle missing snapshot', async () => { - vi.mocked(prisma.$queryRaw).mockResolvedValue([{ total: BigInt(1000) }]); - vi.mocked(prisma.userSyncState.findUnique).mockResolvedValue({ - snapshotData: null, - } as any); + vi.mocked(prisma.$queryRaw).mockResolvedValue([ + { + operations_bytes: BigInt(1000), + snapshot_bytes: BigInt(0), + has_unbackfilled: false, + }, + ]); const result = await service.calculateStorageUsage(1); @@ -72,14 +81,37 @@ describe('StorageQuotaService', () => { operationsBytes: 1000, snapshotBytes: 0, totalBytes: 1000, + hasUnbackfilledRows: false, }); }); - it('should avoid materializing JSON payloads as text', async () => { - vi.mocked(prisma.$queryRaw).mockResolvedValue([{ total: BigInt(1000) }]); - vi.mocked(prisma.userSyncState.findUnique).mockResolvedValue({ - snapshotData: null, - } as any); + it('should include fallback bytes for rows that are still unbackfilled', async () => { + vi.mocked(prisma.$queryRaw).mockResolvedValue([ + { + operations_bytes: BigInt(750), + snapshot_bytes: BigInt(100), + has_unbackfilled: true, + }, + ]); + + const result = await service.calculateStorageUsage(1); + + expect(result).toEqual({ + operationsBytes: 750, + snapshotBytes: 100, + totalBytes: 850, + hasUnbackfilledRows: true, + }); + }); + + it('should use persisted byte counters with a safe unbackfilled fallback', async () => { + vi.mocked(prisma.$queryRaw).mockResolvedValue([ + { + operations_bytes: BigInt(1000), + snapshot_bytes: BigInt(0), + has_unbackfilled: false, + }, + ]); await service.calculateStorageUsage(1); @@ -89,10 +121,27 @@ describe('StorageQuotaService', () => { ]; const query = Array.from(queryParts).join(''); - expect(query).toContain('pg_column_size(payload)'); - expect(query).toContain('pg_column_size(vector_clock)'); - expect(query).not.toContain('payload::text'); - expect(query).not.toContain('vector_clock::text'); + expect(query).toContain('SUM('); + expect(query).toContain('payload_bytes'); + expect(query).toContain('WHEN payload_bytes > 0'); + expect(query).toContain('BOOL_OR(payload_bytes = 0)'); + expect(query).toContain('octet_length(snapshot_data)'); + expect(query).not.toContain('snapshot_data: true'); + expect(query).not.toContain('pg_column_size'); + }); + }); + + describe('assertPayloadBytesBackfillComplete', () => { + it('should resolve when no unbackfilled rows exist', async () => { + vi.mocked(prisma.$queryRaw).mockResolvedValue([{ exists: false }]); + await expect(service.assertPayloadBytesBackfillComplete()).resolves.toBeUndefined(); + }); + + it('should throw when any row has payload_bytes = 0', async () => { + vi.mocked(prisma.$queryRaw).mockResolvedValue([{ exists: true }]); + await expect(service.assertPayloadBytesBackfillComplete()).rejects.toThrow( + /SUPERSYNC_BATCH_UPLOAD is enabled but the operations table still contains rows with payload_bytes = 0/, + ); }); }); @@ -154,10 +203,13 @@ describe('StorageQuotaService', () => { describe('updateStorageUsage', () => { it('should update storage usage from calculated total', async () => { - vi.mocked(prisma.$queryRaw).mockResolvedValue([{ total: BigInt(75000) }]); - vi.mocked(prisma.userSyncState.findUnique).mockResolvedValue({ - snapshotData: Buffer.alloc(25000), - } as any); + vi.mocked(prisma.$queryRaw).mockResolvedValue([ + { + operations_bytes: BigInt(75000), + snapshot_bytes: BigInt(25000), + has_unbackfilled: false, + }, + ]); vi.mocked(prisma.user.update).mockResolvedValue({} as any); await service.updateStorageUsage(1); @@ -168,25 +220,55 @@ describe('StorageQuotaService', () => { }); }); + it('should skip the storage-counter write while unbackfilled rows remain', async () => { + vi.mocked(prisma.$queryRaw).mockResolvedValue([ + { + operations_bytes: BigInt(50000), + snapshot_bytes: BigInt(0), + has_unbackfilled: true, + }, + ]); + vi.mocked(prisma.user.update).mockResolvedValue({} as any); + + // Pre-set the forced-reconcile marker; the skip path must preserve it + // so a post-backfill call self-heals. + service.markNeedsReconcile(1); + await service.updateStorageUsage(1); + + expect(prisma.user.update).not.toHaveBeenCalled(); + expect(service.needsReconcile(1)).toBe(true); + }); + it('should dedupe concurrent reconciles for the same user', async () => { - // Simulate a slow SUM(pg_column_size) scan so concurrent callers + // Simulate a slow exact usage scan so concurrent callers // overlap on the in-flight promise. - let releaseScan: (value: [{ total: bigint }]) => void = () => undefined; + let releaseScan: ( + value: [ + { + operations_bytes: bigint; + snapshot_bytes: bigint; + has_unbackfilled: boolean; + }, + ], + ) => void = () => undefined; vi.mocked(prisma.$queryRaw).mockReturnValueOnce( new Promise((resolve) => { releaseScan = resolve; }) as any, ); - vi.mocked(prisma.userSyncState.findUnique).mockResolvedValue({ - snapshotData: null, - } as any); vi.mocked(prisma.user.update).mockResolvedValue({} as any); const first = service.updateStorageUsage(1); const second = service.updateStorageUsage(1); const third = service.updateStorageUsage(1); - releaseScan([{ total: BigInt(123) }]); + releaseScan([ + { + operations_bytes: BigInt(123), + snapshot_bytes: BigInt(0), + has_unbackfilled: false, + }, + ]); await Promise.all([first, second, third]); // Only one scan + one write should have run for three concurrent calls. @@ -195,10 +277,13 @@ describe('StorageQuotaService', () => { }); it('should re-scan on a subsequent sequential call after the lock clears', async () => { - vi.mocked(prisma.$queryRaw).mockResolvedValue([{ total: BigInt(10) }]); - vi.mocked(prisma.userSyncState.findUnique).mockResolvedValue({ - snapshotData: null, - } as any); + vi.mocked(prisma.$queryRaw).mockResolvedValue([ + { + operations_bytes: BigInt(10), + snapshot_bytes: BigInt(0), + has_unbackfilled: false, + }, + ]); vi.mocked(prisma.user.update).mockResolvedValue({} as any); await service.updateStorageUsage(1); @@ -210,16 +295,19 @@ describe('StorageQuotaService', () => { it('should release the lock when the scan throws', async () => { vi.mocked(prisma.$queryRaw).mockRejectedValueOnce(new Error('db down')); - vi.mocked(prisma.userSyncState.findUnique).mockResolvedValue({ - snapshotData: null, - } as any); vi.mocked(prisma.user.update).mockResolvedValue({} as any); await expect(service.updateStorageUsage(1)).rejects.toThrow('db down'); // Lock must be cleared so the next call retries the scan rather than // returning the rejected promise forever. - vi.mocked(prisma.$queryRaw).mockResolvedValueOnce([{ total: BigInt(0) }]); + vi.mocked(prisma.$queryRaw).mockResolvedValueOnce([ + { + operations_bytes: BigInt(0), + snapshot_bytes: BigInt(0), + has_unbackfilled: false, + }, + ]); await expect(service.updateStorageUsage(1)).resolves.toBeUndefined(); expect(prisma.$queryRaw).toHaveBeenCalledTimes(2); }); @@ -245,10 +333,13 @@ describe('StorageQuotaService', () => { ).inflightReconciles; inflightMap.set(1, neverResolves); - vi.mocked(prisma.$queryRaw).mockResolvedValue([{ total: BigInt(42) }] as any); - vi.mocked(prisma.userSyncState.findUnique).mockResolvedValue({ - snapshotData: null, - } as any); + vi.mocked(prisma.$queryRaw).mockResolvedValue([ + { + operations_bytes: BigInt(42), + snapshot_bytes: BigInt(0), + has_unbackfilled: false, + }, + ] as any); vi.mocked(prisma.user.update).mockResolvedValue({} as any); const insideResult = await Promise.race([ @@ -287,11 +378,14 @@ describe('StorageQuotaService', () => { vi.mocked(prisma.$queryRaw).mockImplementation(async () => { events.push('scan'); - return [{ total: BigInt(123) }]; + return [ + { + operations_bytes: BigInt(123), + snapshot_bytes: BigInt(0), + has_unbackfilled: false, + }, + ]; }); - vi.mocked(prisma.userSyncState.findUnique).mockResolvedValue({ - snapshotData: null, - } as any); vi.mocked(prisma.user.update).mockImplementation(async () => { events.push('write'); return {} as any; @@ -348,10 +442,9 @@ describe('StorageQuotaService', () => { // Marker indicates the cached counter is known stale (e.g. a previous // post-commit increment failed). Quota check must self-heal before // answering, otherwise drift accumulates until daily cleanup. - vi.mocked(prisma.$queryRaw).mockResolvedValue([{ total: BigInt(10_000) }] as any); - vi.mocked(prisma.userSyncState.findUnique).mockResolvedValue({ - snapshotData: null, - } as any); + vi.mocked(prisma.$queryRaw).mockResolvedValue([ + { operations_bytes: BigInt(10_000), snapshot_bytes: BigInt(0) }, + ] as any); vi.mocked(prisma.user.update).mockResolvedValue({} as any); vi.mocked(prisma.user.findUnique).mockResolvedValue({ storageQuotaBytes: BigInt(100_000), diff --git a/packages/super-sync-server/tests/sync-compressed-body.routes.spec.ts b/packages/super-sync-server/tests/sync-compressed-body.routes.spec.ts index f0d55e3530..1e9959cb3e 100644 --- a/packages/super-sync-server/tests/sync-compressed-body.routes.spec.ts +++ b/packages/super-sync-server/tests/sync-compressed-body.routes.spec.ts @@ -64,6 +64,8 @@ vi.mock('../src/db', () => ({ })); import { syncRoutes } from '../src/sync/sync.routes'; +import { SYNC_ERROR_CODES } from '../src/sync/sync.types'; +import { SUPER_SYNC_MAX_OPS_PER_UPLOAD } from '@sp/shared-schema'; const gzipAsync = promisify(zlib.gzip); @@ -187,6 +189,38 @@ describe('Sync compressed body routes', () => { expect(quotaCall[1]).toBeLessThan(payloadSize); }); + it('should reject oversized op batches before schema validation', async () => { + const clientId = 'too-many-ops-client'; + const payload = { + ops: Array.from({ length: SUPER_SYNC_MAX_OPS_PER_UPLOAD + 1 }, (_, i) => ({ + ...createOp(clientId), + entityId: `task-${i}`, + timestamp: Date.now() + i, + })), + clientId, + }; + + const response = await app.inject({ + method: 'POST', + url: '/api/sync/ops', + headers: { + authorization: `Bearer ${authToken}`, + 'content-type': 'application/json', + }, + payload: JSON.stringify(payload), + }); + + expect(response.statusCode).toBe(413); + expect(response.json()).toEqual( + expect.objectContaining({ + errorCode: SYNC_ERROR_CODES.PAYLOAD_TOO_LARGE, + maxOpsPerBatch: SUPER_SYNC_MAX_OPS_PER_UPLOAD, + }), + ); + expect(mocks.syncService.uploadOps).not.toHaveBeenCalled(); + expect(mocks.syncService.checkStorageQuota).not.toHaveBeenCalled(); + }); + it('should fall back to UTF-8 JSON byte size for plain JSON without content-length', async () => { const clientId = 'plain-json-client'; const payload = { @@ -485,7 +519,7 @@ describe('Sync compressed body routes', () => { false, expect.anything(), ); - }); + }, 15000); it('should keep plain JSON snapshots capped at the binary route limit', async () => { const clientId = 'plain-json-large-snapshot-client'; diff --git a/packages/super-sync-server/tests/sync.service.spec.ts b/packages/super-sync-server/tests/sync.service.spec.ts index bdd35aabe6..917ede8c93 100644 --- a/packages/super-sync-server/tests/sync.service.spec.ts +++ b/packages/super-sync-server/tests/sync.service.spec.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { uuidv7 } from 'uuidv7'; import { Prisma } from '@prisma/client'; import { testState, resetTestState } from './sync.service.test-state'; @@ -99,6 +99,7 @@ vi.mock('../src/db', async () => { .filter((op: any) => { if (args.where?.userId !== undefined && args.where.userId !== op.userId) return false; + if (args.where?.id?.in && !args.where.id.in.includes(op.id)) return false; if ( args.where?.serverSeq?.gt !== undefined && op.serverSeq <= args.where.serverSeq.gt @@ -311,6 +312,52 @@ vi.mock('../src/db', async () => { // returning their existing default shape. $queryRaw: vi.fn().mockImplementation(async (strings: any, ...params: any[]) => { const sql = Array.isArray(strings) ? strings.join('') : String(strings); + if (sql.includes('INSERT INTO user_sync_state')) { + const [txUserId, delta] = params as [number, number]; + const existing = state.userSyncStates.get(txUserId); + const lastSeq = (existing?.lastSeq ?? 0) + delta; + state.userSyncStates.set(txUserId, { + ...(existing ?? { userId: txUserId }), + lastSeq, + }); + state.serverSeqCounter = Math.max(state.serverSeqCounter, lastSeq); + return [{ lastSeq }]; + } + if (sql.includes('JOIN (VALUES')) { + const txUserId = params[params.length - 1] as number; + const touchedParams = params.slice(0, -1).flatMap((param: unknown): unknown[] => { + if ( + param && + typeof param === 'object' && + Array.isArray((param as { values?: unknown[] }).values) + ) { + return (param as { values: unknown[] }).values; + } + return [param]; + }); + const touchedPairs = new Set(); + for (let i = 0; i < touchedParams.length; i += 2) { + touchedPairs.add(`${touchedParams[i]}\u0000${touchedParams[i + 1]}`); + } + + const latestByEntity = new Map(); + for (const op of state.operations.values()) { + if (op.userId !== txUserId || !op.entityId) continue; + const key = `${op.entityType}\u0000${op.entityId}`; + if (!touchedPairs.has(key)) continue; + const existing = latestByEntity.get(key); + if (!existing || op.serverSeq > existing.serverSeq) { + latestByEntity.set(key, op); + } + } + + return Array.from(latestByEntity.values()).map((op: any) => ({ + entityType: op.entityType, + entityId: op.entityId, + clientId: op.clientId, + vectorClock: op.vectorClock, + })); + } if (sql.includes('jsonb_each_text(vector_clock)')) { const [txUserId, beforeServerSeq] = params; const aggregate = new Map(); @@ -551,12 +598,29 @@ vi.mock('../src/auth', () => ({ // Import AFTER mocking import { initSyncService, getSyncService, SyncService } from '../src/sync/sync.service'; -import { Operation, DEFAULT_SYNC_CONFIG } from '../src/sync/sync.types'; +import { Operation, DEFAULT_SYNC_CONFIG, SYNC_ERROR_CODES } from '../src/sync/sync.types'; +import { prisma } from '../src/db'; describe('SyncService', () => { const userId = 1; const clientId = 'test-device-1'; + // Factory for the repeated Operation fixture (mirrors createOp in + // sync-fixes.spec.ts). Override only the fields a test cares about. + const makeOp = (overrides: Partial = {}): Operation => ({ + id: uuidv7(), + clientId, + actionType: 'ADD_TASK', + opType: 'CRT', + entityType: 'TASK', + entityId: 'task-1', + payload: { title: 'Test Task' }, + vectorClock: {}, + timestamp: Date.now(), + schemaVersion: 1, + ...overrides, + }); + beforeEach(() => { // Reset all test data stores resetTestState(); @@ -575,21 +639,15 @@ describe('SyncService', () => { initSyncService(); }); + afterEach(() => { + delete process.env.OLD_OPS_CLEANUP_DELETE_BATCH_SIZE; + delete process.env.OLD_OPS_CLEANUP_MAX_DELETED_PER_RUN; + }); + describe('uploadOps', () => { it('should correctly upload operations', async () => { const service = getSyncService(); - const op: Operation = { - id: uuidv7(), - clientId, - actionType: 'ADD_TASK', - opType: 'CRT', - entityType: 'TASK', - entityId: 'task-1', - payload: { title: 'Test Task' }, - vectorClock: {}, - timestamp: Date.now(), - schemaVersion: 1, - }; + const op: Operation = makeOp(); const results = await service.uploadOps(userId, clientId, [op]); @@ -604,6 +662,207 @@ describe('SyncService', () => { it('should handle multiple operations in order', async () => { const service = getSyncService(); const ops: Operation[] = [ + makeOp({ entityId: 'task-1', payload: { title: 'Task 1' } }), + makeOp({ + entityId: 'task-2', + payload: { title: 'Task 2' }, + timestamp: Date.now() + 1, + }), + ]; + + const results = await service.uploadOps(userId, clientId, ops); + + expect(results).toHaveLength(2); + expect(results[0].serverSeq).toBe(1); + expect(results[1].serverSeq).toBe(2); + }); + + it('should batch upload operations behind the rollout flag', async () => { + const service = new SyncService({ batchUpload: true }); + const ops: Operation[] = Array.from({ length: 25 }, (_, index) => + makeOp({ + entityId: `task-${index}`, + payload: { title: `Task ${index}` }, + vectorClock: { [clientId]: index + 1 }, + timestamp: Date.now() + index, + }), + ); + + const results = await service.uploadOps(userId, clientId, ops); + + expect(results).toHaveLength(25); + expect(results.every((result) => result.accepted)).toBe(true); + expect(results.map((result) => result.serverSeq)).toEqual( + Array.from({ length: 25 }, (_, index) => index + 1), + ); + expect(testState.userSyncStates.get(userId)?.lastSeq).toBe(25); + expect(testState.operations.size).toBe(25); + }); + + it('rejects an intra-batch same-id op as DUPLICATE_OPERATION even when its content differs (deliberate divergence from the legacy per-op path, which yields INVALID_OP_ID)', async () => { + // C4: the batch path dedups intra-batch purely by op.id (plan §1a step 2) + // and rejects the later op as DUPLICATE_OPERATION regardless of content. + // The legacy per-op path inserts the first then catches the second at the + // DB and returns INVALID_OP_ID for differing content. Both are terminal + // rejections with no row and no sequence gap, so sync invariants hold; + // the client treats DUPLICATE_OPERATION as a silent success and + // INVALID_OP_ID as a hard rejection. This test pins the chosen batch + // semantics so the divergence stays intentional, not accidental drift. + const service = new SyncService({ batchUpload: true }); + const opId = uuidv7(); + const first = makeOp({ + id: opId, + entityId: 'task-1', + payload: { title: 'Task 1' }, + vectorClock: { [clientId]: 1 }, + }); + const sameIdDifferentContent = makeOp({ + id: opId, + entityId: 'task-2', + payload: { title: 'Task 2' }, + vectorClock: { [clientId]: 2 }, + timestamp: Date.now() + 1, + }); + // Guard the premise: the two ops genuinely differ in content. + expect(sameIdDifferentContent.id).toBe(first.id); + expect(sameIdDifferentContent.payload).not.toEqual(first.payload); + + const results = await service.uploadOps(userId, clientId, [ + first, + sameIdDifferentContent, + ]); + + expect(results[0]).toEqual( + expect.objectContaining({ accepted: true, serverSeq: 1 }), + ); + expect(results[1]).toEqual( + expect.objectContaining({ + accepted: false, + errorCode: SYNC_ERROR_CODES.DUPLICATE_OPERATION, + }), + ); + // No sequence gap: lastSeq advanced by exactly 1, exactly one row. + expect(testState.userSyncStates.get(userId)?.lastSeq).toBe(1); + expect(testState.operations.size).toBe(1); + }); + + it('should reject intra-batch entity conflicts in order', async () => { + const service = new SyncService({ batchUpload: true }); + const ops: Operation[] = [ + { + id: uuidv7(), + clientId, + actionType: 'UPDATE_TASK', + opType: 'UPD', + entityType: 'TASK', + entityId: 'task-1', + payload: { title: 'First' }, + vectorClock: { [clientId]: 1 }, + timestamp: Date.now(), + schemaVersion: 1, + }, + { + id: uuidv7(), + clientId, + actionType: 'UPDATE_TASK', + opType: 'UPD', + entityType: 'TASK', + entityId: 'task-1', + payload: { title: 'Concurrent' }, + vectorClock: { 'other-client': 1 }, + timestamp: Date.now() + 1, + schemaVersion: 1, + }, + ]; + + const results = await service.uploadOps(userId, clientId, ops); + + expect(results[0].accepted).toBe(true); + expect(results[1]).toEqual( + expect.objectContaining({ + accepted: false, + errorCode: SYNC_ERROR_CODES.CONFLICT_CONCURRENT, + }), + ); + expect(testState.userSyncStates.get(userId)?.lastSeq).toBe(1); + expect(testState.operations.size).toBe(1); + }); + + it('should use entityIds when prefetching batch conflicts', async () => { + const service = new SyncService({ batchUpload: true }); + testState.userSyncStates.set(userId, { userId, lastSeq: 1 }); + testState.operations.set('existing-op', { + id: 'existing-op', + userId, + clientId: 'other-client', + serverSeq: 1, + actionType: 'UPDATE_TASK', + opType: 'UPD', + entityType: 'TASK', + entityId: 'task-b', + payload: { title: 'Existing' }, + payloadBytes: BigInt(10), + vectorClock: { 'other-client': 1 }, + schemaVersion: 1, + clientTimestamp: BigInt(Date.now() - 1000), + receivedAt: BigInt(Date.now() - 1000), + isPayloadEncrypted: false, + syncImportReason: null, + }); + + const results = await service.uploadOps(userId, clientId, [ + { + id: uuidv7(), + clientId, + actionType: 'BATCH_UPDATE_TASKS', + opType: 'BATCH', + entityType: 'TASK', + entityId: 'task-a', + entityIds: ['task-a', 'task-b', 'task-c'], + payload: { entities: { 'task-a': { title: 'A' } } }, + vectorClock: { [clientId]: 1 }, + timestamp: Date.now(), + schemaVersion: 1, + }, + ]); + + expect(results[0]).toEqual( + expect.objectContaining({ + accepted: false, + errorCode: SYNC_ERROR_CODES.CONFLICT_CONCURRENT, + }), + ); + expect(testState.userSyncStates.get(userId)?.lastSeq).toBe(1); + }); + + it('should chunk large batch entity prefetch queries', async () => { + const service = new SyncService({ batchUpload: true }); + const entityPairs = Array.from({ length: 250 }, (_, index) => ({ + entityType: 'TASK', + entityId: `task-${index}`, + })); + const tx = { + $queryRaw: vi.fn().mockResolvedValue([]), + }; + + await ( + service as unknown as { + prefetchLatestEntityOpsForBatch: ( + userId: number, + entityPairs: { entityType: string; entityId: string }[], + tx: { $queryRaw: (...args: unknown[]) => Promise }, + ) => Promise>; + } + ).prefetchLatestEntityOpsForBatch(userId, entityPairs, tx); + + expect(tx.$queryRaw).toHaveBeenCalledTimes(3); + }); + + it('should create user sync state for first-time batch uploads', async () => { + const service = new SyncService({ batchUpload: true }); + expect(testState.userSyncStates.get(userId)).toBeUndefined(); + + const results = await service.uploadOps(userId, clientId, [ { id: uuidv7(), clientId, @@ -612,29 +871,165 @@ describe('SyncService', () => { entityType: 'TASK', entityId: 'task-1', payload: { title: 'Task 1' }, - vectorClock: {}, + vectorClock: { [clientId]: 1 }, + timestamp: Date.now(), + schemaVersion: 1, + }, + ]); + + expect(results[0]).toEqual( + expect.objectContaining({ accepted: true, serverSeq: 1 }), + ); + expect(testState.userSyncStates.get(userId)?.lastSeq).toBe(1); + }); + + it('should update device last seen for all-rejected batch uploads', async () => { + const service = new SyncService({ batchUpload: true }); + + const results = await service.uploadOps(userId, clientId, [ + { + id: uuidv7(), + clientId, + actionType: 'ADD_TASK', + opType: 'INVALID' as Operation['opType'], + entityType: 'TASK', + entityId: 'task-1', + payload: { title: 'Task 1' }, + vectorClock: { [clientId]: 1 }, + timestamp: Date.now(), + schemaVersion: 1, + }, + ]); + + expect(results[0]).toEqual( + expect.objectContaining({ + accepted: false, + errorCode: SYNC_ERROR_CODES.INVALID_OP_TYPE, + }), + ); + expect(testState.operations.size).toBe(0); + expect(testState.userSyncStates.get(userId)).toBeUndefined(); + expect(testState.syncDevices.get(`${userId}:${clientId}`)).toEqual( + expect.objectContaining({ userId, clientId }), + ); + }); + + it('runs the prior-vector-clock aggregate exactly once per batch and last full-state op wins, even with multiple full-state ops', async () => { + // NEW-2: a single full-state op cannot prove the "once" invariant — it + // would pass even if the expensive _aggregatePriorVectorClock ran per + // full-state op. Use TWO full-state ops so the test catches a regression + // that reverts to per-op aggregation (the exact perf footgun the batch + // path optimizes away), and assert last-write-wins over N full-state ops. + const service = new SyncService({ batchUpload: true }); + const aggregateSpy = vi.spyOn( + service as unknown as { + _aggregatePriorVectorClock: (...args: unknown[]) => Promise; + }, + '_aggregatePriorVectorClock', + ); + + const results = await service.uploadOps(userId, clientId, [ + { + id: uuidv7(), + clientId, + actionType: '[SP_ALL] Load(import) all data', + opType: 'SYNC_IMPORT', + entityType: 'ALL', + payload: { TASK: {} }, + vectorClock: { [clientId]: 7 }, timestamp: Date.now(), schemaVersion: 1, }, { id: uuidv7(), clientId, - actionType: 'ADD_TASK', - opType: 'CRT', - entityType: 'TASK', - entityId: 'task-2', - payload: { title: 'Task 2' }, - vectorClock: {}, + actionType: '[SP_ALL] Load(import) all data', + opType: 'SYNC_IMPORT', + entityType: 'ALL', + payload: { TASK: {} }, + vectorClock: { [clientId]: 9 }, timestamp: Date.now() + 1, schemaVersion: 1, }, - ]; + makeOp({ + entityId: 'task-after', + payload: { title: 'After' }, + vectorClock: { [clientId]: 10 }, + timestamp: Date.now() + 2, + }), + ]); - const results = await service.uploadOps(userId, clientId, ops); + expect(results.map((result) => result.accepted)).toEqual([true, true, true]); + // Aggregate is computed once for the batch, not once per full-state op. + expect(aggregateSpy).toHaveBeenCalledTimes(1); + // Last full-state op wins: marker points at the SECOND import (seq 2), + // not the first, with its (merged) clock. + expect(testState.userSyncStates.get(userId)).toEqual( + expect.objectContaining({ + lastSeq: 3, + latestFullStateSeq: 2, + latestFullStateVectorClock: { [clientId]: 9 }, + }), + ); + aggregateSpy.mockRestore(); + }); - expect(results).toHaveLength(2); - expect(results[0].serverSeq).toBe(1); - expect(results[1].serverSeq).toBe(2); + it('should classify batch createMany P2002 stale-prefetch races as retryable', async () => { + const service = new SyncService({ batchUpload: true }); + const p2002 = new Prisma.PrismaClientKnownRequestError( + 'Unique constraint failed on the constraint: operations_pkey', + { + code: 'P2002', + clientVersion: '5.0.0', + meta: { target: 'operations_pkey' }, + }, + ); + const tx = { + operation: { + deleteMany: vi.fn(), + findMany: vi.fn().mockResolvedValue([]), + createMany: vi.fn().mockRejectedValue(p2002), + }, + userSyncState: { + updateMany: vi.fn(), + }, + syncDevice: { + deleteMany: vi.fn(), + upsert: vi.fn(), + }, + user: { + update: vi.fn(), + }, + $queryRaw: vi.fn().mockResolvedValue([{ lastSeq: 1 }]), + $executeRaw: vi.fn(), + }; + + vi.mocked(prisma.$transaction).mockImplementationOnce(async (callback) => + callback(tx as unknown as Prisma.TransactionClient), + ); + + const results = await service.uploadOps(userId, clientId, [ + { + id: uuidv7(), + clientId, + actionType: '[SP_ALL] Load(import) all data', + opType: 'SYNC_IMPORT', + entityType: 'ALL', + payload: { TASK: {} }, + vectorClock: { [clientId]: 1 }, + timestamp: Date.now(), + schemaVersion: 1, + }, + ]); + + expect(results).toEqual([ + expect.objectContaining({ + accepted: false, + errorCode: SYNC_ERROR_CODES.INTERNAL_ERROR, + error: 'Concurrent transaction conflict - please retry', + }), + ]); + expect(tx.syncDevice.upsert).not.toHaveBeenCalled(); }); it('should reject duplicate operation IDs (idempotency)', async () => { @@ -1638,7 +2033,9 @@ describe('SyncService', () => { it('drains a single user up to the per-run budget', async () => { const service = getSyncService(); - const totalOps = 25_005; + process.env.OLD_OPS_CLEANUP_DELETE_BATCH_SIZE = '50'; + process.env.OLD_OPS_CLEANUP_MAX_DELETED_PER_RUN = '250'; + const totalOps = 255; const cutoffTime = Date.now() - 50 * 24 * 60 * 60 * 1000; for (let i = 1; i <= totalOps; i++) { @@ -1670,17 +2067,21 @@ describe('SyncService', () => { const { totalDeleted, affectedUserIds } = await service.deleteOldSyncedOpsForAllUsers(cutoffTime); + delete process.env.OLD_OPS_CLEANUP_DELETE_BATCH_SIZE; + delete process.env.OLD_OPS_CLEANUP_MAX_DELETED_PER_RUN; - // 25k per-run budget, 5k per-batch. The inner drain loop keeps + // Per-run budget is larger than one delete batch. The inner drain loop keeps // deleting until the budget hits zero, not just one batch. - expect(totalDeleted).toBe(25_000); + expect(totalDeleted).toBe(250); expect(affectedUserIds).toEqual([userId]); expect(testState.operations.size).toBe(5); }); it('marks user for reconcile when a later batch throws mid-loop', async () => { const service = getSyncService(); - const totalOps = 12_000; + process.env.OLD_OPS_CLEANUP_DELETE_BATCH_SIZE = '50'; + process.env.OLD_OPS_CLEANUP_MAX_DELETED_PER_RUN = '250'; + const totalOps = 120; const cutoffTime = Date.now() - 50 * 24 * 60 * 60 * 1000; for (let i = 1; i <= totalOps; i++) { @@ -1731,15 +2132,19 @@ describe('SyncService', () => { await expect(service.deleteOldSyncedOpsForAllUsers(cutoffTime)).rejects.toThrow( 'simulated transient DB failure', ); + delete process.env.OLD_OPS_CLEANUP_DELETE_BATCH_SIZE; + delete process.env.OLD_OPS_CLEANUP_MAX_DELETED_PER_RUN; - // First batch committed 5k deletes; the user must still be marked so + // First batch committed deletes; the user must still be marked so // the next request reconciles the now-stale-high counter. expect(serviceWithPrivates.storageQuotaService.needsReconcile(userId)).toBe(true); - expect(testState.operations.size).toBe(totalOps - 5_000); + expect(testState.operations.size).toBe(totalOps - 50); }); it('shares the per-run budget across users; tail users wait for next pass', async () => { const service = getSyncService(); + process.env.OLD_OPS_CLEANUP_DELETE_BATCH_SIZE = '50'; + process.env.OLD_OPS_CLEANUP_MAX_DELETED_PER_RUN = '250'; const user2Id = 2; const cutoffTime = Date.now() - 50 * 24 * 60 * 60 * 1000; @@ -1750,8 +2155,8 @@ describe('SyncService', () => { storageUsedBytes: BigInt(0), }); - // Each user has 20k stale ops — more than the 25k per-run budget combined. - const opsPerUser = 20_000; + // Each user has 200 stale ops — more than the 250 per-run budget combined. + const opsPerUser = 200; for (const uid of [userId, user2Id]) { for (let i = 1; i <= opsPerUser; i++) { testState.operations.set(`u${uid}-op-${i}`, { @@ -1791,11 +2196,13 @@ describe('SyncService', () => { const { totalDeleted, affectedUserIds } = await service.deleteOldSyncedOpsForAllUsers(cutoffTime); + delete process.env.OLD_OPS_CLEANUP_DELETE_BATCH_SIZE; + delete process.env.OLD_OPS_CLEANUP_MAX_DELETED_PER_RUN; - // user1 drains fully (20k), user2 only gets the remaining 5k of budget. - expect(totalDeleted).toBe(25_000); + // user1 drains fully, user2 only gets the remaining budget. + expect(totalDeleted).toBe(250); expect(affectedUserIds).toEqual([userId, user2Id]); - expect(testState.operations.size).toBe(15_000); + expect(testState.operations.size).toBe(150); }); it('should delete old operations from all users', async () => {