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 () => {