diff --git a/.serena/memories/backend/storage.md b/.serena/memories/backend/storage.md index 2cbe71126f..f2962b09a2 100644 --- a/.serena/memories/backend/storage.md +++ b/.serena/memories/backend/storage.md @@ -108,13 +108,40 @@ Since `put-object!` uses backend-specific operations (`impl/resolve-backend` + ` ## File Data -- `file-data-backend` accepts `legacy-db`, `db`, or `storage`. -- `legacy-db` stores main data in `file.data` and snapshots in `file_change.data`. +- `file-data-backend` accepts `db` or `storage`. - `db` stores encoded data in `file_data.data`. - `storage` stores encoded data in storage subsystem with `file-data` bucket and keeps `data` nil in `file_data` table. - The `file_data.metadata.storage-ref-id` value points to the storage object. - `fdata/upsert!` touches a storage object from incoming metadata before it stores the new row. - File snapshots use `file_data` for snapshot data and `file_change` for snapshot metadata. +- The `legacy-db` backend is gone: the legacy data was already cleaned up, and + it was the only thing that kept the `data` column of `file_change` alive. + +## File Change + +- `file_change` is partitioned by `HASH (file_id)` in 16 partitions, with the + same bounds as `file_data`, so a join between them is resolved partition by + partition (`enable_partitionwise_join` is set on every pooled connection). +- Its primary key is `(file_id, id)`: a partitioned table needs the partition + key in the primary key, so every access filters by `file_id`. +- It has a `kind` column: `1` is a snapshot (it has a label and its data lives + in `file_data`) and `2` is a revision (it has no label and it holds the + changes). The values are in `app.features.file-changes`. +- `kind` is a deliberate reserve, not a leftover: today no query filters by + it, the queries still tell a snapshot from a revision by its label + (`label IS NOT NULL`), and `kind` takes over when they are rewritten. No + constraint ties `kind` and `label` together on purpose: the day `kind` is + the source of truth, that constraint would have to go. +- A revision carries only the delta of a save (the changes). The features of + the file are not repeated on it: nobody read them from there. +- A snapshot carries the features and the migrations of the file. +- It has no `data`, `data_ref_id` or `data_backend` columns anymore. +- It has no index for the system snapshots. The old one filtered them by + `data IS NOT NULL` (the column is gone) and the new one could only filter + them by `deleted_at IS NULL`, but the system snapshots always carry a + deletion date in the future, so it would never match a row. The query that + would use it filters that date with an "or", which no partial index can + serve. ## Metrics diff --git a/.serena/memories/backend/subtleties.md b/.serena/memories/backend/subtleties.md index e778f31f8e..26456db271 100644 --- a/.serena/memories/backend/subtleties.md +++ b/.serena/memories/backend/subtleties.md @@ -77,13 +77,13 @@ ## File data persistence -- File data backends are `legacy-db`, `db`, and `storage`. The storage backend keeps encoded file data in storage bucket `file-data`; the DB row stores metadata with `storage-ref-id` and nil data. +- File data backends are `db` and `storage`. The storage backend keeps encoded file data in storage bucket `file-data`; the DB row stores metadata with `storage-ref-id` and nil data. The `legacy-db` backend is gone: the config only accepts `db` and `storage`, and the backend refuses to start with any other value. - `fdata/upsert!` touches any storage object referenced by incoming metadata before storing the new row/blob. - Pointer-map fragments are persisted separately as type `fragment`, and only modified pointer maps are written. - `fdata/realize` combines pointer realization and object-map realization. Use it before operations that need complete in-memory file data instead of pointer placeholders. - A `file_data` row is one of three classes, told apart by `type`: `main` is the file's current state and carries `id = file_id`; `snapshot` is a point-in-time copy whose `id` is the one its `file_change` row shares; `fragment` is a piece of the pointer-map, referenced from the file's data. - Their lifecycles differ: `main` and `snapshot` live with the file (dated when it is deleted, undated when it is restored), while a `fragment` carries no date and is collected by `file_gc` once the file stops referencing it. Neither is a session buffer, so do not read a `fragment` as an ephemeral revision. -- File snapshots (versions) keep their metadata row in `file_change` and their payload in `file_data` (`type = 'snapshot'`). With the `db` backend `file_change.data` stays NULL: that column is only written by the `legacy-db` backend, plus the rows written before the 2025-07 file storage refactor. +- File snapshots (versions) keep their metadata row in `file_change` and their payload in `file_data` (`type = 'snapshot'`). The `data` column of `file_change` does not exist anymore: it was only written by the `legacy-db` backend, plus the rows written before the 2025-07 file storage refactor. - Tell a snapshot apart by `label IS NOT NULL` (with `created_by` / `deleted_at` as needed), never by `file_change.data IS NOT NULL`: the label holds with every file data backend, and the payload column stopped meaning "this is a snapshot" when the payload moved out. Queries that identify versions must use the label. - The version quota counts of `app.rpc.quotes` (`sql:get-snapshots-per-file`, `sql:get-snapshots-per-team`) are served by the partial index `file_change__user_snapshots__idx`, whose predicate mirrors their where clause; changing one without the other drops the index only scan and the count walks the whole table again. - Revisions live only in `file_change` and have no `file_data` counterpart. They are born already scheduled for deletion one hour later (`app.rpc.commands.files-update`) and the snapshot paths cannot reach one, because they resolve their row through a `label IS NOT NULL` filter. diff --git a/.serena/memories/clojure/idioms.md b/.serena/memories/clojure/idioms.md index cf85848c3a..82e45d14b3 100644 --- a/.serena/memories/clojure/idioms.md +++ b/.serena/memories/clojure/idioms.md @@ -9,4 +9,5 @@ Behaviors confirmed against the language/stdlib — do not re-derive from assump - Valid: `(defn ^:async f)`, `(defn- ^:async f)`, `(^:async fn [] …)` (the latter is what stock `t/async` generates itself). - `^:async` fns never throw synchronously (rejected promises instead); `try/catch/finally` supported; continuations are microtasks, timers and RxJS schedulers are macrotasks (FIFO). - `!` suffix allowed cases (only exceptions to the no-new-`!` rule in `mem:clojure/design-rules`): mutating public `app.db` API (`insert!`, `update!`, `delete!`, `run!`, `tx-run!`, `exec!`, ...) and fns whose contract is to work over transients (stdlib `conj!`, `assoc!`, `dissoc!`, `disj!`, `persistent!` and custom helpers over them); any further case must be registered here with justification before use. +- The threading macros insert the threaded value at DIFFERENT positions: `->` and `some->` put it FIRST (`(-> x (f a))` → `(f x a)`), `->>` and `some->>` put it LAST (`(->> x (f a))` → `(f a x)`). `some->`/`some->>` also short-circuit on nil. The nesting follows the same split: `->`/`some->` nest to the left (`(-> x (f) (g))` → `(g (f x))`), `->>`/`some->>` to the right (`(->> x (f) (g))` → `(f (g x))`). A pipeline that calls a fn whose first argument is a config/context (`app.features.fdata/resolve-file-data` is `(cfg, file)`) must use `->>`/`some->>`; with `->`/`some->` the arguments are swapped and the call silently returns the wrong value (it cost a broken `get-snapshot` in `app.features.file-snapshots`). - `cuerdas.core/index-of` returns `nil` (not `-1`) when the substring is absent, like `clojure.string/index-of`. `(pos? nil)` **throws on the JVM** but evaluates to `false` in CLJS (JS coercion), so `(if (pos? (str/index-of s "\n")) …)` works in the browser and crashes the backend. Guard with `some?` before `pos?` (this was `app.common.exceptions/first-line`). diff --git a/.serena/memories/prod-infra/core.md b/.serena/memories/prod-infra/core.md index 4441f0175c..b224409de6 100644 --- a/.serena/memories/prod-infra/core.md +++ b/.serena/memories/prod-infra/core.md @@ -4,7 +4,7 @@ Backend (`app.config`, `PENPOT_*` env vars) is parameterized; deployments choose ## Services -- **PostgreSQL**: durable store. Profiles, teams, files, sessions, audit, `storage_object` metadata, the `task` queue, `scheduled_task` cron registry, migrations. File-data also lives here when the file-data backend is `legacy-db`/`db`. One shared DB across all backends. +- **PostgreSQL**: durable store. Profiles, teams, files, sessions, audit, `storage_object` metadata, the `task` queue, `scheduled_task` cron registry, migrations. File-data also lives here when the file-data backend is `db`. One shared DB across all backends. - **Redis (Valkey-compatible)**: per-backend message bus and cache. Concrete uses: msgbus Pub/Sub for collaborative-editing broadcasts and team/profile-org notifications fired by RPC handlers (`app.rpc.notifications`, `files_update`, `teams`, `websocket`); file-summary cache gated by `enable-redis-cache`; rate-limit counters; and the dispatcher→runner work hand-off list `penpot.worker.queue::`. `PENPOT_REDIS_URI`. A burst of client-side `RedisCommandTimeoutException` (2s on `DEL`/`SET`, 10s on `BLPOP`) with the exporter healthy is the JVM paused or starved, not Redis: the timeout fires because nothing reads the reply. - **Object storage**: backends `:s3` and `:fs`. S3 in prod; devenv uses RustFS. Holds uploaded media, file-data when the file-data backend is `storage`, exports. Backend-side details (resolve, dedup, bucket set, object lifecycle, and file-data backends): `mem:backend/storage`. - **SMTP mailer**: invitations, password resets, email verification (sent via the `:sendmail` worker task). diff --git a/backend/src/app/binfile/common.clj b/backend/src/app/binfile/common.clj index 2c6ff77afc..8f4102519b 100644 --- a/backend/src/app/binfile/common.clj +++ b/backend/src/app/binfile/common.clj @@ -200,14 +200,13 @@ f.is_shared, f.has_media_trimmed, f.revn, - f.data AS legacy_data, f.ignore_sync_until, f.comment_thread_seqn, f.features, f.version, f.vern, p.team_id, - coalesce(fd.backend, 'legacy-db') AS backend, + coalesce(fd.backend, 'db') AS backend, fd.metadata AS metadata, fd.data AS data FROM file AS f diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index 8939dbfc48..1af6e82d68 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -282,7 +282,7 @@ [:nrepl-host {:optional true} :string] [:nrepl-port {:optional true} ::sm/int] - [:file-data-backend {:optional true} [:enum "db" "legacy-db" "storage"]] + [:file-data-backend {:optional true} [:enum "db" "storage"]] [:media-directory {:optional true} :string] ;; REVIEW [:media-uri {:optional true} :string] diff --git a/backend/src/app/db.clj b/backend/src/app/db.clj index d34911ff32..754026e5c5 100644 --- a/backend/src/app/db.clj +++ b/backend/src/app/db.clj @@ -108,7 +108,10 @@ (def initsql (str "SET statement_timeout = 300000;\n" - "SET idle_in_transaction_session_timeout = 300000;")) + "SET idle_in_transaction_session_timeout = 300000;\n" + ;; lets the planner resolve a join between two partitioned tables + ;; pairwise, when both are partitioned with the same bounds + "SET enable_partitionwise_join = on;")) (defn- create-datasource-config [{:keys [::uri] :as cfg}] diff --git a/backend/src/app/features/fdata.clj b/backend/src/app/features/fdata.clj index 43484a696b..ee1ed177ee 100644 --- a/backend/src/app/features/fdata.clj +++ b/backend/src/app/features/fdata.clj @@ -83,17 +83,11 @@ ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (defmulti resolve-file-data - (fn [_cfg file] (get file :backend "legacy-db"))) - -(defmethod resolve-file-data "legacy-db" - [_cfg {:keys [legacy-data] :as file}] - (-> file - (assoc :data legacy-data) - (dissoc :legacy-data))) + (fn [_cfg file] (get file :backend "db"))) (defmethod resolve-file-data "db" [_cfg file] - (dissoc file :legacy-data)) + file) (defmethod resolve-file-data "storage" [cfg {:keys [metadata] :as file}] @@ -101,9 +95,7 @@ ref-id (:storage-ref-id metadata) data (->> (sto/get-object storage ref-id) (sto/get-object-bytes storage))] - (-> file - (assoc :data data) - (dissoc :legacy-data)))) + (assoc file :data data))) (defn decode-file-data [_cfg {:keys [data] :as file}] @@ -147,7 +139,7 @@ data]))) (defn- handle-persistence - [cfg {:keys [type backend id file-id data] :as params}] + [cfg {:keys [backend id file-id data] :as params}] (cond (= backend "storage") @@ -179,36 +171,6 @@ params (assoc params :metadata metadata)] (upsert-in-database cfg params)) - (= backend "legacy-db") - (cond - (= type "main") - (do - (db/delete! cfg :file-data - {:id id :file-id file-id :type "main"} - {::db/return-keys false}) - (db/update! cfg :file - {:data data} - {:id file-id} - {::db/return-keys false})) - - (= type "snapshot") - (do - (db/delete! cfg :file-data - {:id id :file-id file-id :type "snapshot"} - {::db/return-keys false}) - (db/update! cfg :file-change - {:data data} - {:file-id file-id :id id} - {::db/return-keys false})) - - (= type "fragment") - (upsert-in-database cfg - (-> (dissoc params :metadata) - (assoc :backend "db"))) - - :else - (throw (RuntimeException. "not implemented"))) - :else (throw (IllegalArgumentException. (str "backend '" backend "' not supported"))))) @@ -234,7 +196,7 @@ [:id ::sm/uuid] [:type [:enum "main" "snapshot" "fragment"]] [:file-id ::sm/uuid] - [:backend {:optional true} [:enum "db" "legacy-db" "storage"]] + [:backend {:optional true} [:enum "db" "storage"]] [:metadata {:optional true} ctf/schema:file-metadata] [:data {:optional true} bytes?] [:created-at {:optional true} ::ct/inst] diff --git a/backend/src/app/features/file_changes.clj b/backend/src/app/features/file_changes.clj new file mode 100644 index 0000000000..d9e9707c99 --- /dev/null +++ b/backend/src/app/features/file_changes.clj @@ -0,0 +1,20 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS SUBSIDIARY SL + +(ns app.features.file-changes + "The kinds of the rows of the `file_change` table. + + A snapshot is the state of a file at a point: it has a label and its + data lives in `file_data`. A revision is the delta of a save: it has + no label and it holds the changes.") + +(def snapshot-kind + "The kind of a snapshot row." + 1) + +(def revision-kind + "The kind of a revision row." + 2) diff --git a/backend/src/app/features/file_snapshots.clj b/backend/src/app/features/file_snapshots.clj index 9be4b1b572..96b74a9ad1 100644 --- a/backend/src/app/features/file_snapshots.clj +++ b/backend/src/app/features/file_snapshots.clj @@ -19,6 +19,7 @@ [app.db :as db] [app.db.sql :as-alias sql] [app.features.fdata :as fdata] + [app.features.file-changes :as fchanges] [app.storage :as sto] [app.util.blob :as blob] [app.worker :as wrk] @@ -38,9 +39,8 @@ c.migrations, c.version, c.file_id, - c.data AS legacy_data, fd.data AS data, - coalesce(fd.backend, 'legacy-db') AS backend, + coalesce(fd.backend, 'db') AS backend, fd.metadata AS metadata FROM file_change AS c LEFT JOIN file_data AS fd ON (fd.file_id = c.file_id @@ -66,36 +66,52 @@ LEFT JOIN file_data AS fd ON (fd.file_id = f.id AND fd.id = f.id) WHERE f.id = ?") +(def ^:private sql:snapshot-columns + "c.id, + c.label, + c.revn, + c.created_at, + c.modified_at, + c.deleted_at, + c.profile_id, + c.created_by, + c.locked_by, + c.features, + c.metadata, + c.migrations, + c.version, + c.file_id") + +(def ^:private sql:snapshot-visible + "AND CASE WHEN c.created_by = 'user' + THEN c.deleted_at IS NULL + WHEN c.created_by = 'system' + THEN c.deleted_at IS NULL OR c.deleted_at >= ?::timestamptz + END") + (def ^:private sql:get-snapshot-without-data (str "WITH snapshots AS (" sql:snapshots ")" - "SELECT c.id, - c.label, - c.revn, - c.created_at, - c.modified_at, - c.deleted_at, - c.profile_id, - c.created_by, - c.locked_by, - c.features, - c.metadata, - c.migrations, - c.version, - c.file_id - FROM snapshots AS c - WHERE c.id = ? - AND CASE WHEN c.created_by = 'user' - THEN c.deleted_at IS NULL - WHEN c.created_by = 'system' - THEN c.deleted_at IS NULL OR c.deleted_at >= ?::timestamptz - END")) + "SELECT " sql:snapshot-columns + " FROM snapshots AS c + WHERE c.file_id = ? + AND c.id = ? " sql:snapshot-visible)) (defn get-minimal-snapshot - [cfg snapshot-id] + "Get the minimal snapshot info (without the data blob). The query filters + by `file_id`, which lets the planner prune the partitions of `file_change` + when it chooses an index scan. Raises `:snapshot-not-found` when there is + no matching snapshot." + [cfg file-id snapshot-id] (let [now (ct/now)] - (-> (db/get-with-sql cfg [sql:get-snapshot-without-data snapshot-id now] - {::db/remove-deleted false}) - (decode-snapshot)))) + (or (-> (db/get-with-sql cfg [sql:get-snapshot-without-data file-id snapshot-id now] + {::db/remove-deleted false + ::db/throw-if-not-exists false}) + (decode-snapshot)) + (ex/raise :type :not-found + :code :snapshot-not-found + :hint "unable to find snapshot with the provided id" + :snapshot-id snapshot-id + :file-id file-id)))) (def ^:private sql:get-snapshot (str sql:snapshots @@ -109,14 +125,16 @@ (defn get-snapshot "Get a fully decoded snapshot for read-only preview or restoration. - Returns the snapshot map with decoded :data field." + Returns the snapshot map with decoded :data field, or nil when the file has + no snapshot with that id." [cfg file-id snapshot-id] (let [now (ct/now)] - (->> (db/get-with-sql cfg [sql:get-snapshot file-id snapshot-id now] - {::db/remove-deleted false}) - (decode-snapshot) - (fdata/resolve-file-data cfg) - (fdata/decode-file-data cfg)))) + (some->> (db/get-with-sql cfg [sql:get-snapshot file-id snapshot-id now] + {::db/remove-deleted false + ::db/throw-if-not-exists false}) + (decode-snapshot) + (fdata/resolve-file-data cfg) + (fdata/decode-file-data cfg)))) (def ^:private sql:get-visible-snapshots (str "WITH " @@ -259,6 +277,7 @@ (-> snapshot (update :features into-array) (update :migrations into-array) + (assoc :kind fchanges/snapshot-kind) (assoc :updated-at created-at) (assoc :profile-id profile-id) (assoc :session-id session-id) @@ -338,7 +357,7 @@ (when-not snapshot (ex/raise :type :not-found :code :snapshot-not-found - :hint "unable to find snapshot with the provided label" + :hint "unable to find snapshot with the provided id" :snapshot-id snapshot-id :file-id file-id)) @@ -413,7 +432,8 @@ :file-id file-id :id id}}) (db/update! cfg :file-change - {:deleted-at deleted-at} + {:deleted-at deleted-at + :updated-at (ct/now)} {:id id :file-id file-id} {::db/return-keys false}) true) @@ -422,19 +442,25 @@ (str sql:snapshots " AND c.file_id = ?")) (defn lock-by! - [conn id profile-id] + [conn file-id id profile-id] + (assert (uuid? file-id) "missing file-id") + (assert (uuid? id) "missing id") (-> (db/update! conn :file-change - {:locked-by profile-id} - {:id id} + {:locked-by profile-id + :updated-at (ct/now)} + {:id id :file-id file-id} {::db/return-keys false}) (db/get-update-count) (pos?))) (defn unlock! - [conn id] + [conn file-id id] + (assert (uuid? file-id) "missing file-id") + (assert (uuid? id) "missing id") (-> (db/update! conn :file-change - {:locked-by nil} - {:id id} + {:locked-by nil + :updated-at (ct/now)} + {:id id :file-id file-id} {::db/return-keys false}) (db/get-update-count) (pos?))) diff --git a/backend/src/app/http/debug.clj b/backend/src/app/http/debug.clj index 6995464290..837ec09eb4 100644 --- a/backend/src/app/http/debug.clj +++ b/backend/src/app/http/debug.clj @@ -138,7 +138,7 @@ "select revn, changes from file_change where file_id=? and revn >= ? and revn <= ? order by revn") (def sql:retrieve-single-change - "select revn, changes, data from file_change where file_id=? and revn = ?") + "select revn, changes from file_change where file_id=? and revn = ?") (defn- download-file-data [cfg {:keys [params ::session/profile-id] :as request}] diff --git a/backend/src/app/migrations.clj b/backend/src/app/migrations.clj index 34751179b7..99e91d5821 100644 --- a/backend/src/app/migrations.clj +++ b/backend/src/app/migrations.clj @@ -526,7 +526,10 @@ :fn (mg/resource "app/migrations/sql/0155-add-idx-to-file-change.sql")} {:name "0156-reset-pg-storage-defaults" - :fn (mg/resource "app/migrations/sql/0156-reset-pg-storage-defaults.sql")}]) + :fn (mg/resource "app/migrations/sql/0156-reset-pg-storage-defaults.sql")} + + {:name "0157-partition-file-change" + :fn (mg/resource "app/migrations/sql/0157-partition-file-change.sql")}]) (defn apply-migrations! [pool name migrations] diff --git a/backend/src/app/migrations/sql/0157-partition-file-change.sql b/backend/src/app/migrations/sql/0157-partition-file-change.sql new file mode 100644 index 0000000000..9787b8cedb --- /dev/null +++ b/backend/src/app/migrations/sql/0157-partition-file-change.sql @@ -0,0 +1,247 @@ +-- Partition `file_change` by `file_id`. +-- +-- The table grows with every file change and its TOAST holds the blobs of +-- the changes, so it is the biggest relation of the database. Partitioning it +-- by `file_id` lets the queries that filter by file prune to a single +-- partition, and lets the join with `file_data` (already partitioned with the +-- same modulus) be resolved partition by partition. +-- +-- The primary key has to include the partition key, so it becomes +-- `(file_id, id)`. Every access by id alone would lose its index, so the +-- access paths of the application filter by `file_id` too. +-- +-- The new table is created with the final schema: the columns that nobody +-- used anymore are gone (`data_ref_id` and `data_backend`, dead since the +-- file storage refactor, and `data`, from the legacy file data backend, +-- which no longer exists), and the new `kind` column tells the snapshots and +-- the revisions apart. +-- +-- The `kind` column is a reserve: the queries still tell a snapshot from a +-- revision by its label (`label IS NOT NULL`), and `kind` takes over when +-- they are rewritten. That is why no constraint ties the two together: the +-- day `kind` is the source of truth, the constraint would have to go. +-- +-- The old table is kept aside as `file_change_legacy`: it is the way back if +-- something goes wrong. It is dropped by hand, days later (or right here, +-- when it is empty: on a new instance there is nothing to go back to). +-- +-- This migration is not meant to run on a big instance: the copy of the +-- blobs (the legacy ones and the rows of the table) happens inside the +-- transaction of the migration. On those instances mark the migration as +-- applied and partition the table by hand, with the procedure described in +-- the runbook of the issue. + +-- 1. The legacy blobs of the snapshots move to `file_data` before the column +-- disappears. On the instances that already did it (the big ones, by +-- hand, with the procedure of the runbook) this copies nothing; on the +-- others it is the safety net, so no content is left behind when the +-- column goes away. The rows without a label are the ephemeral +-- revisions: they are out of scope, same as in the manual migration. +INSERT INTO file_data (file_id, id, created_at, modified_at, deleted_at, + type, backend, metadata, data) +SELECT fc.file_id, fc.id, fc.created_at, fc.updated_at, fc.deleted_at, + 'snapshot', 'db', NULL, fc.data + FROM file_change AS fc + WHERE fc.data IS NOT NULL + AND fc.label IS NOT NULL +ON CONFLICT (file_id, id) DO NOTHING; + +-- The migration does not go on if a snapshot with legacy data is left +-- without its blob: the backend is better down than losing content. +DO $$ +DECLARE + missing bigint; + revisions bigint; +BEGIN + SELECT count(*) INTO missing + FROM file_change AS fc + WHERE fc.data IS NOT NULL + AND fc.label IS NOT NULL + AND NOT EXISTS (SELECT 1 FROM file_data AS fd + WHERE fd.file_id = fc.file_id + AND fd.id = fc.id + AND fd.type = 'snapshot'); + + IF missing > 0 THEN + RAISE EXCEPTION + 'file_change has % snapshots with legacy data that are not in file_data: migrate them before running this migration', + missing; + END IF; + + -- The revisions with legacy data are not copied. Their data stays in the + -- legacy table, which is the way back, but the operator has to know that + -- they are there. + SELECT count(*) INTO revisions + FROM file_change AS fc + WHERE fc.data IS NOT NULL + AND fc.label IS NULL; + + IF revisions > 0 THEN + RAISE NOTICE + 'file_change has % revisions with legacy data: their data is not copied to file_data and stays in the legacy table', + revisions; + END IF; +END $$; + +-- 2. The current table becomes the legacy one. The names of its indexes and +-- constraints live in the schema, not in the table, so they are renamed +-- with a `_legacy` suffix to free the canonical names. +ALTER TABLE file_change RENAME TO file_change_legacy; + +ALTER TABLE file_change_legacy RENAME CONSTRAINT file_change_pkey + TO file_change_pkey_legacy; +ALTER TABLE file_change_legacy RENAME CONSTRAINT file_change_file_id_fkey + TO file_change_file_id_fkey_legacy; +ALTER TABLE file_change_legacy RENAME CONSTRAINT file_change_locked_by_fkey + TO file_change_locked_by_fkey_legacy; +ALTER TABLE file_change_legacy RENAME CONSTRAINT file_change_profile_id_fkey + TO file_change_profile_id_fkey_legacy; + +ALTER INDEX file_change__data_ref_id__idx + RENAME TO file_change__data_ref_id__idx_legacy; +ALTER INDEX file_change__deleted_at__idx + RENAME TO file_change__deleted_at__idx_legacy; +ALTER INDEX file_change__file_id__revn__idx + RENAME TO file_change__file_id__revn__idx_legacy; +ALTER INDEX file_change__locked_by__idx + RENAME TO file_change__locked_by__idx_legacy; +ALTER INDEX file_change__profile_id__idx + RENAME TO file_change__profile_id__idx_legacy; +ALTER INDEX file_change__system_snapshots__idx + RENAME TO file_change__system_snapshots__idx_legacy; + +-- This one comes from another branch, so an instance may not have it. +ALTER INDEX IF EXISTS file_change__user_snapshots__idx + RENAME TO file_change__user_snapshots__idx_legacy; + +-- 3. The new table, partitioned with the same bounds as `file_data`, so both +-- relations can be joined partition by partition. +CREATE TABLE file_change ( + file_id uuid NOT NULL, + id uuid NOT NULL DEFAULT gen_random_uuid(), + created_at timestamptz NOT NULL DEFAULT now(), + session_id uuid, + revn bigint NOT NULL DEFAULT 0, + changes bytea, + profile_id uuid, + features text[], + label text, + version integer, + created_by text NOT NULL DEFAULT 'system', + updated_at timestamptz DEFAULT now(), + deleted_at timestamptz, + migrations text[], + locked_by uuid, + kind smallint NOT NULL, + CONSTRAINT file_change_kind_check CHECK (kind IN (1, 2)) +) PARTITION BY HASH (file_id); + +-- the columns keep the storage of PostgreSQL by default (migration +-- 0156-reset-pg-storage-defaults dropped the explicit EXTERNAL overrides) + +COMMENT ON COLUMN file_change.kind IS + 'The kind of the row: 1 is a snapshot (it has a label and its data lives in file_data) and 2 is a revision (it has no label and it holds the changes).'; +COMMENT ON COLUMN file_change.locked_by IS + 'Profile ID of user who has locked this version. Only the creator can lock/unlock their own versions. Locked versions cannot be deleted by others.'; + +CREATE TABLE file_change_00 PARTITION OF file_change + FOR VALUES WITH (MODULUS 16, REMAINDER 0); +CREATE TABLE file_change_01 PARTITION OF file_change + FOR VALUES WITH (MODULUS 16, REMAINDER 1); +CREATE TABLE file_change_02 PARTITION OF file_change + FOR VALUES WITH (MODULUS 16, REMAINDER 2); +CREATE TABLE file_change_03 PARTITION OF file_change + FOR VALUES WITH (MODULUS 16, REMAINDER 3); +CREATE TABLE file_change_04 PARTITION OF file_change + FOR VALUES WITH (MODULUS 16, REMAINDER 4); +CREATE TABLE file_change_05 PARTITION OF file_change + FOR VALUES WITH (MODULUS 16, REMAINDER 5); +CREATE TABLE file_change_06 PARTITION OF file_change + FOR VALUES WITH (MODULUS 16, REMAINDER 6); +CREATE TABLE file_change_07 PARTITION OF file_change + FOR VALUES WITH (MODULUS 16, REMAINDER 7); +CREATE TABLE file_change_08 PARTITION OF file_change + FOR VALUES WITH (MODULUS 16, REMAINDER 8); +CREATE TABLE file_change_09 PARTITION OF file_change + FOR VALUES WITH (MODULUS 16, REMAINDER 9); +CREATE TABLE file_change_10 PARTITION OF file_change + FOR VALUES WITH (MODULUS 16, REMAINDER 10); +CREATE TABLE file_change_11 PARTITION OF file_change + FOR VALUES WITH (MODULUS 16, REMAINDER 11); +CREATE TABLE file_change_12 PARTITION OF file_change + FOR VALUES WITH (MODULUS 16, REMAINDER 12); +CREATE TABLE file_change_13 PARTITION OF file_change + FOR VALUES WITH (MODULUS 16, REMAINDER 13); +CREATE TABLE file_change_14 PARTITION OF file_change + FOR VALUES WITH (MODULUS 16, REMAINDER 14); +CREATE TABLE file_change_15 PARTITION OF file_change + FOR VALUES WITH (MODULUS 16, REMAINDER 15); + +-- 4. The rows move to the partitioned table. The column list is explicit +-- because the legacy table has columns that the new one does not have. +-- The kind comes from the label: a row with a label is a snapshot, a row +-- without one is a revision. +INSERT INTO file_change (id, file_id, created_at, session_id, revn, changes, + profile_id, features, label, version, created_by, + updated_at, deleted_at, migrations, locked_by, kind) +SELECT id, file_id, created_at, session_id, revn, changes, + profile_id, features, label, version, created_by, + updated_at, deleted_at, migrations, locked_by, + CASE WHEN label IS NOT NULL THEN 1 ELSE 2 END + FROM file_change_legacy; + +-- 5. The primary key includes the partition key now. +ALTER TABLE file_change ADD PRIMARY KEY (file_id, id); + +-- 6. The foreign keys. +ALTER TABLE file_change ADD FOREIGN KEY (file_id) + REFERENCES file(id) DEFERRABLE; +ALTER TABLE file_change ADD FOREIGN KEY (locked_by) + REFERENCES profile(id) ON DELETE SET NULL DEFERRABLE; +ALTER TABLE file_change ADD FOREIGN KEY (profile_id) + REFERENCES profile(id) ON DELETE SET NULL DEFERRABLE; + +-- 7. The indexes, with the canonical names. The one of the deleted rows now +-- carries the `file_id`, so the garbage collector can read the rows it has +-- to delete without touching the table. +-- +-- There is no index for the system snapshots: the old one filtered them by +-- `data IS NOT NULL`, and the column is gone. The new one could only +-- filter them by `deleted_at IS NULL`, and the system snapshots always +-- carry a deletion date in the future, so it would never match a row (and +-- the query that would use it filters that date with an "or", which no +-- partial index can serve). +CREATE INDEX file_change__deleted_at__idx + ON file_change (deleted_at, file_id, id) WHERE deleted_at IS NOT NULL; +CREATE INDEX file_change__file_id__revn__idx + ON file_change (file_id, revn); +CREATE INDEX file_change__locked_by__idx + ON file_change (locked_by) WHERE locked_by IS NOT NULL; +CREATE INDEX file_change__profile_id__idx + ON file_change (profile_id) WHERE profile_id IS NOT NULL; +CREATE INDEX file_change__user_snapshots__idx + ON file_change (file_id) + WHERE label IS NOT NULL + AND created_by = 'user' + AND deleted_at IS NULL; + +-- 8. The legacy table is kept: it is the way back. It is dropped by hand, +-- days after the cutover: +-- DROP TABLE file_change_legacy; +-- +-- Its foreign key on `file_id` goes away with the copy: it is `NO +-- ACTION`, so while the legacy table has rows, the objects GC cannot +-- delete a file (it deletes the rows of the file first, and the legacy +-- ones are not its business). The legacy table is frozen, and the +-- rollback adds the constraint back with `NOT VALID`. +ALTER TABLE file_change_legacy + DROP CONSTRAINT file_change_file_id_fkey_legacy; + +-- On a new instance there is nothing to go back to, so the empty table +-- is dropped here. +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM file_change_legacy) THEN + DROP TABLE file_change_legacy; + END IF; +END $$; diff --git a/backend/src/app/rpc/commands/files.clj b/backend/src/app/rpc/commands/files.clj index 5c17effec4..7b06ad6e08 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -564,7 +564,6 @@ (calculate-library-summary file))] (-> file (assoc :library-summary result) - (dissoc :legacy-data) (dissoc :data)))) persist-to-cache @@ -1346,16 +1345,14 @@ WHERE file_id = ANY(?::uuid[])") (def ^:private sql:restore-file-changes - ;; Only durable history comes back: a snapshot (carries `label`) or a legacy - ;; payload row (carries `data`, the column the new model replaces with - ;; `file_data`). Revisions have neither and are deliberately left alone: they - ;; are born already scheduled for deletion one hour later, and the collector - ;; only looks at rows that carry a date, so unmarking one here would keep it - ;; in the table for ever. The `data` half is a guard for the rows still - ;; pending migration, not a dependency: drop it with the last one. - "UPDATE file_change SET deleted_at = null + ;; Only durable history comes back: a snapshot, which carries `label`. + ;; Revisions have none and are deliberately left alone: they are born + ;; already scheduled for deletion one hour later, and the collector only + ;; looks at rows that carry a date, so unmarking one here would keep it in + ;; the table for ever. + "UPDATE file_change SET deleted_at = null, updated_at = now() WHERE file_id = ANY(?::uuid[]) - AND (label IS NOT NULL OR data IS NOT NULL)") + AND label IS NOT NULL") (def ^:private sql:restore-file-data "UPDATE file_data SET deleted_at = null diff --git a/backend/src/app/rpc/commands/files_snapshot.clj b/backend/src/app/rpc/commands/files_snapshot.clj index cb3e93285f..fa2b26ffe5 100644 --- a/backend/src/app/rpc/commands/files_snapshot.clj +++ b/backend/src/app/rpc/commands/files_snapshot.clj @@ -50,6 +50,7 @@ "Retrieve a file bundle with data from a specific snapshot for read-only preview. Does not modify any database state." {::doc/added "2.16" + ::doc/changes [["2.19" "An unknown snapshot is reported with the :snapshot-not-found error code"]] ::sm/params schema:get-file-snapshot ::sm/result files/schema:file-with-permissions ::db/transaction true} @@ -151,28 +152,32 @@ (def ^:private schema:update-file-snapshot [:map {:title "update-file-snapshot"} + [:file-id ::sm/uuid] [:id ::sm/uuid] [:label ::sm/text]]) (sv/defmethod ::update-file-snapshot {::doc/added "1.20" + ::doc/changes [["2.19" "The :file-id param is now required"]] ::sm/params schema:update-file-snapshot ::db/transaction true} - [{:keys [::db/conn]} {:keys [::rpc/profile-id id label]}] - (let [snapshot (fsnap/get-minimal-snapshot conn id)] + [{:keys [::db/conn]} {:keys [::rpc/profile-id file-id id label]}] + (let [snapshot (fsnap/get-minimal-snapshot conn file-id id)] (files/check-edition-permissions! conn profile-id (:file-id snapshot)) (fsnap/update! conn (assoc snapshot :label label)))) (def ^:private schema:remove-file-snapshot [:map {:title "remove-file-snapshot"} + [:file-id ::sm/uuid] [:id ::sm/uuid]]) (sv/defmethod ::delete-file-snapshot {::doc/added "1.20" + ::doc/changes [["2.19" "The :file-id param is now required"]] ::sm/params schema:remove-file-snapshot ::db/transaction true} - [{:keys [::db/conn]} {:keys [::rpc/profile-id id]}] - (let [snapshot (fsnap/get-minimal-snapshot conn id)] + [{:keys [::db/conn]} {:keys [::rpc/profile-id file-id id]}] + (let [snapshot (fsnap/get-minimal-snapshot conn file-id id)] (files/check-edition-permissions! conn profile-id (:file-id snapshot)) (when (not= (:created-by snapshot) "user") @@ -200,14 +205,16 @@ (def ^:private schema:lock-file-snapshot [:map {:title "lock-file-snapshot"} + [:file-id ::sm/uuid] [:id ::sm/uuid]]) (sv/defmethod ::lock-file-snapshot {::doc/added "1.20" + ::doc/changes [["2.19" "The :file-id param is now required"]] ::sm/params schema:lock-file-snapshot ::db/transaction true} - [{:keys [::db/conn]} {:keys [::rpc/profile-id id]}] - (let [snapshot (fsnap/get-minimal-snapshot conn id)] + [{:keys [::db/conn]} {:keys [::rpc/profile-id file-id id]}] + (let [snapshot (fsnap/get-minimal-snapshot conn file-id id)] (files/check-edition-permissions! conn profile-id (:file-id snapshot)) (when (not= (:created-by snapshot) "user") @@ -235,18 +242,20 @@ :profile-id profile-id :locked-by (:locked-by snapshot))) - (fsnap/lock-by! conn id profile-id))) + (fsnap/lock-by! conn (:file-id snapshot) id profile-id))) (def ^:private schema:unlock-file-snapshot [:map {:title "unlock-file-snapshot"} + [:file-id ::sm/uuid] [:id ::sm/uuid]]) (sv/defmethod ::unlock-file-snapshot {::doc/added "1.20" + ::doc/changes [["2.19" "The :file-id param is now required"]] ::sm/params schema:unlock-file-snapshot ::db/transaction true} - [{:keys [::db/conn]} {:keys [::rpc/profile-id id]}] - (let [snapshot (fsnap/get-minimal-snapshot conn id)] + [{:keys [::db/conn]} {:keys [::rpc/profile-id file-id id]}] + (let [snapshot (fsnap/get-minimal-snapshot conn file-id id)] (files/check-edition-permissions! conn profile-id (:file-id snapshot)) (when (not= (:created-by snapshot) "user") @@ -273,4 +282,4 @@ :snapshot-id id :profile-id profile-id)) - (fsnap/unlock! conn id))) + (fsnap/unlock! conn (:file-id snapshot) id))) diff --git a/backend/src/app/rpc/commands/files_update.clj b/backend/src/app/rpc/commands/files_update.clj index 85a0a58b1f..c44cd830b2 100644 --- a/backend/src/app/rpc/commands/files_update.clj +++ b/backend/src/app/rpc/commands/files_update.clj @@ -20,6 +20,7 @@ [app.config :as cf] [app.db :as db] [app.features.fdata :as fdata] + [app.features.file-changes :as fchanges] [app.features.file-snapshots :as fsnap] [app.features.logical-deletion :as ldel] [app.http.errors :as errors] @@ -290,8 +291,13 @@ ;; Insert change (xlog) with deleted_at in a future data for ;; make them automatically eleggible for GC once they expires + ;; + ;; The revision carries only the delta of the save: the features of the + ;; file are not repeated here (nobody reads them from a revision, and + ;; they are duplicated on every save). (db/insert! conn :file-change {:id (uuid/next) + :kind fchanges/revision-kind :session-id session-id :profile-id profile-id :created-at timestamp @@ -300,7 +306,6 @@ :file-id (:id file) :revn (:revn file) :version (:version file) - :features (into-array (:features file)) :changes (blob/encode changes)} {::db/return-keys false}) diff --git a/backend/src/app/rpc/quotes.clj b/backend/src/app/rpc/quotes.clj index 96309afd49..903dc31070 100644 --- a/backend/src/app/rpc/quotes.clj +++ b/backend/src/app/rpc/quotes.clj @@ -422,6 +422,8 @@ (sm/lazy-validator schema:snapshots-per-file)) (def ^:private sql:get-snapshots-per-file + ;; A version is told apart by its label, and the partial index with the same + ;; predicate answers the count without touching the table. "SELECT count(*) AS total FROM file_change AS fc WHERE fc.file_id = ? @@ -453,11 +455,16 @@ (sm/lazy-validator schema:snapshots-per-team)) (def ^:private sql:get-snapshots-per-team + ;; The files of the team are resolved first, so the count filters by + ;; `file_id`: the planner can prune the partitions of `file_change` when it + ;; chooses an index scan (with a sequential scan it reads all of them). A + ;; version is told apart by its label. "SELECT count(*) AS total FROM file_change AS fc - JOIN file AS f ON (f.id = fc.file_id) - JOIN project AS p ON (p.id = f.project_id) - WHERE p.team_id = ? + WHERE fc.file_id = ANY (SELECT f.id + FROM file AS f + JOIN project AS p ON (p.id = f.project_id) + WHERE p.team_id = ?) AND fc.created_by = 'user' AND fc.deleted_at IS NULL AND fc.label IS NOT NULL") diff --git a/backend/src/app/scripts/decode_file_data.clj b/backend/src/app/scripts/decode_file_data.clj index b7d775eafc..8df07115c1 100644 --- a/backend/src/app/scripts/decode_file_data.clj +++ b/backend/src/app/scripts/decode_file_data.clj @@ -7,8 +7,8 @@ (ns app.scripts.decode-file-data "Decode a file's `file_data` binary blob to `jq`-readable JSON. - The content of `file_data.data` (and `file.data` on the `legacy-db` - backend, or the `file-data` object on storage) is not text: it is a blob + The content of `file_data.data` (or the `file-data` object on storage) is + not text: it is a blob encoded with `app.util.blob` (versions 1, 3, 4 and 5; transit+json or fressian plus lz4/zstd). That is why it cannot be inspected with SQL directly. @@ -24,7 +24,7 @@ 2. Inside the penpot context (nREPL with the backend running, where `sys/system` already exists): call `get-file-json` or `get-snapshot-json`. - They resolve the backend (`legacy-db`, `db`, `storage`), realize the + They resolve the backend (`db`, `storage`), realize the pointer-maps and objects-maps and return JSON. (require '[app.scripts.decode-file-data :as dfd] :reload) @@ -39,6 +39,7 @@ `[.data[\"pages-index\"][] | {id, name}]`, etc." (:require [app.binfile.common :as bfc] + [app.common.exceptions :as ex] [app.common.json :as json] [app.db :as db] [app.features.file-snapshots :as fsnap] @@ -183,16 +184,23 @@ (defn get-snapshot-json "Return a snapshot as JSON (realized). For nREPL. - Takes the same `format` and `output` as `get-file-json`." + Takes the same `format` and `output` as `get-file-json`. It raises when the + file has no snapshot with that id." [system file-id snapshot-id & {:keys [format output] :or {format :full}}] (let [snapshot (db/run! system (fn [cfg] (fsnap/get-snapshot cfg file-id snapshot-id))) - data (case format - :full snapshot - :data (:data snapshot) - :summary (summarize-data (:data snapshot)) - :pages (:pages (summarize-data (:data snapshot))))] + data (if snapshot + (case format + :full snapshot + :data (:data snapshot) + :summary (summarize-data (:data snapshot)) + :pages (:pages (summarize-data (:data snapshot)))) + (ex/raise :type :not-found + :code :snapshot-not-found + :hint "unable to find snapshot with the provided id" + :snapshot-id snapshot-id + :file-id file-id))] (write-output (data->json data) output))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/backend/src/app/srepl/procs/fdata_storage.clj b/backend/src/app/srepl/procs/fdata_storage.clj deleted file mode 100644 index 5759aa43b9..0000000000 --- a/backend/src/app/srepl/procs/fdata_storage.clj +++ /dev/null @@ -1,141 +0,0 @@ -;; This Source Code Form is subject to the terms of the Mozilla Public -;; License, v. 2.0. If a copy of the MPL was not distributed with this -;; file, You can obtain one at http://mozilla.org/MPL/2.0/. -;; -;; Copyright (c) KALEIDOS SUBSIDIARY SL - -(ns app.srepl.procs.fdata-storage - (:require - [app.common.logging :as l] - [app.db :as db])) - -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -;; SNAPSHOTS -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; - -(def sql:get-unmigrated-snapshots - "SELECT fc.id, fc.file_id - FROM file_change AS fc - WHERE fc.data IS NOT NULL - AND fc.label IS NOT NULL - ORDER BY fc.id ASC") - -(def sql:get-migrated-snapshots - "SELECT f.id, f.file_id - FROM file_data AS f - WHERE f.data IS NOT NULL - AND f.type = 'snapshot' - AND f.id != f.file_id - ORDER BY f.id ASC") - -(defn migrate-snapshot-to-storage - "Migrate the current existing files to store data in new storage - tables." - {:query sql:get-unmigrated-snapshots} - [{:keys [::db/conn]} {:keys [id file-id]} & {:as options}] - (let [{:keys [id file-id data created-at updated-at]} - (db/get* conn :file-change {:id id :file-id file-id} - ::db/for-update true - ::db/remove-deleted false)] - (when data - (l/inf :hint "migrating snapshot" :file-id (str file-id) :id (str id)) - (db/update! conn :file-change - {:data nil} - {:id id :file-id file-id} - {::db/return-keys false}) - (db/insert! conn :file-data - {:backend "db" - :metadata nil - :type "snapshot" - :data data - :created-at created-at - :modified-at updated-at - :file-id file-id - :id id} - {::db/return-keys false})))) - -(defn rollback-snapshot-from-storage - "Migrate back to the file table storage." - {:query sql:get-unmigrated-snapshots} - [{:keys [::db/conn]} {:keys [id file-id]} & {:as opts}] - (when-let [{:keys [id file-id data]} - (db/get* conn :file-data {:id id :file-id file-id :type "snapshot"} - ::db/for-update true - ::db/remove-deleted false)] - (l/inf :hint "rollback snapshot" :file-id (str file-id) :id (str id)) - (db/update! conn :file-change - {:data data} - {:id id :file-id file-id} - {::db/return-keys false}) - (db/delete! conn :file-data - {:id id :file-id file-id :type "snapshot"} - {::db/return-keys false}))) - -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -;; FILES -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; - -(def sql:get-unmigrated-files - "SELECT f.id - FROM file AS f - WHERE f.data IS NOT NULL - ORDER BY f.modified_at ASC") - -(def sql:get-migrated-files - "SELECT f.id, f.file_id - FROM file_data AS f - WHERE f.data IS NOT NULL - AND f.id = f.file_id - ORDER BY f.id ASC") - -(defn migrate-file-to-storage - "Migrate the current existing files to store data in new storage - tables." - {:query sql:get-unmigrated-files} - [{:keys [::db/conn] :as cfg} {:keys [id]} & {:as opts}] - (let [{:keys [id data created-at modified-at]} - (db/get* conn :file {:id id} - ::db/for-update true - ::db/remove-deleted false)] - - (when data - (l/inf :hint "migrating file" :file-id (str id)) - - (db/update! conn :file {:data nil} {:id id} ::db/return-keys false) - (db/insert! conn :file-data - {:backend "db" - :metadata nil - :type "main" - :data data - :created-at created-at - :modified-at modified-at - :file-id id - :id id} - {::db/return-keys false})) - - (let [snapshots-sql - (str "WITH snapshots AS (" sql:get-unmigrated-snapshots ") " - "SELECT s.* FROM snapshots AS s WHERE s.file_id = ?")] - (run! (fn [params] - (migrate-snapshot-to-storage cfg params opts)) - (db/plan cfg [snapshots-sql id]))))) - - -(defn rollback-file-from-storage - "Migrate back to the file table storage." - {:query sql:get-migrated-files} - [{:keys [::db/conn] :as cfg} {:keys [id]} & {:as opts}] - (when-let [{:keys [id data]} - (db/get* conn :file-data {:id id :file-id id :type "main"} - ::db/for-update true - ::db/remove-deleted false)] - (l/inf :hint "rollback file" :file-id (str id)) - (db/update! conn :file {:data data} {:id id} ::db/return-keys false) - (db/delete! conn :file-data {:file-id id :id id :type "main"} ::db/return-keys false) - - (let [snapshots-sql - (str "WITH snapshots AS (" sql:get-migrated-snapshots ") " - "SELECT s.* FROM snapshots AS s WHERE s.file_id = ?")] - (run! (fn [params] - (rollback-snapshot-from-storage cfg params opts)) - (db/plan cfg [snapshots-sql id]))))) diff --git a/backend/src/app/tasks/delete_object.clj b/backend/src/app/tasks/delete_object.clj index 2545fe97e4..0398264f90 100644 --- a/backend/src/app/tasks/delete_object.clj +++ b/backend/src/app/tasks/delete_object.clj @@ -51,7 +51,8 @@ :deleted-at (ct/format-inst deleted-at)) (db/update! conn :file-change - {:deleted-at deleted-at} + {:deleted-at deleted-at + :updated-at (ct/now)} {:id id :file-id file-id} {::db/return-keys false}) @@ -91,7 +92,8 @@ ;; lifetime; the collector would still take it, just much later than it ;; should. (db/update! conn :file-change - {:deleted-at deleted-at} + {:deleted-at deleted-at + :updated-at (ct/now)} {:file-id id :deleted-at nil} {::db/return-keys false}) diff --git a/backend/src/app/tasks/objects_gc.clj b/backend/src/app/tasks/objects_gc.clj index 1d6f842e30..b1ee555772 100644 --- a/backend/src/app/tasks/objects_gc.clj +++ b/backend/src/app/tasks/objects_gc.clj @@ -637,41 +637,49 @@ {:fetch-size 5})))))) (def ^:private sql:get-file-change - "SELECT id FROM file_change + "SELECT id, file_id FROM file_change WHERE deleted_at IS NOT NULL AND deleted_at <= ? ORDER BY deleted_at ASC") (def ^:private sql:lock-file-change "SELECT id, file_id, deleted_at FROM file_change - WHERE id = ANY(?::uuid[]) + WHERE file_id = ? + AND id = ? AND deleted_at IS NOT NULL AND deleted_at <= ? FOR UPDATE SKIP LOCKED") (defn- delete-file-changes-chunk - [cfg ids] + [cfg rows] (try (db/tx-run! cfg (fn [{:keys [::db/conn ::timestamp]}] (db/exec-one! conn ["SET LOCAL rules.deletion_protection TO off"]) - (let [ids (db/create-array conn "uuid" ids)] - (->> (db/exec! conn [sql:lock-file-change ids timestamp]) - (reduce (fn [total {:keys [id file-id deleted-at]}] - (l/trc :obj "file-change" - :id (str id) - :file-id (str file-id) - :deleted-at (ct/format-inst deleted-at)) + (reduce (fn [total {:keys [id file-id]}] + ;; the row is locked by its (file-id, id) pair, so + ;; the lookup uses the primary key and prunes to + ;; the partition that holds the file. + (if-let [{:keys [deleted-at]} + (db/exec-one! conn [sql:lock-file-change + file-id id timestamp])] + (do + (l/trc :obj "file-change" + :id (str id) + :file-id (str file-id) + :deleted-at (ct/format-inst deleted-at)) - (let [affected (-> (db/delete! conn :file-change {:id id}) - (db/get-update-count))] - (+ total affected))) - 0))))) + (let [affected (-> (db/delete! conn :file-change + {:id id :file-id file-id}) + (db/get-update-count))] + (+ total affected))) + total)) + 0 rows))) (catch Throwable cause (l/err :obj "file-change" :hint "error on chunk deletion, skipping it" - :chunk ids + :chunk rows :cause cause) 0))) @@ -681,7 +689,10 @@ ([total chunk] (+ total (delete-file-changes-chunk cfg chunk))))] (db/tx-run! (assoc cfg ::db/read-only true) (fn [{:keys [::db/conn]}] - (transduce (comp (map :id) (partition-all chunk-size)) rf 0 + (transduce (comp (map (fn [row] {:id (:id row) + :file-id (:file-id row)})) + (partition-all chunk-size)) + rf 0 (db/plan conn [sql:get-file-change timestamp] {:fetch-size 5})))))) diff --git a/backend/test/backend_tests/db_test.clj b/backend/test/backend_tests/db_test.clj index 29d3095f32..7345e0b5d5 100644 --- a/backend/test/backend_tests/db_test.clj +++ b/backend/test/backend_tests/db_test.clj @@ -62,3 +62,11 @@ (t/deftest uuid->hash-code-stable-for-zero-uuid (t/is (= 3659997967308761462 (db/uuid->hash-code uuid/zero)))) + +(t/deftest pool-enables-the-partitionwise-join + ;; The pool sets the GUC on every connection, so the planner can resolve a + ;; join between two partitioned tables pairwise, partition by partition. + (let [row (db/exec-one! th/*pool* + ["SELECT current_setting('enable_partitionwise_join') AS partitionwise_join"])] + + (t/is (= "on" (:partitionwise-join row))))) diff --git a/backend/test/backend_tests/file_change_retention_test.clj b/backend/test/backend_tests/file_change_retention_test.clj index 7cede5a164..2c6e07bb37 100644 --- a/backend/test/backend_tests/file_change_retention_test.clj +++ b/backend/test/backend_tests/file_change_retention_test.clj @@ -10,7 +10,7 @@ A file has two kinds of row. Durable history: the snapshots, which a restore brings back. And ephemeral revisions: the xlog of an edit, born already scheduled for deletion one hour later, which nobody needs once the sessions - that were going to read them are gone. + that were going to read them are gone. The `kind` column tells them apart. The invariant under test is that a deletion date is assigned once and never moved: restoring a file must not unmark a revision, and deleting a file must @@ -49,9 +49,8 @@ (defn- insert-change "Inserts a `file_change` row directly, so the test builds the exact shape it needs. The row class is decided by what the caller passes: a revision carries - only `changes`, a snapshot carries `label`, and a legacy payload row carries - `data` without a label. Returns the new row id." - [file-id {:keys [label data changes created-at deleted-at created-by]}] + only `changes` and a snapshot carries `label`. Returns the new row id." + [file-id {:keys [label changes created-at deleted-at created-by]}] (let [created-at (or created-at (ct/now)) id (uuid/next)] (th/db-insert! :file-change @@ -64,7 +63,7 @@ :version 0 :created-by (or created-by "system") :label label - :data data + :kind (if label 1 2) :changes changes} {::db/return-keys false}) id)) @@ -104,10 +103,6 @@ snapshot-id (insert-change file-id {:created-at old-at :created-by "user" :label "a snapshot" - :deleted-at trash-at}) - - legacy-id (insert-change file-id {:created-at old-at - :data (blob/encode {:legacy true}) :deleted-at trash-at})] (restore-file file-id) @@ -116,10 +111,7 @@ (t/is (= revision-at (:deleted-at (find-change revision-id))))) (t/testing "the snapshot comes back" - (t/is (nil? (:deleted-at (find-change snapshot-id))))) - - (t/testing "the legacy payload row comes back" - (t/is (nil? (:deleted-at (find-change legacy-id)))))))) + (t/is (nil? (:deleted-at (find-change snapshot-id)))))))) (t/deftest deleting-a-file-does-not-push-the-revision-schedule-out (binding [ct/*clock* (ct/fixed-clock test-now)] @@ -170,6 +162,7 @@ out (th/command! {::th/type :update-file-snapshot ::rpc/profile-id (:id profile) + :file-id file-id :id revision-id :label "renamed"})] diff --git a/backend/test/backend_tests/file_change_schema_test.clj b/backend/test/backend_tests/file_change_schema_test.clj new file mode 100644 index 0000000000..b8ef5f697b --- /dev/null +++ b/backend/test/backend_tests/file_change_schema_test.clj @@ -0,0 +1,111 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC Sucursal en España SL + +(ns backend-tests.file-change-schema-test + (:require + [app.db :as db] + [backend-tests.helpers :as th] + [clojure.test :as t])) + +(t/use-fixtures :once th/state-init) +(t/use-fixtures :each th/database-reset) + +(defn- query + "Runs a query that returns a single value and returns it." + [sql & params] + (-> (db/exec! th/*pool* (into [sql] params)) + first + (vals) + first)) + +(defn- partition-names + [] + (->> (db/exec! th/*pool* ["SELECT c.relname AS name + FROM pg_class c + JOIN pg_partition_tree('file_change') t ON t.relid = c.oid + WHERE t.isleaf + ORDER BY c.relname"]) + (mapv :name))) + +(t/deftest file-change-is-partitioned-in-sixteen-named-partitions + (t/is (= (mapv #(format "file_change_%02d" %) (range 16)) + (partition-names)))) + +(t/deftest file-change-primary-key-is-file-id-and-id + (t/is (= "PRIMARY KEY (file_id, id)" + (query "SELECT pg_get_constraintdef(oid) AS def + FROM pg_constraint + WHERE conrelid = 'file_change'::regclass AND contype = 'p'")))) + +(t/deftest file-change-has-three-foreign-keys-and-one-check + (t/is (= 3 (query "SELECT count(*) AS total + FROM pg_constraint + WHERE conrelid = 'file_change'::regclass AND contype = 'f'"))) + (t/is (= 1 (query "SELECT count(*) AS total + FROM pg_constraint + WHERE conrelid = 'file_change'::regclass AND contype = 'c'"))) + (t/is (re-find #"kind" + (query "SELECT pg_get_constraintdef(oid) AS def + FROM pg_constraint + WHERE conrelid = 'file_change'::regclass AND contype = 'c'")))) + +(t/deftest file-change-has-the-six-canonical-indexes + (t/is (= ["file_change__deleted_at__idx" + "file_change__file_id__revn__idx" + "file_change__locked_by__idx" + "file_change__profile_id__idx" + "file_change__user_snapshots__idx" + "file_change_pkey"] + (->> (db/exec! th/*pool* ["SELECT indexname AS name + FROM pg_indexes + WHERE tablename = 'file_change' + AND schemaname = current_schema() + ORDER BY indexname"]) + (mapv :name))))) + +(t/deftest file-change-has-no-legacy-columns-and-a-kind-column + (t/is (= [] + (->> (db/exec! th/*pool* ["SELECT attname AS name + FROM pg_attribute + WHERE attrelid = 'file_change'::regclass + AND attnum > 0 AND NOT attisdropped + AND attname IN ('data', 'data_ref_id', 'data_backend')"]) + (mapv :name)))) + (t/is (true? (query "SELECT attnotnull AS notnull + FROM pg_attribute + WHERE attrelid = 'file_change'::regclass + AND attname = 'kind'")))) + +(t/deftest the-legacy-table-has-no-foreign-key-to-file + ;; The legacy table is kept as the way back, but its foreign key on + ;; `file_id` is `NO ACTION`: while it has rows, the objects GC cannot + ;; delete the files they point to. + (t/is (= 0 (query "SELECT count(*) AS total + FROM pg_constraint con + JOIN pg_class rel ON rel.oid = con.conrelid + WHERE rel.relname = 'file_change_legacy' + AND con.contype = 'f' + AND con.confrelid = 'file'::regclass")))) + +(t/deftest inserting-a-row-without-a-kind-fails + ;; The column has no default on purpose: an insert that forgets it has to + ;; fail instead of storing the wrong kind silently. + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile)})] + (t/is (thrown? java.sql.SQLException + (db/exec! th/*pool* ["INSERT INTO file_change (file_id, id) + VALUES (?, gen_random_uuid())" + (:id file)]))))) + +(t/deftest inserting-a-row-with-an-unknown-kind-fails + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile)})] + (t/is (thrown? java.sql.SQLException + (db/exec! th/*pool* ["INSERT INTO file_change (file_id, id, kind) + VALUES (?, gen_random_uuid(), 3)" + (:id file)]))))) diff --git a/backend/test/backend_tests/objects_gc_test.clj b/backend/test/backend_tests/objects_gc_test.clj index 5a3052901f..9f4f880683 100644 --- a/backend/test/backend_tests/objects_gc_test.clj +++ b/backend/test/backend_tests/objects_gc_test.clj @@ -10,6 +10,7 @@ [app.common.time :as ct] [app.config :as cf] [app.db :as db] + [app.features.file-changes :as fchanges] [app.rpc :as-alias rpc] [app.storage :as sto] [app.tasks.objects-gc :as ogc] @@ -73,6 +74,25 @@ (t/is (pos? (:processed result))) (t/is (nil? (find-file (:id file)))))) +(t/deftest deletes-the-pending-file-changes + ;; The pending file changes are locked and deleted by their (file-id, id) + ;; pair, so the rows of a file that is pending deletion are gone after the + ;; run. + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile)}) + + _ (th/db-insert! :file-change + {:file-id (:id file) + :kind fchanges/revision-kind + :deleted-at (ct/in-past "10m")}) + + result (th/run-task! :objects-gc {})] + + (t/is (pos? (:processed result))) + (t/is (nil? (th/db-get :file-change {:file-id (:id file)} + {::db/remove-deleted false}))))) + (t/deftest profile-cascade-keeps-profile-deletion-date ;; The cascade marks the owned rows with the deletion date of the profile, ;; like the RPC delete-profile flow does, not with the timestamp of the run. diff --git a/backend/test/backend_tests/restore_storage_objects_test.clj b/backend/test/backend_tests/restore_storage_objects_test.clj index 5986170b20..bb2201659d 100644 --- a/backend/test/backend_tests/restore_storage_objects_test.clj +++ b/backend/test/backend_tests/restore_storage_objects_test.clj @@ -9,6 +9,7 @@ [app.common.time :as ct] [app.common.uuid :as uuid] [app.db :as db] + [app.features.file-changes :as fchanges] [app.rpc.commands.files :as files] [app.srepl.main :as srepl] [app.storage :as sto] @@ -130,3 +131,29 @@ (srepl/restore-team! (:id team))) (t/is (nil? (:deleted-at (find-object sobject)))))) + +(t/deftest restoring-a-file-updates-its-file-changes + ;; Restoring a file brings its file changes back to life, and that mutation + ;; has to be observable through `updated_at`, the watermark used by the + ;; online partitioning copy of `file_change`. + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile)}) + + stale-at #inst "2020-01-01T00:00:00.000Z" + + _ (th/db-insert! :file-change + {:file-id (:id file) + :label "v1" + :kind fchanges/snapshot-kind + :deleted-at (ct/in-past {:days 1}) + :updated-at stale-at})] + + (restore-file! (:id file)) + + (let [change (th/db-get :file-change {:file-id (:id file)} + {::db/remove-deleted false})] + (t/is (nil? (:deleted-at change))) + (t/is (pos? (- (inst-ms (:updated-at change)) + (inst-ms stale-at))) + "updated_at must be bumped by the restore")))) diff --git a/backend/test/backend_tests/rpc_file_snapshot_test.clj b/backend/test/backend_tests/rpc_file_snapshot_test.clj index aed2b5ce39..39b01bb06e 100644 --- a/backend/test/backend_tests/rpc_file_snapshot_test.clj +++ b/backend/test/backend_tests/rpc_file_snapshot_test.clj @@ -79,6 +79,19 @@ (t/is (= 0 (:revn row))) (t/is (= (:id profile) (:profile-id row)))))) + (t/testing "update snapshot with file-id" + (let [params {::th/type :update-file-snapshot + ::rpc/profile-id (:id profile) + :file-id (:id file) + :id @snapshot-id + :label "label1-updated"} + out (th/command! params)] + ;; (th/print-result! out) + (t/is (nil? (:error out))) + (t/is (true? (:result out))) + (t/is (= "label1-updated" + (:label (th/db-get :file-change {:id @snapshot-id})))))) + (t/testing "restore snapshot" (let [params {::th/type :restore-file-snapshot ::rpc/profile-id (:id profile) @@ -220,3 +233,225 @@ ;; (th/print-result! out) (t/is (nil? (:error out))) (t/is (true? (:result out))))))) + +(t/deftest snapshots-require-file-id + ;; Since version 2.19 the snapshot commands require the `file-id` + ;; parameter, so partition pruning can always be applied on the + ;; `file_change` table. + (let [profile (th/create-profile* 1 {:is-active true}) + team-id (:default-team-id profile) + proj-id (:default-project-id profile) + + file (th/create-file* 1 {:profile-id (:id profile) + :project-id proj-id + :is-shared false}) + + snapshot + (let [params {::th/type :create-file-snapshot + ::rpc/profile-id (:id profile) + :file-id (:id file) + :label "label1"} + out (th/command! params)] + ;; (th/print-result! out) + + (t/is (nil? (:error out))) + (:result out))] + + (doseq [type [:update-file-snapshot :lock-file-snapshot + :unlock-file-snapshot :delete-file-snapshot]] + (let [params (merge {::th/type type + ::rpc/profile-id (:id profile) + :id (:id snapshot)} + (when (= type :update-file-snapshot) + {:label "label2"})) + out (th/command! params)] + + (t/testing (str (name type) " without file-id") + (t/is (th/ex-of-code? (:error out) :params-validation))))))) + +(t/deftest snapshots-reject-a-file-id-that-does-not-match + ;; The snapshot commands filter by `file_id` so that, once `file_change` is + ;; hash-partitioned by it, the lookup prunes to a single partition. Sending + ;; a `file-id` of another file must not find nor touch the snapshot. + (let [profile (th/create-profile* 1 {:is-active true}) + file-a (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + file-b (th/create-file* 2 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + + snapshot (-> (th/command! {::th/type :create-file-snapshot + ::rpc/profile-id (:id profile) + :file-id (:id file-a) + :label "label1"}) + :result)] + + (doseq [type [:update-file-snapshot :lock-file-snapshot + :unlock-file-snapshot :delete-file-snapshot]] + (let [params (merge {::th/type type + ::rpc/profile-id (:id profile) + :file-id (:id file-b) + :id (:id snapshot)} + (when (= type :update-file-snapshot) + {:label "label2"})) + out (th/command! params)] + + (t/testing (name type) + (t/is (th/ex-info? (:error out))) + (t/is (= :not-found (:type (ex-data (:error out))))) + (t/is (= :snapshot-not-found (:code (ex-data (:error out))))) + (t/is (= "label1" + (:label (th/db-get :file-change {:id (:id snapshot)} + {::db/remove-deleted false}))) + "the snapshot of file-a stays intact")))))) + +(t/deftest file-change-mutations-maintain-updated-at + ;; `updated_at` is the per-row watermark used by the online partitioning + ;; copy of `file_change` (hash partitioning by file_id): every mutation + ;; must bump it, otherwise the copy misses the row. Lock, unlock and the + ;; soft-delete used to leave it stale. + (let [profile (th/create-profile* 1 {:is-active true}) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + + snapshot + (let [params {::th/type :create-file-snapshot + ::rpc/profile-id (:id profile) + :file-id (:id file) + :label "label1"} + out (th/command! params)] + ;; (th/print-result! out) + + (t/is (nil? (:error out))) + (:result out)) + + stale-at #inst "2020-01-01T00:00:00.000Z"] + + (doseq [type [:lock-file-snapshot :unlock-file-snapshot :delete-file-snapshot]] + (t/testing (name type) + ;; make the row look old, so the mutation has to bump it + (th/db-update! :file-change {:updated-at stale-at} + {:id (:id snapshot)} + {::db/remove-deleted false}) + + (let [params {::th/type type + ::rpc/profile-id (:id profile) + :file-id (:id file) + :id (:id snapshot)} + out (th/command! params)] + + (t/is (nil? (:error out))) + (t/is (true? (:result out))) + (t/is (pos? (- (inst-ms (:updated-at (th/db-get :file-change + {:id (:id snapshot)} + {::db/remove-deleted false}))) + (inst-ms stale-at))) + "updated_at must be bumped by the mutation")))) + + (t/testing "the deleted snapshot is marked as deleted" + (t/is (some? (:deleted-at (th/db-get :file-change {:id (:id snapshot)} + {::db/remove-deleted false}))))) + + ;; process the delete-object task enqueued by the snapshot delete + (th/run-pending-tasks!))) + +(t/deftest unknown-snapshot-is-reported-as-not-found + ;; The error points at the id that was not found: the snapshots are looked + ;; up by id, not by label. Both commands that resolve a snapshot report it + ;; the same way. + (let [profile (th/create-profile* 1 {:is-active true}) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + + params {::rpc/profile-id (:id profile) + :file-id (:id file) + :id (uuid/random)}] + + (doseq [type [:restore-file-snapshot :get-file-snapshot]] + (t/testing (name type) + (let [out (th/command! (assoc params ::th/type type))] + + (t/is (th/ex-of-code? (:error out) :snapshot-not-found)) + (t/is (= "unable to find snapshot with the provided id" + (:hint (ex-data (:error out)))))))))) + +(t/deftest revision-and-snapshot-carry-different-content + ;; A revision is the delta of a save: it carries the changes and nothing + ;; else. A snapshot is the state of the file at a point: it carries the + ;; features and the migrations. + (let [profile (th/create-profile* 1 {:is-active true}) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + page-id (uuid/random)] + + (update-file! + :file-id (:id file) + :profile-id (:id profile) + :revn 0 + :vern 0 + :changes + [{:type :add-page + :name "test" + :id page-id}]) + + (t/testing "the revision carries the changes and not the features" + (let [rows (th/db-query :file-change {:file-id (:id file)}) + revision (first (filter #(nil? (:label %)) rows))] + + (t/is (some? revision)) + (t/is (some? (:changes revision))) + (t/is (nil? (:features revision))))) + + (t/testing "the snapshot carries the migrations and the features" + (let [out (th/command! {::th/type :create-file-snapshot + ::rpc/profile-id (:id profile) + :file-id (:id file) + :label "label1"})] + + (t/is (nil? (:error out))) + + (let [snapshot (th/db-get :file-change {:id (get-in out [:result :id])})] + (t/is (= "label1" (:label snapshot))) + (t/is (some? (:migrations snapshot))) + (t/is (some? (:features snapshot)))))))) + +(t/deftest file-change-rows-have-a-kind + ;; The kind tells the two kinds of rows of `file_change` apart: a snapshot + ;; (1) and a revision (2). The values are the contract with the database. + (let [profile (th/create-profile* 1 {:is-active true}) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + page-id (uuid/random)] + + (update-file! + :file-id (:id file) + :profile-id (:id profile) + :revn 0 + :vern 0 + :changes + [{:type :add-page + :name "test" + :id page-id}]) + + (t/testing "the revision is a revision" + (let [rows (th/db-query :file-change {:file-id (:id file)}) + revision (first (filter #(nil? (:label %)) rows))] + + (t/is (some? revision)) + (t/is (= 2 (:kind revision))))) + + (t/testing "the snapshot is a snapshot" + (let [out (th/command! {::th/type :create-file-snapshot + ::rpc/profile-id (:id profile) + :file-id (:id file) + :label "label1"})] + + (t/is (nil? (:error out))) + + (let [snapshot (th/db-get :file-change {:id (get-in out [:result :id])})] + (t/is (= 1 (:kind snapshot)))))))) diff --git a/backend/test/backend_tests/rpc_quotes_test.clj b/backend/test/backend_tests/rpc_quotes_test.clj index 03e0e05389..cb4bda799f 100644 --- a/backend/test/backend_tests/rpc_quotes_test.clj +++ b/backend/test/backend_tests/rpc_quotes_test.clj @@ -560,6 +560,7 @@ ;; deleting a version takes it out of the count right away (let [out (th/command! {::th/type :delete-file-snapshot ::rpc/profile-id (:id profile) + :file-id (:id file) :id (:id snapshot)})] (t/is (nil? (:error out))) (t/is (true? (:result out))) diff --git a/backend/test/backend_tests/scripts_decode_file_data_test.clj b/backend/test/backend_tests/scripts_decode_file_data_test.clj index e14618542f..3a6484b4ed 100644 --- a/backend/test/backend_tests/scripts_decode_file_data_test.clj +++ b/backend/test/backend_tests/scripts_decode_file_data_test.clj @@ -10,11 +10,14 @@ [app.common.uuid :as uuid] [app.scripts.decode-file-data :as dfd] [app.util.blob :as blob] + [backend-tests.helpers :as th] [clojure.java.io :as io] [clojure.test :as t]) (:import java.util.Base64)) +(t/use-fixtures :once th/state-init) + (defn- sample-file-data [] (let [p1 (uuid/next) @@ -80,3 +83,11 @@ parsed (json/decode (slurp tmp-out))] (t/is (== 2 (get-in parsed ["stats" "pages"]))) (t/is (= ["Accueil1" "Contact"] (mapv #(get % "name") (get parsed "pages")))))) + +(t/deftest get-snapshot-json-fails-when-the-snapshot-does-not-exist + ;; The snapshot lookup returns nil when there is no match, and a nil + ;; snapshot used to be printed as an empty JSON instead of failing. + (let [file-id (uuid/next) + snapshot-id (uuid/next)] + (t/is (thrown? clojure.lang.ExceptionInfo + (dfd/get-snapshot-json th/*system* file-id snapshot-id))))) diff --git a/docs/technical-guide/configuration.md b/docs/technical-guide/configuration.md index e99e8d5cd5..fe636d0ce7 100644 --- a/docs/technical-guide/configuration.md +++ b/docs/technical-guide/configuration.md @@ -778,10 +778,11 @@ __Since version 2.11.0__ You can change the default file data storage backend with `PENPOT_FILE_DATA_BACKEND` environment variable. Possible values are: -- `legacy-db`: the current default backend, continues storing the file data of files and snapshots in the same location as previous versions of Penpot (< 2.11.0), this is a conservative default behaviour and will be changed to `db` in next versions. -- `db`: stores the file data on an specific table (the future default backend). +- `db`: the default backend, stores the file data of files and snapshots on a specific table. - `storage`: stores the file data using the objects storage system (S3 or FS, depending on which one is configured) +The `legacy-db` backend (the default until now, which kept the file data of files and snapshots in the same location as the versions of Penpot previous to 2.11.0) is not available anymore, and the backend refuses to start with it. An instance that used it has to migrate its file data to the `db` backend before upgrading. + This also comes with an additional feature that allows offload the "inactive" files on file storage backend and leaves the database only for the active files. To enable it, you should use the `enable-tiered-file-data-storage` flag and `db` as file data storage backend. ```bash diff --git a/frontend/src/app/main/data/workspace/versions.cljs b/frontend/src/app/main/data/workspace/versions.cljs index 5e376d6a98..28045405a3 100644 --- a/frontend/src/app/main/data/workspace/versions.cljs +++ b/frontend/src/app/main/data/workspace/versions.cljs @@ -92,7 +92,7 @@ (rx/of (update-versions-state {:editing nil}) (ev/event {::ev/name "rename-version" :file-id file-id})) - (->> (rp/cmd! :update-file-snapshot {:id id :label label}) + (->> (rp/cmd! :update-file-snapshot {:id id :file-id file-id :label label}) (rx/map fetch-versions))))))) (defn- clear-preview-state @@ -135,9 +135,10 @@ (ptk/reify ::delete-version ptk/WatchEvent - (watch [_ _ _] - (->> (rp/cmd! :delete-file-snapshot {:id id}) - (rx/map fetch-versions))))) + (watch [_ state _] + (let [file-id (:current-file-id state)] + (->> (rp/cmd! :delete-file-snapshot {:id id :file-id file-id}) + (rx/map fetch-versions)))))) (defn pin-version [id] @@ -147,8 +148,10 @@ (watch [_ state _] (when-let [version (->> (dm/get-in state [:workspace-versions :data]) (d/seek #(= (:id %) id)))] - (let [params {:id id - :label (ct/format-inst (:created-at version) :localized-date)}] + (let [file-id (:current-file-id state) + params {:id id + :file-id file-id + :label (ct/format-inst (:created-at version) :localized-date)}] (->> (rp/cmd! :update-file-snapshot params) (rx/mapcat (fn [_] @@ -161,18 +164,20 @@ (assert (uuid? id) "expected valid uuid for `id`") (ptk/reify ::lock-version ptk/WatchEvent - (watch [_ _ _] - (->> (rp/cmd! :lock-file-snapshot {:id id}) - (rx/map fetch-versions))))) + (watch [_ state _] + (let [file-id (:current-file-id state)] + (->> (rp/cmd! :lock-file-snapshot {:id id :file-id file-id}) + (rx/map fetch-versions)))))) (defn unlock-version [id] (assert (uuid? id) "expected valid uuid for `id`") (ptk/reify ::unlock-version ptk/WatchEvent - (watch [_ _ _] - (->> (rp/cmd! :unlock-file-snapshot {:id id}) - (rx/map fetch-versions))))) + (watch [_ state _] + (let [file-id (:current-file-id state)] + (->> (rp/cmd! :unlock-file-snapshot {:id id :file-id file-id}) + (rx/map fetch-versions)))))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; RESTORE VERSION EVENTS diff --git a/frontend/src/app/plugins/file.cljs b/frontend/src/app/plugins/file.cljs index 59d222512b..1124cefd09 100644 --- a/frontend/src/app/plugins/file.cljs +++ b/frontend/src/app/plugins/file.cljs @@ -54,7 +54,7 @@ :else (do (swap! data assoc :label value :created-by "user") - (->> (rp/cmd! :update-file-snapshot {:id (:id @data) :label value}) + (->> (rp/cmd! :update-file-snapshot {:id (:id @data) :file-id file-id :label value}) (rx/take 1) (rx/subs! #(st/emit! (se/event plugin-id "rename-version" :file-id file-id)))))))} @@ -95,7 +95,7 @@ :else (let [version-id (:id @data)] - (->> (rp/cmd! :delete-file-snapshot {:id version-id}) + (->> (rp/cmd! :delete-file-snapshot {:id version-id :file-id file-id}) (rx/map (constantly nil)) (rx/subs! resolve reject))))))) @@ -111,8 +111,9 @@ (u/reject-not-valid reject :pin "Only auto-saved versions can be pinned") :else - (let [params {:id (:id @data) - :label (ct/format-inst (:created-at @data) :localized-date)}] + (let [params {:id (:id @data) + :file-id file-id + :label (ct/format-inst (:created-at @data) :localized-date)}] (->> (rx/zip (rp/cmd! :get-team-users {:file-id file-id}) (rp/cmd! :update-file-snapshot params)) (rx/subs! diff --git a/frontend/test/frontend_tests/data/workspace_versions_test.cljs b/frontend/test/frontend_tests/data/workspace_versions_test.cljs index 5ec7fa87bb..ecf574b917 100644 --- a/frontend/test/frontend_tests/data/workspace_versions_test.cljs +++ b/frontend/test/frontend_tests/data/workspace_versions_test.cljs @@ -6,6 +6,7 @@ (ns frontend-tests.data.workspace-versions-test (:require + [app.common.time :as ct] [app.common.uuid :as uuid] [app.main.data.changes :as dch] [app.main.data.persistence :as dps] @@ -187,3 +188,32 @@ (t/is (nil? (get-in result [:workspace-versions :backup])))) (finally (rx/dispose! sub))))) + +(t/deftest version-events-send-the-current-file-id + ;; The snapshot RPC commands require the `file-id` (they need it for + ;; partition pruning on the `file_change` table); the workspace version + ;; events must always send it, taken from the current file. + (let [file-id (uuid/next) + version-id (uuid/next) + state {:current-file-id file-id + :workspace-versions {:data [{:id version-id + :created-at (ct/now) + :created-by "system"}]}} + call! (fn [event] + (let [requests (atom [])] + (with-redefs [rp/cmd! (mock/stub (fn [cmd params] + (swap! requests conj [cmd params]) + (rx/of true)))] + (->> (ptk/watch event state (rx/empty)) + (rx/subs! (fn [_]))) + (first @requests))))] + + (doseq [[cmd event] [[:update-file-snapshot (versions/rename-version version-id "name")] + [:delete-file-snapshot (versions/delete-version version-id)] + [:lock-file-snapshot (versions/lock-version version-id)] + [:unlock-file-snapshot (versions/unlock-version version-id)] + [:update-file-snapshot (versions/pin-version version-id)]]] + (t/testing (name cmd) + (let [[sent-cmd params] (call! event)] + (t/is (= cmd sent-cmd)) + (t/is (= file-id (:file-id params)))))))) diff --git a/operations/README.md b/operations/README.md new file mode 100644 index 0000000000..5a9a2508e9 --- /dev/null +++ b/operations/README.md @@ -0,0 +1,15 @@ +# Operations + +Procedures that are executed by hand on an instance, not by the application. + +They are not part of the product: they describe how to do a one-off operation +on a running database, with the exact statements to run and what to check +between them. + +Each procedure lives here while the operation is alive. When the operation is +done, the file is either deleted or marked as historical, so the folder does +not grow with procedures that nobody will run again. + +| Procedure | Date | +|---|---| +| [Partition `file_change` by `file_id`](file-change-partition.md) | 2026-10 | diff --git a/operations/file-change-partition.md b/operations/file-change-partition.md new file mode 100644 index 0000000000..6c8f2e3d42 --- /dev/null +++ b/operations/file-change-partition.md @@ -0,0 +1,781 @@ +# Partition `file_change` by `file_id` + +Online procedure to turn `file_change` into a table partitioned by +`HASH (file_id)` with 16 partitions, aligned with the ones of `file_data`. + +- Migration: `backend/src/app/migrations/sql/0157-partition-file-change.sql` +- Issue: + +The result has to be **exactly** the schema of the migration, because on a big +instance the migration is marked as applied and never runs. This procedure +runs the same statements by hand. + +## The state of PRO when this was written + +2026-10-22: PostgreSQL 16.3, database 3.1 TB, `file_change` 683 GB (10 GB of +heap, 669 GB of TOAST, 3.1 GB of indexes), 1,135,255 rows. + +The legacy data is already migrated: the `data` column of `file_change` is +empty and the blobs live in `file_data` (1.56 TB, already partitioned in 16). +What is left in the TOAST is the xlog of the revisions, 4.4 kB on average, so +roughly 2.5 GB of live data against 669 GB on disk: **almost all of the TOAST +is dead space**, the legacy blobs that were emptied plus the churn of the +updates. + +So the copy carries about 13 GB (the heap plus the live TOAST) and writes a +table of roughly 7 GB. The disk comes back when the legacy table is dropped, +days later: some 675 GB. The copy itself tells the real numbers; these are +the estimates behind it. + +These numbers describe that instance and nothing else; the procedure does not +depend on them. + +## What changes + +- `file_change` is partitioned by `HASH (file_id)` with 16 partitions, the + same bounds as `file_data`, so the join between them is resolved partition by + partition. +- The primary key goes from `(id)` to `(file_id, id)`: PostgreSQL requires the + partition key in the primary key, and the application already filters by + `file_id` everywhere. +- The table has 16 columns: `data`, `data_ref_id` and `data_backend` are gone + (the legacy file data backend that still wrote them no longer exists) and + `kind` appears (1 = snapshot, 2 = revision). +- The old table is kept as `file_change_legacy`, the way back if something + goes wrong, and dropped days later. +- Almost all of the size of the table is dead space, so the table shrinks + from 683 GB to some 7 GB. The disk comes back when the legacy table is + dropped: some 675 GB, and the database goes from 3.1 TB to about 2.4 TB. + +## Why this mechanism + +`ATTACH` as a `DEFAULT` partition is not possible: the 16 hash siblings cover +the whole hash space, so the constraint of a `DEFAULT` partition could never +hold, and the `ATTACH` fails when it validates the 1.25 M rows. A `UNION` +view with triggers, dual writes in the application and logical replication +were discarded too. + +What is left is a copy in phases, driven by the `updated_at` watermark, and a +cutover of minutes: plain SQL, resumable, no new infrastructure. The heap is +10 GB, so a delta pass without an index scans 10 GB and only detoastes the +rows that changed. + +--- + +## Phase 0 — Before anything + +### 0.1 Mark the migration as applied + +The runner applies at startup every migration that is not in the `migrations` +table. Without this, it would copy the whole table inside the transaction of the +startup. + +```sql +INSERT INTO migrations (module, step) +VALUES ('main', '0157-partition-file-change'); +``` + +The module is `main` and the step has no `.sql` (see +`backend/src/app/util/migrations.clj`). + +### 0.2 Add the `kind` column to the current table + +Until the cutover, `file_change` is the current table, and the new code writes +`kind` on every insert: without the column the backend does not start. + +```sql +ALTER TABLE file_change ADD COLUMN IF NOT EXISTS kind smallint; +``` + +Nullable and without a default on purpose: it is a metadata-only operation +(it does not rewrite a single row of the 683 GB), the old code keeps inserting +rows, and the values of the rows that are already there are computed by the +copy from their `label`. + +### 0.3 Deploy the code + +The `updated_at` watermark has to be alive **before** the bulk copy: the new +code updates it on every mutation (locks, soft deletes), the old one does +not. Rows changed between the bulk and a delta pass would be lost. + +```sql +SELECT max(updated_at) FROM file_change; +``` + +### 0.4 Index for the delta passes + +The delta passes filter by `updated_at`. Without an index on that column every +pass scans the whole heap (10 GB); with it, it is an index range that only +touches the rows that changed. + +```sql +-- CONCURRENTLY: it does not block the writes. It cannot run inside a +-- transaction: run it alone. +CREATE INDEX CONCURRENTLY IF NOT EXISTS file_change__updated_at__idx + ON file_change (updated_at); +``` + +- The build reads the heap once (10 GB sequential) and writes ~25 MB: a few + minutes, not hours (an index on a column without TOAST does not touch the + TOAST). +- If it is interrupted, the index is left `INVALID` and has to be dropped and + built again: + +```sql +SELECT i.indisvalid + FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid + WHERE c.relname = 'file_change__updated_at__idx'; +``` + +- The index goes with the old table: at the cutover it is renamed to + `file_change__updated_at__idx_legacy` and disappears with it. The new table + does not carry it (nothing queries by `updated_at`; the GC uses + `deleted_at`). + +### 0.5 Check the environment + +```sql +-- Space: peak estimate 683 GB (the old table, it does not grow) + ~10 GB +-- (the new table) + WAL ~15 GB during the bulk +SELECT pg_size_pretty(pg_database_size(current_database())); + +-- The WAL must not run out of room (note the free space of the mount point): +-- df -h + +-- Nothing must reference file_change (0 rows expected): +SELECT conrelid::regclass FROM pg_constraint + WHERE confrelid = 'file_change'::regclass; + +-- There must be no triggers on file_change: +SELECT tgname FROM pg_trigger + WHERE tgrelid = 'file_change'::regclass AND NOT tgisinternal; + +-- Replication slots: the bulk generates some 15 GB of WAL. +SELECT slot_name, active, pg_size_pretty(pg_wal_lsn_diff( + pg_current_wal_lsn(), restart_lsn)) AS retained_wal + FROM pg_replication_slots; + +-- The legacy blobs of the snapshots have to be in `file_data` before the +-- cutover: the new table does not carry the `data` column. This number has +-- to be 0. +SELECT count(*) AS snapshots_without_data + FROM file_change AS fc + WHERE fc.data IS NOT NULL + AND fc.label IS NOT NULL + AND NOT EXISTS (SELECT 1 FROM file_data AS fd + WHERE fd.file_id = fc.file_id + AND fd.id = fc.id + AND fd.type = 'snapshot'); + +-- The revisions with legacy data are not migrated (they are the ephemeral +-- changes, out of scope). This number is informative: their data stays in +-- the legacy table, which is the way back. +SELECT count(*) AS revisions_with_data + FROM file_change + WHERE data IS NOT NULL + AND label IS NULL; +``` + +- If the first number is not 0, the blobs of those snapshots have to be + copied to `file_data` before starting (the same statement the migration + uses): + +```sql +INSERT INTO file_data (file_id, id, created_at, modified_at, deleted_at, + type, backend, metadata, data) +SELECT fc.file_id, fc.id, fc.created_at, fc.updated_at, fc.deleted_at, + 'snapshot', 'db', NULL, fc.data + FROM file_change AS fc + WHERE fc.data IS NOT NULL + AND fc.label IS NOT NULL +ON CONFLICT (file_id, id) DO NOTHING; +``` + +- Check that the backup / PITR is working before starting. +- Pick a low activity window for the cutover: a freeze of the writes to + `file_change` of a couple of minutes (the final delta inside the lock + copies only the rows changed since the bulk, so it is quick; the saves of + files during the freeze fail and the user retries, and the rest of the + database keeps working). The window has to cover the whole cutover, with + the backend and the workers stopped and started again (see 4.2). + +--- + +## Phase 1 — Create the partitioned table + +The new indexes and constraints carry a `_new` suffix because they live with +the ones of the old table until the cutover (the names live in the schema, +not in the table). At the cutover they are swapped for the canonical ones. +The partitions are the exception: they are created with their final name, +because their indexes are named after the partition and not after the +table. + +```sql +CREATE TABLE file_change_new ( + file_id uuid NOT NULL, + id uuid NOT NULL DEFAULT gen_random_uuid(), + created_at timestamptz NOT NULL DEFAULT now(), + session_id uuid, + revn bigint NOT NULL DEFAULT 0, + changes bytea, + profile_id uuid, + features text[], + label text, + version integer, + created_by text NOT NULL DEFAULT 'system', + updated_at timestamptz DEFAULT now(), + deleted_at timestamptz, + migrations text[], + locked_by uuid, + kind smallint NOT NULL, + CONSTRAINT file_change_new_kind_check CHECK (kind IN (1, 2)) +) PARTITION BY HASH (file_id); + +-- the columns keep the storage of PostgreSQL by default (migration +-- 0156-reset-pg-storage-defaults dropped the explicit EXTERNAL overrides) + +COMMENT ON COLUMN file_change_new.kind IS + 'The kind of the row: 1 is a snapshot (it has a label and its data lives in file_data) and 2 is a revision (it has no label and it holds the changes).'; +COMMENT ON COLUMN file_change_new.locked_by IS + 'Profile ID of user who has locked this version. Only the creator can lock/unlock their own versions. Locked versions cannot be deleted by others.'; + +-- 16 partitions, the same bounds as file_data. The name of the table is +-- temporary (the old one holds it until the cutover) but the name of the +-- partitions is not: it is the final one, so their indexes are born with +-- the canonical names (renaming a partition does not rename its indexes) +-- and the result is exactly the schema of the migration. +CREATE TABLE file_change_00 PARTITION OF file_change_new + FOR VALUES WITH (MODULUS 16, REMAINDER 0); +CREATE TABLE file_change_01 PARTITION OF file_change_new + FOR VALUES WITH (MODULUS 16, REMAINDER 1); +CREATE TABLE file_change_02 PARTITION OF file_change_new + FOR VALUES WITH (MODULUS 16, REMAINDER 2); +CREATE TABLE file_change_03 PARTITION OF file_change_new + FOR VALUES WITH (MODULUS 16, REMAINDER 3); +CREATE TABLE file_change_04 PARTITION OF file_change_new + FOR VALUES WITH (MODULUS 16, REMAINDER 4); +CREATE TABLE file_change_05 PARTITION OF file_change_new + FOR VALUES WITH (MODULUS 16, REMAINDER 5); +CREATE TABLE file_change_06 PARTITION OF file_change_new + FOR VALUES WITH (MODULUS 16, REMAINDER 6); +CREATE TABLE file_change_07 PARTITION OF file_change_new + FOR VALUES WITH (MODULUS 16, REMAINDER 7); +CREATE TABLE file_change_08 PARTITION OF file_change_new + FOR VALUES WITH (MODULUS 16, REMAINDER 8); +CREATE TABLE file_change_09 PARTITION OF file_change_new + FOR VALUES WITH (MODULUS 16, REMAINDER 9); +CREATE TABLE file_change_10 PARTITION OF file_change_new + FOR VALUES WITH (MODULUS 16, REMAINDER 10); +CREATE TABLE file_change_11 PARTITION OF file_change_new + FOR VALUES WITH (MODULUS 16, REMAINDER 11); +CREATE TABLE file_change_12 PARTITION OF file_change_new + FOR VALUES WITH (MODULUS 16, REMAINDER 12); +CREATE TABLE file_change_13 PARTITION OF file_change_new + FOR VALUES WITH (MODULUS 16, REMAINDER 13); +CREATE TABLE file_change_14 PARTITION OF file_change_new + FOR VALUES WITH (MODULUS 16, REMAINDER 14); +CREATE TABLE file_change_15 PARTITION OF file_change_new + FOR VALUES WITH (MODULUS 16, REMAINDER 15); + +-- The primary key includes the partition key (required on a partitioned table) +ALTER TABLE file_change_new ADD PRIMARY KEY (file_id, id); + +-- Foreign keys (no CREATE TABLE copies them) +ALTER TABLE file_change_new ADD FOREIGN KEY (file_id) + REFERENCES file(id) DEFERRABLE; +ALTER TABLE file_change_new ADD FOREIGN KEY (locked_by) + REFERENCES profile(id) ON DELETE SET NULL DEFERRABLE; +ALTER TABLE file_change_new ADD FOREIGN KEY (profile_id) + REFERENCES profile(id) ON DELETE SET NULL DEFERRABLE; + +-- Secondary indexes: the canonical names, with the _new suffix. There is no +-- index for the system snapshots: the old one filtered them by +-- `data IS NOT NULL`, and the column is gone. The new one could only filter +-- them by `deleted_at IS NULL`, and the system snapshots always carry a +-- deletion date in the future, so it would never match a row. +CREATE INDEX file_change_new__deleted_at__idx + ON file_change_new (deleted_at, file_id, id) WHERE deleted_at IS NOT NULL; +CREATE INDEX file_change_new__file_id__revn__idx + ON file_change_new (file_id, revn); +CREATE INDEX file_change_new__locked_by__idx + ON file_change_new (locked_by) WHERE locked_by IS NOT NULL; +CREATE INDEX file_change_new__profile_id__idx + ON file_change_new (profile_id) WHERE profile_id IS NOT NULL; +CREATE INDEX file_change_new__user_snapshots__idx + ON file_change_new (file_id) + WHERE label IS NOT NULL AND created_by = 'user' AND deleted_at IS NULL; +``` + +The `deleted_at` index carries `file_id` (the primary key is `(file_id, id)`): +the objects GC needs the `file_id` to lock by pair, and with the index it +reads the rows it has to delete without touching the table. + +```sql +SELECT count(*) AS partitions FROM pg_partition_tree('file_change_new') t + WHERE t.isleaf; +-- 16 +``` + +--- + +## Phase 2 — Bulk copy + +Copies every row (including the soft deleted ones: the GC manages them the +same way in the new table). The `kind` is computed from the `label`: a row +with a label is a snapshot, a row without one is a revision. + +The pointer of the copy lives in the database, not in the terminal: a batch +can be interrupted and resumed without losing anything. + +```sql +CREATE TABLE IF NOT EXISTS file_change_copy_state ( + name text PRIMARY KEY, + last_id uuid +); + +INSERT INTO file_change_copy_state (name, last_id) +VALUES ('bulk', '00000000-0000-0000-0000-000000000000') +ON CONFLICT (name) DO NOTHING; +``` + +One batch (idempotent). It returns how many rows it copied: + +```sql +WITH batch AS MATERIALIZED ( + SELECT id FROM file_change + WHERE id > (SELECT last_id FROM file_change_copy_state WHERE name = 'bulk') + ORDER BY id + LIMIT 10000 +), +copied AS ( + INSERT INTO file_change_new (id, file_id, created_at, session_id, revn, changes, + profile_id, features, label, version, created_by, + updated_at, deleted_at, migrations, locked_by, kind) + SELECT id, file_id, created_at, session_id, revn, changes, + profile_id, features, label, version, created_by, + updated_at, deleted_at, migrations, locked_by, + CASE WHEN label IS NOT NULL THEN 1 ELSE 2 END + FROM file_change + WHERE id IN (SELECT id FROM batch) + ON CONFLICT (file_id, id) DO UPDATE + SET created_at = EXCLUDED.created_at, + session_id = EXCLUDED.session_id, + revn = EXCLUDED.revn, + changes = EXCLUDED.changes, + profile_id = EXCLUDED.profile_id, + features = EXCLUDED.features, + label = EXCLUDED.label, + version = EXCLUDED.version, + created_by = EXCLUDED.created_by, + updated_at = EXCLUDED.updated_at, + deleted_at = EXCLUDED.deleted_at, + migrations = EXCLUDED.migrations, + locked_by = EXCLUDED.locked_by, + kind = EXCLUDED.kind + RETURNING 1 +) +UPDATE file_change_copy_state + SET last_id = (SELECT id FROM batch ORDER BY id DESC LIMIT 1) + WHERE name = 'bulk' + AND EXISTS (SELECT 1 FROM copied) +RETURNING (SELECT count(*) FROM copied) AS copied; +``` + +Repeat until it returns `0`: + +```bash +until [ "$(psql -At -f batch-bulk.sql)" = "0" ]; do sleep 0.3; done +``` + +- The pointer is a `uuid` (random order): the pagination is stable but it does + not group by physical position. The total read is the 10 GB of the heap, in + a scattered order. +- **Estimated duration**: minutes, not hours. The bulk reads about 13 GB (the + heap plus the live TOAST, the dead space of the TOAST is never read) and + writes a table of roughly 7 GB. With 1.14 M rows and batches of 10 K it is + some 114 batches. +- Adjust the throttle (`sleep`) if the streaming replica falls behind or the + backend sees latency. Every batch detoastes its share of the TOAST: the cost + is I/O, not CPU. +- Watch: replica lag, `pg_stat_activity` (locks between the bulk and + autovacuum), the space of `pg_wal` and of the data dir. + +```sql +SELECT (SELECT count(*) FROM file_change) AS old_rows, + (SELECT count(*) FROM file_change_new) AS new_rows; +``` + +They match except for the rows written during the bulk (the delta passes pick +them up). + +--- + +## Phase 3 — Delta passes + +Every pass copies the rows modified since the bulk started. The window is a +constant in the script (not the output of the previous pass): the hour of +margin absorbs the clock drift between the application and the database, and +the drift between batches. + +```sql +-- :bulk_start = the time the bulk started, minus one hour +INSERT INTO file_change_new (id, file_id, created_at, session_id, revn, changes, + profile_id, features, label, version, created_by, + updated_at, deleted_at, migrations, locked_by, kind) +SELECT id, file_id, created_at, session_id, revn, changes, + profile_id, features, label, version, created_by, + updated_at, deleted_at, migrations, locked_by, + CASE WHEN label IS NOT NULL THEN 1 ELSE 2 END + FROM file_change + WHERE updated_at > TIMESTAMPTZ '2026-10-25 08:00:00+02' -- :bulk_start +ON CONFLICT (file_id, id) DO UPDATE + SET created_at = EXCLUDED.created_at, + session_id = EXCLUDED.session_id, + revn = EXCLUDED.revn, + changes = EXCLUDED.changes, + profile_id = EXCLUDED.profile_id, + features = EXCLUDED.features, + label = EXCLUDED.label, + version = EXCLUDED.version, + created_by = EXCLUDED.created_by, + updated_at = EXCLUDED.updated_at, + deleted_at = EXCLUDED.deleted_at, + migrations = EXCLUDED.migrations, + locked_by = EXCLUDED.locked_by, + kind = EXCLUDED.kind; +``` + +- Repeat every 1-6 h (depending on the activity) until the pass copies very + few rows. With the index of phase 0.4 the pass is an index range over the + rows changed since the mark, so it takes seconds; without it, it scans the + 10 GB of the heap. To see how many are left: + +```sql +SELECT count(*) FROM file_change WHERE updated_at > TIMESTAMPTZ '2026-10-25 08:00:00+02'; +``` + +- The objects GC can keep running: its soft deletes are covered by the + watermark, and its hard deletes leave a row with `deleted_at` in the new + table that the GC itself cleans up later. The hard delete of a **file** is + blocked while the old table (or the new one) has rows for it, because the + foreign key on `file_id` is `NO ACTION`: the file stays in the trash and + the GC picks it up in the next run. **Do not run the cutover at the same + time as a big batch of the GC.** + +--- + +## Phase 4 — Cutover (a freeze of minutes) + +### 4.1 Before the lock + +- One last delta pass. +- Prepare and rehearse the cutover script in staging. + +### 4.2 The cutover (one transaction, a brief exclusive lock) + +- **Stop the backend and the workers before the transaction, and start them + again after it.** The pool keeps server-side prepared statements, and a + prepared statement holds the OID of the table it was prepared with: + renaming the table does not change that OID. A connection that had already + run `insert into file_change` five times (the default threshold of the + driver) would keep writing to `file_change_legacy` after the cutover, and + the pool recycles its connections every 30 minutes, so the window of + silent writes is long. +- Run the transaction. +- Start the backend and the workers again. + +```sql +BEGIN; + +LOCK TABLE file_change IN ACCESS EXCLUSIVE MODE; + +-- The final delta: the rows modified since the bulk started +INSERT INTO file_change_new (id, file_id, created_at, session_id, revn, changes, + profile_id, features, label, version, created_by, + updated_at, deleted_at, migrations, locked_by, kind) +SELECT id, file_id, created_at, session_id, revn, changes, + profile_id, features, label, version, created_by, + updated_at, deleted_at, migrations, locked_by, + CASE WHEN label IS NOT NULL THEN 1 ELSE 2 END + FROM file_change + WHERE updated_at > TIMESTAMPTZ '2026-10-25 08:00:00+02' +ON CONFLICT (file_id, id) DO UPDATE + SET updated_at = EXCLUDED.updated_at, + deleted_at = EXCLUDED.deleted_at, + label = EXCLUDED.label, + locked_by = EXCLUDED.locked_by, + created_by = EXCLUDED.created_by, + changes = EXCLUDED.changes, + kind = EXCLUDED.kind; + +-- Check inside the lock +SELECT (SELECT count(*) FROM file_change) AS old_rows, + (SELECT count(*) FROM file_change_new) AS new_rows; + +-- The old table frees the canonical names (the same names the migration +-- 0157 uses, with the _legacy suffix) +ALTER TABLE file_change RENAME CONSTRAINT file_change_pkey + TO file_change_pkey_legacy; +ALTER TABLE file_change RENAME CONSTRAINT file_change_file_id_fkey + TO file_change_file_id_fkey_legacy; +ALTER TABLE file_change RENAME CONSTRAINT file_change_locked_by_fkey + TO file_change_locked_by_fkey_legacy; +ALTER TABLE file_change RENAME CONSTRAINT file_change_profile_id_fkey + TO file_change_profile_id_fkey_legacy; + +ALTER INDEX file_change__data_ref_id__idx + RENAME TO file_change__data_ref_id__idx_legacy; +ALTER INDEX file_change__deleted_at__idx + RENAME TO file_change__deleted_at__idx_legacy; +ALTER INDEX file_change__file_id__revn__idx + RENAME TO file_change__file_id__revn__idx_legacy; +ALTER INDEX file_change__locked_by__idx + RENAME TO file_change__locked_by__idx_legacy; +ALTER INDEX file_change__profile_id__idx + RENAME TO file_change__profile_id__idx_legacy; +ALTER INDEX file_change__system_snapshots__idx + RENAME TO file_change__system_snapshots__idx_legacy; +ALTER INDEX IF EXISTS file_change__user_snapshots__idx + RENAME TO file_change__user_snapshots__idx_legacy; +ALTER INDEX IF EXISTS file_change__updated_at__idx + RENAME TO file_change__updated_at__idx_legacy; + +-- And the new table takes them +ALTER TABLE file_change_new RENAME CONSTRAINT file_change_new_pkey + TO file_change_pkey; +ALTER TABLE file_change_new RENAME CONSTRAINT file_change_new_file_id_fkey + TO file_change_file_id_fkey; +ALTER TABLE file_change_new RENAME CONSTRAINT file_change_new_locked_by_fkey + TO file_change_locked_by_fkey; +ALTER TABLE file_change_new RENAME CONSTRAINT file_change_new_profile_id_fkey + TO file_change_profile_id_fkey; +ALTER TABLE file_change_new RENAME CONSTRAINT file_change_new_kind_check + TO file_change_kind_check; + +ALTER INDEX file_change_new__deleted_at__idx + RENAME TO file_change__deleted_at__idx; +ALTER INDEX file_change_new__file_id__revn__idx + RENAME TO file_change__file_id__revn__idx; +ALTER INDEX file_change_new__locked_by__idx + RENAME TO file_change__locked_by__idx; +ALTER INDEX file_change_new__profile_id__idx + RENAME TO file_change__profile_id__idx; +ALTER INDEX file_change_new__user_snapshots__idx + RENAME TO file_change__user_snapshots__idx; + +-- And the tables are swapped. The partitions and their indexes keep the +-- names they were born with (they are named after the partition, not after +-- the table), so there is nothing else to rename. +ALTER TABLE file_change RENAME TO file_change_legacy; +ALTER TABLE file_change_new RENAME TO file_change; + +-- The foreign key of the legacy table goes away: it is `NO ACTION`, so +-- while the legacy table has rows for a file, the objects GC cannot delete +-- that file (it deletes the rows of the file first, and the legacy ones are +-- not its business). The legacy table is frozen, and the rollback adds the +-- constraint back. +ALTER TABLE file_change_legacy + DROP CONSTRAINT file_change_file_id_fkey_legacy; + +COMMIT; + +ANALYZE file_change; +``` + +- `file_change_legacy` keeps its data and its indexes (with the `_legacy` + suffix): it stays frozen as the way back. Its foreign key on `file_id` is + dropped, so the objects GC can delete the files again. +- The writes to `file_change` during the lock fail: they are seconds-minutes, + the affected saves are retried. +- The objects GC and the four RPC commands keep working: same table name and + same index names after the rename. +- Once the backend is up again, check that the saves go to the partitioned + table and that nothing went to the table that is going away: + +```sql +-- the rows written after the cutover: this one grows with every save +SELECT count(*) FROM file_change + WHERE updated_at > TIMESTAMPTZ '2026-10-25 08:00:00+02' -- :cutover + +-- the rows that went to the legacy table: this one has to stay at 0 +SELECT count(*) FROM file_change_legacy + WHERE updated_at > TIMESTAMPTZ '2026-10-25 08:00:00+02'; -- :cutover +``` + +A number above zero in the second query means that some connection was +still writing with a prepared statement of the old table: those rows are +lost when the legacy table is dropped, so they have to be copied over +before that. + +### 4.3 Rollback + +- **Before the `COMMIT`**: `ROLLBACK` and that is it. +- **After the cutover**: the new writes go to the partitioned table. To revert, + rename the tables back and copy the delta the other way + (`INSERT INTO file_change_legacy ... WHERE updated_at > :cutover ON CONFLICT + (id) DO UPDATE`). The foreign key on `file_id` was dropped at the cutover, + so it goes back with `NOT VALID` (the existing rows are not checked) and it + is validated later: + +```sql +ALTER TABLE file_change_legacy + ADD CONSTRAINT file_change_file_id_fkey + FOREIGN KEY (file_id) REFERENCES file(id) DEFERRABLE NOT VALID; +``` + + Reasonable window: only if there is a serious defect in the first hours. + After that, treat any problem the normal way (the partitioned table is the + good state). + +--- + +## Phase 5 — After the cutover + +1. Check that the access paths prune to one partition (when the planner + chooses an index scan): + +```sql +EXPLAIN SELECT * FROM file_change + WHERE file_id = '00000000-0000-0000-0000-000000000000' + AND id = '00000000-0000-0000-0000-000000000000'; +``` + +It has to show a single partition (`file_change_NN`), not an `Append` of 16. + +2. Check that the schema is the one of the migration `0157`. Every row of this + query has to show `got = expected`: + +```sql +SELECT 'columns' AS what, count(*) AS got, 16 AS expected + FROM pg_attribute + WHERE attrelid = 'file_change'::regclass AND attnum > 0 AND NOT attisdropped +UNION ALL +SELECT 'partitions', count(*), 16 + FROM pg_partition_tree('file_change') t WHERE t.isleaf +UNION ALL +SELECT 'primary key', count(*), 1 + FROM pg_constraint + WHERE conrelid = 'file_change'::regclass AND contype = 'p' +UNION ALL +SELECT 'foreign keys', count(*), 3 + FROM pg_constraint + WHERE conrelid = 'file_change'::regclass AND contype = 'f' +UNION ALL +SELECT 'check constraints', count(*), 1 + FROM pg_constraint + WHERE conrelid = 'file_change'::regclass AND contype = 'c' +UNION ALL +SELECT 'indexes', count(*), 6 + FROM pg_indexes + WHERE tablename = 'file_change' AND schemaname = current_schema() +UNION ALL +SELECT 'partitions out of name', count(*), 0 + FROM pg_class + WHERE relkind = 'r' AND relname ~ '^file_change_new[0-9]' +UNION ALL +SELECT 'child indexes out of name', count(*), 0 + FROM pg_class c + JOIN pg_index i ON i.indexrelid = c.oid + JOIN pg_class p ON p.oid = i.indrelid + WHERE p.relname ~ '^file_change_[0-9]{2}$' + AND c.relname NOT LIKE p.relname || '_%'; +``` + + And the names, to compare them with the migration: + +```sql +SELECT a.attname AS column + FROM pg_attribute a + WHERE a.attrelid = 'file_change'::regclass AND a.attnum > 0 + AND NOT a.attisdropped + ORDER BY a.attnum; + +SELECT indexname FROM pg_indexes + WHERE tablename = 'file_change' AND schemaname = current_schema() + ORDER BY indexname; + +-- the partitions, with their name +SELECT c.relname AS partition + FROM pg_class c + JOIN pg_partition_tree('file_change') t ON t.relid = c.oid + WHERE t.isleaf + ORDER BY c.relname; + +-- the indexes of the partitions: their name starts with the partition +SELECT c.relname AS child_index + FROM pg_class c + JOIN pg_index i ON i.indexrelid = c.oid + JOIN pg_class p ON p.oid = i.indrelid + WHERE p.relname ~ '^file_change_[0-9]{2}$' + ORDER BY c.relname; +``` + +3. The partition-wise join is already on: the pool of the backend sets + `enable_partitionwise_join = on` on every connection (it is in the code, + there is nothing to change in the database role). + +4. Autovacuum: the 16 partitions have TOASTs of ~18 GB; with + `autovacuum_vacuum_scale_factor = 0.2` the threshold is reached earlier + and in steps. Nothing to change, just watch `last_autovacuum` of the + partitions the first week. + +5. The aggregations that do not filter by `file_id`. Measured with 1.2 M rows, + 60 K files and 16 partitions: + + | query | plain table (PRO today) | partitioned | | + |---|---|---|---| + | team snapshot quota | 186 ms | 228 ms | +22% | + | team snapshot quota, as rewritten in the code | 204 ms | **130 ms** | -30% | + | telemetry `count(*)` | 90 ms | 129 ms | +43% | + | telemetry `count(distinct file_id)` | 116 ms | 113 ms | 0% | + + - The team quota is already fixed in the code: it resolves the `file_id` of + the team first, which lets the planner prune the partitions when it + chooses an index scan (with a sequential scan it reads all of them). + - The telemetry does not get worse in practice: both metrics scan the whole + heap (10 GB in PRO), the same before and after. The +43% of the bench is + the cost per partition on a table that fits in the cache. To remove that + scan, the index `file_change__created_at__idx` turns both metrics into an + index scan over the rows of the day, at the cost of one more index on the + biggest table. Decide it before phase 1, the index has to exist in both + tables. + +6. Drop the helper table of the copy and, days later, the old table: + +```sql +DROP TABLE IF EXISTS file_change_copy_state; +DROP TABLE file_change_legacy; -- out of hours: it generates WAL by + -- dropping the pages, let it roll +``` + +That is where the dead space comes back: some 675 GB, and the database goes +from 3.1 TB to about 2.4 TB. + +--- + +## Risks + +| Risk | Mitigation | +|---|---| +| Massive WAL during the bulk (some 15 GB) | Throttle between batches; watch `pg_wal`, the replication slots and the replica lag | +| Latency seen by the application during the bulk | Throttle; low traffic hours; the bulk is I/O and does not block writes | +| `updated_at` not reliable in the rows changed before the deploy | The deploy goes before the bulk (0.3) and the delta passes cover from the start of the bulk | +| Lock/unlock/soft delete during the passes | Covered by the watermark; the cutover includes the final delta inside the lock | +| Hard delete of the GC between passes | The row is left with `deleted_at` in the new table; the GC cleans it up later | +| Orphan rows if a `file` is deleted halfway | The foreign key on `file_id` is `NO ACTION`: the GC deletes the rows of the file before the file, so a chunk that fails leaves the file in the trash and the next run deletes it | +| The files in the trash cannot be deleted while the legacy table has rows for them | The foreign key of the legacy table is dropped at the cutover; until then the files wait in the trash | +| Saves failing during the lock of the cutover | Freeze of minutes in a low traffic window; manual retries; the rollback is documented | +| Planner regression after the cutover | Immediate `ANALYZE` + the pruning `EXPLAIN`; the pool already sets the GUC | +| Clock drift between the application and the database | The delta window starts an hour before the bulk; the margin absorbs the drift | + +## Success metrics + +- After the cutover: `file_change` (partitioned) of some 7 GB, against 683 GB. +- After the `DROP` of the old table: the database goes from 3.1 TB to about + 2.4 TB. +- The access paths that filter by `file_id` (the lookup of a snapshot, the + quota of a file and the lock of the GC by pair) prune to one partition when + the planner chooses an index scan (`EXPLAIN`). The quota of the team and + the discovery of the GC do not filter by a single file: they read all the + 16 partitions. +- No visible change for the user.