Files
readest/docker/volumes/db/migrations/016_add_books_synced_at.sql
T
Huang Xin 9155ae627c feat(sync): decouple the incremental-pull cursor from updated_at via server synced_at (#4678) (#4712)
* feat(sync): decouple the incremental-pull cursor from updated_at (#4678)

`books.updated_at` was overloaded as both the incremental-pull cursor
(`GET /api/sync?since=…` filters `updated_at > since`, devices keep one
global `max(updated_at)` watermark) and the library "date read" sort key.
A server-resolved reading-status merge had to be written with a timestamp
greater than every peer's global cursor to propagate, which forced
`updated_at = now()` and reordered the date-read library by sync-processing
time (the #4677 symptom).

Introduce a server-assigned `synced_at` column on `books`, stamped by a
`BEFORE INSERT OR UPDATE` trigger on every write, used only as the pull
cursor. `updated_at` stays pure client event time used only for sorting.

- Migration 016 + baseline schema: add `synced_at` (NOT NULL DEFAULT now()),
  index `(user_id, synced_at)`, trigger `set_books_synced_at`. Backfill
  `synced_at = updated_at` before creating the trigger so existing devices'
  cursors hand over without a re-sync storm.
- GET: books filters/orders on `synced_at > since` (a delete bumps synced_at,
  so the deleted_at clause is dropped); configs/notes stay on updated_at.
- POST: extract `buildStatusPropagationRow` and drop the `updated_at = now()`
  bump — the trigger advances synced_at so peers re-pull the status change
  while updated_at (the sort key) stays put.
- Client `computeMaxTimestamp` keys on synced_at, falling back to
  updated_at/deleted_at for pre-migration servers and configs/notes.

Backward-compatible: `synced_at >= updated_at` always, 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 idempotent
re-pull of rare server-merged rows). The koplugin's shared pull/push cursor
is left untouched; a proper split is a follow-up.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(sync): make the books synced_at backfill safe for large live tables (#4678)

The single `UPDATE … WHERE synced_at IS NULL` deadlocked on a 3.8M-row
production `books` table: it rewrites every row in one transaction while the
live /api/sync push path upserts books rows, and the two lock rows in opposite
orders. `ALTER COLUMN … SET NOT NULL` (full-table ACCESS EXCLUSIVE scan) and a
plain CREATE INDEX (write-blocking SHARE lock) compounded it.

Rework migration 016 as an online migration (run via psql, not in a wrapping
transaction):
- backfill in small autocommitted batches via a procedure, using
  FOR UPDATE SKIP LOCKED so it never waits on an app-locked row;
- CREATE INDEX CONCURRENTLY instead of a blocking build;
- install the trigger last (so it can't clobber the updated_at backfill);
- drop the hard SET NOT NULL (the default + trigger + backfill keep the column
  populated and the client falls back to updated_at); a NOT VALID CHECK +
  VALIDATE alternative is included, commented, for operators who want it.

The baseline schema.sql (fresh, empty installs) keeps the simple inline
NOT NULL DEFAULT now() + trigger.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-21 21:24:01 +02:00

119 lines
5.8 KiB
PL/PgSQL

-- Migration 016: Add a server-assigned `synced_at` cursor to books (issue #4678)
--
-- `books.updated_at` was overloaded as two things with conflicting needs:
-- 1. the incremental-pull cursor (GET /api/sync?since=… filters updated_at >
-- since, and each device keeps a single global max(updated_at) watermark);
-- 2. the library "date read" sort key (wants the client event time).
--
-- A server-resolved merge (e.g. the reading_status field-level LWW in #4634)
-- has to be written with a timestamp greater than every peer's global cursor to
-- propagate, which forced updated_at = now() and reordered the date-read library
-- by sync-processing time (the #4677 symptom).
--
-- Decouple the two: `synced_at` is a monotonic, server-stamped cursor used ONLY
-- by the incremental pull, while `updated_at` stays pure client event time used
-- ONLY for sorting. A BEFORE INSERT/UPDATE trigger forces synced_at = now() on
-- every server write (clients never send it), so a status merge propagates by
-- bumping synced_at without touching updated_at.
--
-- Backfill synced_at = updated_at so existing devices' updated_at-based cursors
-- hand over seamlessly: `synced_at > since` returns the same rows as before
-- (synced_at == updated_at) plus, going forward, server-resolved merges.
--
-- ┌───────────────────────────────────────────────────────────────────────────┐
-- │ RUN ONLINE, NOT INSIDE A TRANSACTION. │
-- │ │
-- │ Apply with psql against a live, large `books` table (millions of rows): │
-- │ psql "$DATABASE_URL" -v ON_ERROR_STOP=1 -f 016_add_books_synced_at.sql │
-- │ │
-- │ Do NOT paste it into a wrapping BEGIN/COMMIT or the Supabase dashboard SQL │
-- │ editor: it uses CREATE INDEX CONCURRENTLY and a CALL into a procedure that │
-- │ COMMITs each backfill batch — both are rejected inside a transaction. │
-- │ │
-- │ A single bulk `UPDATE … WHERE synced_at IS NULL` deadlocks against the │
-- │ live /api/sync upserts (both lock books rows, in opposite orders). The │
-- │ backfill below instead walks the table in small autocommitted batches and │
-- │ uses FOR UPDATE SKIP LOCKED so it never waits on a row the app holds. │
-- └───────────────────────────────────────────────────────────────────────────┘
-- 1. Add the column (nullable, no default → metadata-only, instant) and set the
-- default up front so rows INSERTed during the backfill already get now().
ALTER TABLE public.books
ADD COLUMN IF NOT EXISTS synced_at timestamp with time zone NULL;
ALTER TABLE public.books
ALTER COLUMN synced_at SET DEFAULT now();
-- 2. Backfill in small, individually-committed batches. SKIP LOCKED steps over
-- rows currently locked by a concurrent push (they fall to a later pass), so
-- the backfill never deadlocks with live traffic. The trigger is installed
-- only AFTER this completes, so it can't clobber the updated_at backfill.
CREATE OR REPLACE PROCEDURE public.backfill_books_synced_at(batch_size int DEFAULT 10000)
LANGUAGE plpgsql
AS $$
DECLARE
n int;
BEGIN
LOOP
WITH todo AS (
SELECT ctid
FROM public.books
WHERE synced_at IS NULL
LIMIT batch_size
FOR UPDATE SKIP LOCKED
)
UPDATE public.books b
SET synced_at = COALESCE(b.updated_at, b.created_at, now())
FROM todo
WHERE b.ctid = todo.ctid;
GET DIAGNOSTICS n = ROW_COUNT;
COMMIT;
IF n = 0 THEN
-- A pass updated nothing: either we're done, or the only rows left are
-- momentarily app-locked. Stop when truly none remain, else briefly wait.
EXIT WHEN NOT EXISTS (SELECT 1 FROM public.books WHERE synced_at IS NULL);
PERFORM pg_sleep(0.1);
END IF;
END LOOP;
END;
$$;
CALL public.backfill_books_synced_at(10000);
DROP PROCEDURE public.backfill_books_synced_at(int);
-- 3. Index the cursor without blocking writes (CONCURRENTLY → no SHARE lock).
CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_books_user_synced
ON public.books (user_id, synced_at);
-- 4. Install the trigger LAST, so from here every write is server-stamped.
CREATE OR REPLACE FUNCTION public.set_books_synced_at()
RETURNS trigger
LANGUAGE plpgsql
AS $$
BEGIN
-- Server-authoritative: ignore any client-supplied value and stamp the
-- transaction time. transaction_timestamp() (= now()) is stable within a
-- batch upsert, which is fine — a batch is a single pull delta.
NEW.synced_at := now();
RETURN NEW;
END;
$$;
DROP TRIGGER IF EXISTS books_set_synced_at ON public.books;
CREATE TRIGGER books_set_synced_at
BEFORE INSERT OR UPDATE ON public.books
FOR EACH ROW
EXECUTE FUNCTION public.set_books_synced_at();
-- 5. (Optional) Enforce NOT NULL without the full-table ACCESS EXCLUSIVE scan
-- that `ALTER COLUMN … SET NOT NULL` takes. A NOT VALID check skips existing
-- rows; VALIDATE then scans under a lighter SHARE UPDATE EXCLUSIVE lock that
-- still allows concurrent reads and writes. Safe to omit — the default, the
-- trigger and the backfill already keep the column populated, and the client
-- falls back to updated_at when synced_at is absent.
-- ALTER TABLE public.books
-- ADD CONSTRAINT books_synced_at_not_null CHECK (synced_at IS NOT NULL) NOT VALID;
-- ALTER TABLE public.books
-- VALIDATE CONSTRAINT books_synced_at_not_null;