5.4 KiB
name, description, metadata
| name | description | metadata | ||||||
|---|---|---|---|---|---|---|---|---|
| sync-synced-at-cursor-4678 | Decouple the incremental-pull cursor from updated_at via a server-stamped synced_at column on books (#4678) |
|
Decouple sync pull cursor from updated_at — server synced_at (#4678, branch feat/sync-synced-at-cursor-4678)
Follow-up cleanup to sync-statusless-book-rebump-4677. books.updated_at was overloaded: (1) incremental-pull cursor (GET /api/sync?since=… filters updated_at>since; device keeps one global max(updated_at)) AND (2) library "date read" sort key. A server-resolved merge (reading_status LWW #4634) had to be written > every peer's global cursor to propagate → forced updated_at=now() → reordered the date-read library by sync time.
Decision (user-chosen): server synced_at, books-only, koplugin untouched. Scoped to books because it's the ONLY table with the overload — the only server-side updated_at=now() bumps are the books status-merge propagation (sync.ts) + progress piggyback writes books rows; configs/notes are never server-bumped and aren't a sort key; stats.updated_at is already server-assigned. (Confirmed both scope decisions via AskUserQuestion.)
Implementation:
- Migration
docker/volumes/db/migrations/016_add_books_synced_at.sql+ baselinedocker/volumes/db/init/schema.sql: addsynced_at timestamptz NOT NULL DEFAULT now(); backfill= COALESCE(updated_at,created_at,now())BEFORE creating the trigger (else trigger clobbers backfill to now() → full re-sync storm); index(user_id, synced_at);BEFORE INSERT OR UPDATEtriggerset_books_synced_at()forcesNEW.synced_at = now()(server-authoritative; clients never send it). First trigger on these tables — justified because synced_at must be server-stamped on EVERY write path (insert / client-wins update / status-merge / piggyback) and a trigger is DRY + unforgeable. GET(src/pages/api/sync.tsqueryTables): books filters.gt('synced_at', since)+ orders by synced_at; drop thedeleted_atclause for books (a delete bumps synced_at). configs/notes unchanged (updated_at+deleted_at).POSTstatus-merge: extracted purebuildStatusPropagationRow(serverBook, status)— grafts fresher status, removedupdated_at: now(). Trigger advances synced_at so peers re-pull; updated_at stays = event time → no reorder. Progress piggyback unchanged (trigger now reliably propagates it too).- Client
src/hooks/useSync.ts: exportedcomputeMaxTimestamp, keys onsynced_atfirst, falls back tomax(updated_at,deleted_at)when absent. Addedsynced_at?: string|nulltoBookDataRecord(src/types/book.ts);Book.syncedAtalready existed (unused, left unpopulated).
Why backward-compatible (key insight): synced_at >= updated_at always (backfill makes old rows equal, trigger makes new rows now() ≥ client event time), so synced_at>since is a strict SUPERSET of updated_at>since. Old web clients AND the koplugin keep working with no data loss — at worst a redundant re-pull of rare server-merged rows (idempotent upsert). koplugin left unchanged: its last_books_pulled_at is SHARED between pull-cursor (vs server) and push-delta detection (getChangedBooks vs local updated_at) — retargeting it to synced_at would need a risky pull/push cursor split (follow-up). koplugin advances books cursor from row updated_at/deleted_at (syncbooks.lua pullBooks), notes from os.time()*1000 — both fine under the superset filter.
Tests (test-first, pure units; no DB in unit tests): __tests__/pages/api/sync-synced-at-cursor.test.ts (buildStatusPropagationRow keeps updated_at), __tests__/hooks/useSync-cursor.test.ts (computeMaxTimestamp prefers synced_at, falls back). Full suite + lint + format:check green. No Lua/Rust changed. PR #4712.
Online-migration gotcha (prod books = 3.8M rows): the naive UPDATE … WHERE synced_at IS NULL (one txn, all rows) DEADLOCKED against live /api/sync upserts (40P01, both lock books rows in opposite orders); ALTER COLUMN SET NOT NULL (full-table ACCESS EXCLUSIVE scan) + plain CREATE INDEX (write-blocking SHARE) compound it. Rewrote 016 as an ONLINE migration — run via psql, NOT in a wrapping txn / NOT the Supabase dashboard editor (uses CREATE INDEX CONCURRENTLY + a CALL proc that COMMITs per batch, both rejected inside a txn): (1) ADD COLUMN nullable + SET DEFAULT now() up front (inserts during backfill get now()); (2) batched backfill in a PROCEDURE — WITH todo AS (SELECT ctid FROM books WHERE synced_at IS NULL LIMIT 10000 FOR UPDATE SKIP LOCKED) UPDATE … FROM todo, COMMIT each batch, EXIT when no NULLs remain — SKIP LOCKED never waits on an app-locked row so no deadlock; (3) CREATE INDEX CONCURRENTLY; (4) trigger LAST (else it clobbers the backfill to now()); (5) hard NOT NULL dropped (default+trigger+backfill keep it populated, client falls back to updated_at) — optional ADD CONSTRAINT … CHECK (synced_at IS NOT NULL) NOT VALID then VALIDATE (lighter SHARE UPDATE EXCLUSIVE, no full AccessExclusive scan). Note now() is STABLE not VOLATILE → ADD COLUMN … DEFAULT now() is fast metadata-only (one value for all existing rows) but that = migration-time-now ≠ updated_at, which would force a full re-sync storm — hence the explicit updated_at backfill. COMMIT is allowed in a PROCEDURE-via-CALL but NOT in a DO block nor inside a BEGIN…EXCEPTION sub-block.