diff --git a/.serena/memories/backend/storage.md b/.serena/memories/backend/storage.md index 529fcd1cc8..cfd16f2976 100644 --- a/.serena/memories/backend/storage.md +++ b/.serena/memories/backend/storage.md @@ -131,6 +131,21 @@ Since `put-object!` uses backend-specific operations (`impl/resolve-backend` + ` - Writes always resolve the backend through `fdata/default-backend` (`cf/get :file-data-backend`), so post-offload edits land back in hot (`db`) storage without extra code; reads stay transparent through `fdata/resolve-file-data` + `decode-file-data` + `realize`. - Deleting an offloaded row touches its storage object first (`objects-gc/delete-file-data-chunk` via `fdata/process-metadata`), so `storage-gc-touched` reclaims the blob with the usual delay; restoring a file unmarks the referenced objects. +## 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). +- The automatic migration (`0158-partition-file-change`) refuses to run when `file_change` is above 10 GiB total size or 2M rows (`pg_total_relation_size` + `pg_class.reltuples`, catalog reads). It analyzes the table first, because `reltuples` is -1 without statistics. It raises with the path of `operations/2.19.0-file-change-partition.md`, because the copy happens inside the migration transaction and a big instance would restart in a loop. Below the guard it raises `statement_timeout` to 30 minutes for the copy and the index builds, and analyzes the new partitions at the end, after the copy and the indexes, so the planner does not start with empty statistics. +- 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`. From 2.19 the four snapshot commands (`update-file-snapshot`, `delete-file-snapshot`, `lock-file-snapshot`, `unlock-file-snapshot`) require the `file-id` param, because they use it to prune the partition; without it they answer `:params-validation`. +- 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. +- Every write of the application to a row of `file_change` moves `updated_at` to the moment of the write (`lock-by!`, `unlock!`, the snapshot rename and delete, `delete-object`, the restore of a file, and the `legacy-db` snapshot write of `fdata`). The actions of the foreign keys (`ON DELETE SET NULL` on `locked_by` and `profile_id`) change the row without touching it; during the partition both tables carry the same keys, so both receive them. The runbook of the partition uses it as the watermark of its delta passes: a write of the application that leaves it behind is lost at the cutover. +- `file_change_legacy` is the way back of the partition: it keeps the rows and the indexes of the old table. A new instance (empty table) does not keep it. On an upgraded instance the table stays until the operator drops it by hand once the rollback window is over (`DROP TABLE file_change_legacy;`, days later, see `operations/2.19.0-file-change-partition.md`); nothing drops it automatically. +- 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 keeps the `data` column: the `legacy-db` backend is still supported (migration `0157-migrate-legacy-file-data` moved the existing blobs, but the backend keeps reading and writing that column). It has no `data_ref_id` or `data_backend` columns: nothing read them since the file storage refactor. +- It has no index for the system snapshots. The old one filtered them by `data IS NOT NULL`, and with the payload living in `file_data` the column no longer tells snapshots apart. 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. +- The `objects-gc` purge groups each chunk by `file_id` and locks it with `file_id = ?` and `id = ANY(?::uuid[])`: a tuple `IN ((file_id, id), ...)` gives the planner no constant partition key, so it scans all 16 partitions in every chunk. Each group is one `FOR UPDATE SKIP LOCKED` query inside the chunk transaction. + ## Metrics - `bucket` is always the Penpot logical bucket (object metadata), never an S3 bucket. Unknown/absent buckets are labeled `"unknown"`. diff --git a/.serena/memories/clojure/idioms.md b/.serena/memories/clojure/idioms.md index 7e81fba7ac..9cac57d17b 100644 --- a/.serena/memories/clojure/idioms.md +++ b/.serena/memories/clojure/idioms.md @@ -10,4 +10,5 @@ Behaviors confirmed against the language/stdlib — do not re-derive from assump - `^: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. - Penpot extends `clojure.core/Inst` in `app.common.time` ( JVM-only section), so `inst-ms` also accepts `Duration` (returns `.toMillis`, NOT epoch millis), `FileTime` and `OffsetDateTime`. It only works where that namespace is loaded; on a bare `Duration`, prefer explicit `.toMillis`, which reads as what it is (an amount, not an instant). +- 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. With unary calls both nest the same way (`(-> x (f) (g))` and `(->> x (f) (g))` are both `(g (f x))`): what changes between them is the position inside each call, not the order of the steps. 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/backend/src/app/db.clj b/backend/src/app/db.clj index c9998b49b8..23b5ddad7a 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 d18da11ea6..da8b662514 100644 --- a/backend/src/app/features/fdata.clj +++ b/backend/src/app/features/fdata.clj @@ -205,8 +205,11 @@ (db/delete! cfg :file-data {:id id :file-id file-id :type "snapshot"} {::db/return-keys false}) + ;; every mutation of `file_change` moves `updated_at`: the partition + ;; runbook uses it as the watermark of the delta passes (db/update! cfg :file-change - {:data data} + {:data data + :updated-at (ct/now)} {:file-id file-id :id id} {::db/return-keys false})) diff --git a/backend/src/app/features/file_changes.clj b/backend/src/app/features/file_changes.clj new file mode 100644 index 0000000000..f01b7bfe6c --- /dev/null +++ b/backend/src/app/features/file_changes.clj @@ -0,0 +1,21 @@ +;; 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` (old `legacy-db` payloads live in + `file_change.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 2370758d28..7c1818b743 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.jobs :as jobs] [app.storage :as sto] [app.util.blob :as blob] @@ -74,57 +75,71 @@ 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 " AND c.file_id = ? - AND 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")) + AND c.id = ? " + sql:snapshot-visible)) (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 " @@ -267,6 +282,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) @@ -313,7 +329,10 @@ (-> (db/update! cfg :file-change {:label label :created-by "user" - :updated-at modified-at + ;; the partition runbook uses updated_at as the + ;; watermark of its delta passes: a rename has to move + ;; it, unlike file_data.modified_at above + :updated-at (ct/now) :deleted-at nil} {:file-id file-id :id id} @@ -346,7 +365,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)) @@ -426,7 +445,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) @@ -435,19 +455,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/migrations.clj b/backend/src/app/migrations.clj index 104222cc00..39ef9b5b71 100644 --- a/backend/src/app/migrations.clj +++ b/backend/src/app/migrations.clj @@ -532,7 +532,10 @@ :fn (mg/resource "app/migrations/sql/0156-reset-pg-storage-defaults.sql")} {:name "0157-add-audit-log-pending-table" - :fn (mg/resource "app/migrations/sql/0157-add-audit-log-pending-table.sql")}]) + :fn (mg/resource "app/migrations/sql/0157-add-audit-log-pending-table.sql")} + + {:name "0158-partition-file-change" + :fn (mg/resource "app/migrations/sql/0158-partition-file-change.sql")}]) (defn apply-migrations! [pool name migrations] diff --git a/backend/src/app/migrations/sql/0158-partition-file-change.sql b/backend/src/app/migrations/sql/0158-partition-file-change.sql new file mode 100644 index 0000000000..d9461d40e4 --- /dev/null +++ b/backend/src/app/migrations/sql/0158-partition-file-change.sql @@ -0,0 +1,251 @@ +-- 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), the `data` column is kept as-is (the `legacy-db` +-- file data backend reads old payloads from it: migration +-- 0157-migrate-legacy-file-data moved the existing blobs), 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 +-- rows of the table happens inside the transaction of the migration, and +-- the pool sets a 5 minute statement timeout. On those instances mark the +-- migration as applied and partition the table by hand, with the procedure +-- of `operations/2.19.0-file-change-partition.md`. +-- +-- It assumes the service is stopped (a stop-start upgrade): it renames the +-- table, and a process of the previous version would keep writing to +-- `file_change_legacy` through its server-side prepared statements, which +-- hold the OID of the table and survive the rename. A rolling deployment is +-- the scenario of the runbook, not of this path. +-- +-- The GRANTs of the table are not carried over: this creates a new table, +-- so an instance with custom grants on `file_change` has to re-apply them +-- after the upgrade. + +-- 0. The guard for the big instances, before anything is touched: a copy +-- that does not fit keeps the backend down in a restart loop, and the +-- operator has to learn it from a clear message instead of from a +-- `statement timeout`. Both numbers are catalog reads, not table scans. +-- +-- The row count comes from `pg_class.reltuples`, which is -1 on a +-- relation that was never analyzed (a restore without `--analyze`, for +-- instance). Analyze first so the number is real; on a table that fails +-- the guard this costs seconds per boot attempt, not a table scan. +-- +-- The copy, the index builds and the `ANALYZE` of an instance that +-- passes the guard can take longer than the 5 minutes of the pool. The +-- timeout is bounded, so a wrong estimate still fails instead of hanging +-- the boot, and it covers the `ANALYZE`: on a cold, large table it can +-- outlast the pool too, and the guard has to be the one that speaks. +SET LOCAL statement_timeout = '30min'; + +ANALYZE file_change; + +DO $$ +BEGIN + IF pg_total_relation_size('file_change'::regclass) > 10::bigint * 1024 * 1024 * 1024 + OR (SELECT reltuples FROM pg_class WHERE oid = 'file_change'::regclass) > 2000000 + THEN + RAISE EXCEPTION + 'file_change is too large for the automatic partition migration: mark 0158 as applied and follow operations/2.19.0-file-change-partition.md'; + END IF; +END $$; + +-- 1. The legacy blobs of the snapshots were already moved to `file_data` +-- by migration 0157-migrate-legacy-file-data (on the big instances, by +-- hand, with its runbook), so there is nothing to copy here. The `data` +-- column travels with the row in step 4 below: the `legacy-db` backend +-- still reads old payloads from it. + +-- 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, + data bytea, + 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 +-- (`data_ref_id` and `data_backend`). +-- 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, data, 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, data, 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 with the payload living in `file_data` the +-- column no longer tells snapshots apart. 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 (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. Statistics for the new partitions. The `ANALYZE` of step 0 ran against +-- the old table, before the copy: without this one the planner starts +-- with empty statistics on the partitions and picks plans by guesswork +-- until autovacuum gets to them. The backend starts serving right after +-- this migration, so it is the worst moment to leave them unanalyzed. +ANALYZE file_change; + +-- 9. 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 c326fd11cf..daec5000c9 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -1401,7 +1401,9 @@ ;; 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 + ;; Every write moves `updated_at`: the delta passes copy the rows whose + ;; `updated_at` changed, so a restore that left it stale could be missed. + "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)") diff --git a/backend/src/app/rpc/commands/files_snapshot.clj b/backend/src/app/rpc/commands/files_snapshot.clj index ba27fb5186..778bb33d4d 100644 --- a/backend/src/app/rpc/commands/files_snapshot.clj +++ b/backend/src/app/rpc/commands/files_snapshot.clj @@ -52,6 +52,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} @@ -207,28 +208,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 and an unknown snapshot answers :snapshot-not-found"]] ::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 and an unknown snapshot answers :snapshot-not-found"]] ::sm/params schema:remove-file-snapshot ::db/transaction true} - [{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id id]}] - (let [snapshot (fsnap/get-minimal-snapshot conn id)] + [{:keys [::db/conn] :as cfg} {: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") @@ -256,14 +261,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 and an unknown snapshot answers :snapshot-not-found"]] ::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") @@ -291,18 +298,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 and an unknown snapshot answers :snapshot-not-found"]] ::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") @@ -329,4 +338,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 34902922bf..86e69be7f8 100644 --- a/backend/src/app/rpc/commands/files_update.clj +++ b/backend/src/app/rpc/commands/files_update.clj @@ -21,6 +21,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] @@ -334,6 +335,7 @@ (when repair (db/insert! conn :file-change {:id (uuid/next) + :kind fchanges/revision-kind :session-id repair-session-id :profile-id profile-id :created-at timestamp @@ -354,8 +356,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 change-created-at @@ -364,7 +371,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 ac79505d16..1086c2423e 100644 --- a/backend/src/app/rpc/quotes.clj +++ b/backend/src/app/rpc/quotes.clj @@ -423,6 +423,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 = ? @@ -454,11 +456,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..a732a34fc1 100644 --- a/backend/src/app/scripts/decode_file_data.clj +++ b/backend/src/app/scripts/decode_file_data.clj @@ -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/tasks/delete_object.clj b/backend/src/app/tasks/delete_object.clj index 87cacd7a53..b45e0d8ca2 100644 --- a/backend/src/app/tasks/delete_object.clj +++ b/backend/src/app/tasks/delete_object.clj @@ -53,7 +53,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}) @@ -93,7 +94,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 1fca7bf321..80f212a6b1 100644 --- a/backend/src/app/tasks/objects_gc.clj +++ b/backend/src/app/tasks/objects_gc.clj @@ -673,41 +673,48 @@ {: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[]) + "SELECT file_id, id, deleted_at FROM file_change + WHERE file_id = ? + AND id = ANY(?::uuid[]) 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 ::threshold]}] (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 threshold]) - (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)) + ;; The rows are locked per file, with a constant `file_id`, + ;; so the planner prunes to the partition that holds each + ;; one instead of scanning all of them for a tuple `IN`. + (->> (group-by :file-id rows) + (mapcat (fn [[file-id file-rows]] + (let [ids (db/create-array conn "uuid" (map :id file-rows))] + (db/exec! conn [sql:lock-file-change file-id ids threshold])))) + (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)) - (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))) + 0)))) (catch Throwable cause (l/err :obj "file-change" :hint "error on chunk deletion, skipping it" - :chunk ids + :chunk rows :cause cause) 0))) @@ -720,7 +727,10 @@ (+ total deleted))))] (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 threshold] {:fetch-size 5})))))) diff --git a/backend/test/backend_tests/db_test.clj b/backend/test/backend_tests/db_test.clj index aa6732ead5..b0f20cb256 100644 --- a/backend/test/backend_tests/db_test.clj +++ b/backend/test/backend_tests/db_test.clj @@ -80,3 +80,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/fdata_resolve_test.clj b/backend/test/backend_tests/fdata_resolve_test.clj new file mode 100644 index 0000000000..e9d777d900 --- /dev/null +++ b/backend/test/backend_tests/fdata_resolve_test.clj @@ -0,0 +1,42 @@ +;; 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.fdata-resolve-test + "Resolution of file data by backend (`app.features.fdata/resolve-file-data`). + + A row without a `file_data` twin carries no backend: the read queries mark + it with `coalesce(fd.backend, 'legacy-db')` and hand its blob over as + `:legacy-data`. The `legacy-db` backend is still supported, so that case + has to resolve to the blob instead of nil." + (:require + [app.features.fdata :as fdata] + [clojure.test :as t])) + +(t/deftest legacy-db-resolves-the-legacy-blob + (let [blob (vec (.getBytes "legacy-blob" "UTF-8")) + file {:backend "legacy-db" + :legacy-data (byte-array blob)} + out (fdata/resolve-file-data nil file)] + (t/is (= blob (vec (:data out)))) + (t/is (not (contains? out :legacy-data))))) + +(t/deftest missing-backend-defaults-to-legacy-db + ;; The queries leave `:backend` nil when the `LEFT JOIN` with `file_data` + ;; finds no twin; the dispatch default keeps that row on `legacy-db`. + (let [blob (vec (.getBytes "legacy-blob" "UTF-8")) + file {:legacy-data (byte-array blob)} + out (fdata/resolve-file-data nil file)] + (t/is (= blob (vec (:data out)))) + (t/is (not (contains? out :legacy-data))))) + +(t/deftest db-backend-drops-the-legacy-key + (let [data (vec (.getBytes "db-blob" "UTF-8")) + file {:backend "db" + :data (byte-array data) + :legacy-data (byte-array (vec (.getBytes "stale" "UTF-8")))} + out (fdata/resolve-file-data nil file)] + (t/is (= data (vec (:data out)))) + (t/is (not (contains? out :legacy-data))))) diff --git a/backend/test/backend_tests/fdata_upsert_test.clj b/backend/test/backend_tests/fdata_upsert_test.clj new file mode 100644 index 0000000000..62fd2913c2 --- /dev/null +++ b/backend/test/backend_tests/fdata_upsert_test.clj @@ -0,0 +1,55 @@ +;; 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 backend-tests.fdata-upsert-test + "Covers the invariants of `fdata/upsert!` that the partition runbook relies + on." + + (:require + [app.common.time :as ct] + [app.common.uuid :as uuid] + [app.db :as db] + [app.features.fdata :as fdata] + [app.features.file-changes :as fchanges] + [backend-tests.helpers :as th] + [clojure.test :as t])) + +(t/use-fixtures :once th/state-init) +(t/use-fixtures :each th/database-reset) + +(t/deftest legacy-db-snapshot-moves-the-updated-at-watermark + ;; The runbook of the partition picks the rows changed since the bulk copy + ;; with `updated_at`, so every mutation of `file_change` has to move it. The + ;; `legacy-db` snapshot path writes the payload into the row, and it used to + ;; leave the watermark behind. + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile)}) + id (uuid/next) + past (ct/in-past "2h") + payload (byte-array [1 2 3])] + + (th/db-insert! :file-change + {:id id + :file-id (:id file) + :revn 1 + :label "v1" + :kind fchanges/snapshot-kind + :updated-at past}) + + (t/is (true? (fdata/upsert! th/*system* + {:id id + :file-id (:id file) + :type "snapshot" + :backend "legacy-db" + :data payload}))) + + (let [row (th/db-get :file-change {:id id} {::db/remove-deleted false})] + (t/testing "the payload lands in the legacy column" + (t/is (= (seq payload) (seq ^bytes (:data row))))) + + (t/testing "the watermark moves to the moment of the mutation" + (t/is (pos? (compare (inst-ms (:updated-at row)) (inst-ms past)))))))) diff --git a/backend/test/backend_tests/file_change_retention_test.clj b/backend/test/backend_tests/file_change_retention_test.clj index 7cede5a164..792f931e10 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 @@ -21,6 +21,7 @@ [app.common.time :as ct] [app.common.uuid :as uuid] [app.db :as db] + [app.features.file-changes :as fchanges] [app.rpc :as-alias rpc] [app.rpc.commands.files :as files] [app.util.blob :as blob] @@ -64,6 +65,7 @@ :version 0 :created-by (or created-by "system") :label label + :kind (if label fchanges/snapshot-kind fchanges/revision-kind) :data data :changes changes} {::db/return-keys false}) @@ -170,6 +172,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..521d056a01 --- /dev/null +++ b/backend/test/backend_tests/file_change_schema_test.clj @@ -0,0 +1,110 @@ +;; 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-keeps-the-data-column-and-a-kind-column + ;; The `data` column is kept: the `legacy-db` backend reads old payloads + ;; from it (migration 0157 moved the existing blobs). The dead + ;; `data_ref_id`/`data_backend` columns stay gone: no code read them since + ;; the file storage refactor. + (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_ref_id', 'data_backend')"]) + (mapv :name)))) + (t/is (= ["bytea"] + (->> (db/exec! th/*pool* ["SELECT pg_catalog.format_type(atttypid, atttypmod) AS type + FROM pg_attribute + WHERE attrelid = 'file_change'::regclass + AND attname = 'data'"]) + (mapv :type)))) + (t/is (true? (query "SELECT attnotnull AS notnull + FROM pg_attribute + WHERE attrelid = 'file_change'::regclass + AND attname = 'kind'")))) + +(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/migrations_partition_file_change_test.clj b/backend/test/backend_tests/migrations_partition_file_change_test.clj new file mode 100644 index 0000000000..7e2dbd6cef --- /dev/null +++ b/backend/test/backend_tests/migrations_partition_file_change_test.clj @@ -0,0 +1,177 @@ +;; 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 backend-tests.migrations-partition-file-change-test + "Covers the data copy of 0158-partition-file-change: the rows of the old + table move to the partitioned one with their columns in place, the `kind` + comes from the label (1 snapshot, 2 revision), and the legacy table keeps + the rows." + + (:require + [app.common.time :as ct] + [app.common.uuid :as uuid] + [app.db :as db] + [app.util.blob :as blob] + [backend-tests.helpers :as th] + [clojure.java.io :as io] + [clojure.test :as t])) + +(t/use-fixtures :once th/state-init) +(t/use-fixtures :each th/database-reset) + +(def ^:private sql:old-schema + "CREATE TABLE file (id uuid PRIMARY KEY); + CREATE TABLE profile (id uuid PRIMARY KEY); + + 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, + data bytea, + 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, + data_ref_id uuid, + data_backend text, + CONSTRAINT file_change_pkey PRIMARY KEY (id), + CONSTRAINT file_change_file_id_fkey FOREIGN KEY (file_id) + REFERENCES file(id) DEFERRABLE, + CONSTRAINT file_change_locked_by_fkey FOREIGN KEY (locked_by) + REFERENCES profile(id) ON DELETE SET NULL DEFERRABLE, + CONSTRAINT file_change_profile_id_fkey FOREIGN KEY (profile_id) + REFERENCES profile(id) ON DELETE SET NULL DEFERRABLE); + + CREATE INDEX file_change__data_ref_id__idx + ON file_change (data_ref_id); + CREATE INDEX file_change__deleted_at__idx + ON file_change (deleted_at) 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__system_snapshots__idx + ON file_change (file_id, created_at) + WHERE created_by = 'system' AND deleted_at IS 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;") + +(defn- run-migration! + [conn] + (let [sql (slurp (io/resource "app/migrations/sql/0158-partition-file-change.sql"))] + (db/exec! conn [sql]))) + +(t/deftest copies-the-rows-into-the-partitioned-table + ;; The migration runs in its own schema, with the shape of the table before + ;; it: the run below is the only place where its INSERT..SELECT runs with + ;; rows, so a swapped column or a wrong `kind` fails here and not on a real + ;; instance. + (let [schema (str "test_0158_" (subs (str (uuid/next)) 0 8)) + fid (uuid/next) + snap-id (uuid/next) + rev-id (uuid/next) + created-at (ct/in-past "2h") + updated-at (ct/in-past "1h") + deleted-at (ct/in-past "30m") + snap-data (blob/encode {:snapshot 1}) + rev-changes (blob/encode [{:type :add-obj :id (uuid/next)}])] + + (db/tx-run! th/*pool* + (fn [{:keys [::db/conn]}] + ;; the schema is dropped before the commit, and a failure + ;; rolls the whole transaction back: nothing is left behind + (db/exec! conn [(str "CREATE SCHEMA " schema)]) + (db/exec! conn [(str "SET LOCAL search_path TO " schema)]) + (db/exec! conn [sql:old-schema]) + + (db/insert! conn :file {:id fid}) + + (db/insert! conn :file-change + {:id snap-id + :file-id fid + :revn 1 + :data snap-data + :label "v1" + :version 1 + :created-by "user" + :created-at created-at + :updated-at updated-at + :deleted-at deleted-at}) + + (db/insert! conn :file-change + {:id rev-id + :file-id fid + :revn 2 + :changes rev-changes + :created-at created-at + :updated-at updated-at}) + + (run-migration! conn) + + (let [rows (db/exec! conn ["SELECT id, revn, kind, label, data, changes, + created_at, updated_at, deleted_at, created_by + FROM file_change + ORDER BY revn"]) + snapshot (first rows) + revision (second rows) + legacy (db/exec! conn ["SELECT id, revn, data, changes + FROM file_change_legacy + ORDER BY revn"])] + + (t/testing "both rows are in the partitioned table" + (t/is (= 2 (count rows))) + (t/is (= [1 2] (mapv :revn rows)))) + + (t/testing "the labelled row is a snapshot with its payload" + (t/is (= 1 (:kind snapshot))) + (t/is (= "v1" (:label snapshot))) + (t/is (= "user" (:created-by snapshot))) + (t/is (= (seq snap-data) (seq ^bytes (:data snapshot)))) + (t/is (nil? (:changes snapshot))) + (t/is (= (inst-ms created-at) (inst-ms (:created-at snapshot)))) + (t/is (= (inst-ms updated-at) (inst-ms (:updated-at snapshot)))) + (t/is (= (inst-ms deleted-at) (inst-ms (:deleted-at snapshot))))) + + (t/testing "the unlabelled row is a revision with its changes" + (t/is (= 2 (:kind revision))) + (t/is (nil? (:label revision))) + (t/is (= (seq rev-changes) (seq ^bytes (:changes revision)))) + (t/is (nil? (:data revision))) + (t/is (= (inst-ms created-at) (inst-ms (:created-at revision))))) + + (t/testing "the legacy table keeps the rows for the rollback" + (t/is (= 2 (count legacy))) + (t/is (= [snap-id rev-id] (mapv :id legacy))) + (t/is (= (seq snap-data) + (seq ^bytes (:data (first legacy))))) + (t/is (= (seq rev-changes) + (seq ^bytes (:changes (second legacy))))))) + + (t/testing "the legacy table no longer points at file" + ;; Its foreign key on `file_id` is `NO ACTION`: while the table has + ;; rows, the objects GC cannot delete the files they point to. The + ;; migration drops that one and keeps the two `ON DELETE SET NULL` ones. + (t/is (= ["file_change_locked_by_fkey_legacy" + "file_change_profile_id_fkey_legacy"] + (->> (db/exec! conn ["SELECT con.conname AS name + FROM pg_constraint con + WHERE con.conrelid = 'file_change_legacy'::regclass + AND con.contype = 'f' + ORDER BY con.conname"]) + (mapv :name))))) + + (db/exec! conn [(str "DROP SCHEMA " schema " CASCADE")]))))) diff --git a/backend/test/backend_tests/objects_gc_test.clj b/backend/test/backend_tests/objects_gc_test.clj index 6696a7f2f0..646650a4ca 100644 --- a/backend/test/backend_tests/objects_gc_test.clj +++ b/backend/test/backend_tests/objects_gc_test.clj @@ -8,8 +8,10 @@ (:require [app.common.logging :as l] [app.common.time :as ct] + [app.common.uuid :as uuid] [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 +75,69 @@ (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 per file, with a constant `file_id`, + ;; 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 deletes-the-pending-file-changes-of-several-chunks + ;; More pending rows than the chunk size: the plan walks the cursor and each + ;; chunk locks its own rows, one query per file, with a constant `file_id` + ;; so the planner prunes the partition. + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile)}) + ids (repeatedly 5 uuid/next) + _ (doseq [id ids] + (th/db-insert! :file-change + {:id id + :file-id (:id file) + :kind fchanges/revision-kind + :deleted-at (ct/in-past "10m")})) + result (th/run-task! :objects-gc {:chunk-size 2})] + + (t/is (pos? (:processed result))) + (t/is (empty? (th/db-query :file-change {:file-id (:id file)} + {::db/remove-deleted false}))))) + +(t/deftest a-chunk-spanning-several-files-deletes-all-of-them + ;; The rows of one chunk can belong to different files. Each group is locked + ;; with its own `file_id`, and every row is still deleted; buffering the + ;; groups instead of locking them would leave them behind. + (let [profile (th/create-profile* 1) + opts {:profile-id (:id profile) + :project-id (:default-project-id profile)} + files [(th/create-file* 1 opts) + (th/create-file* 2 opts)] + + _ (doseq [file files] + (dotimes [_ 2] + (th/db-insert! :file-change + {:file-id (:id file) + :kind fchanges/revision-kind + :deleted-at (ct/in-past "10m")}))) + + ;; one chunk holds the rows of both files + result (th/run-task! :objects-gc {:chunk-size 4})] + + (t/is (pos? (:processed result))) + (doseq [file files] + (t/is (empty? (th/db-query :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 9238a25b79..0af89b8397 100644 --- a/backend/test/backend_tests/rpc_file_snapshot_test.clj +++ b/backend/test/backend_tests/rpc_file_snapshot_test.clj @@ -87,6 +87,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) @@ -739,3 +752,225 @@ (let [stored (fsnap/get-snapshot th/*system* file-id snap-id)] (t/is (nil? (:migrations stored))) (t/is (= 60 (:version stored))))))) +(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 + :update-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 (cond-> {::th/type type + ::rpc/profile-id (:id profile) + :file-id (:id file) + :id (:id snapshot)} + (= :update-file-snapshot type) + (assoc :label "renamed")) + 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 + :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 + :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 cd49f85042..8bc7c78fee 100644 --- a/backend/test/backend_tests/rpc_quotes_test.clj +++ b/backend/test/backend_tests/rpc_quotes_test.clj @@ -563,6 +563,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/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..28c4f9a0a6 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,15 +111,22 @@ (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! - (fn [[users data]] + (fn [[users _]] (let [users (d/index-by :id users)] (st/emit! (se/event plugin-id "pin-version" :file-id file-id)) - (resolve (file-version-proxy plugin-id file-id users @data)))) + ;; the command answers `true`: the pinned row is the + ;; current one with the new label, as a user version + (resolve (file-version-proxy plugin-id file-id users + (assoc @data + :label (:label params) + :created-by "user" + :deleted-at nil))))) reject)))))))))) (defn file-proxy? [p] diff --git a/frontend/test/frontend_tests/data/workspace_versions_test.cljs b/frontend/test/frontend_tests/data/workspace_versions_test.cljs index 5ec7fa87bb..57f06b5999 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,34 @@ (t/is (nil? (get-in result [:workspace-versions :backup])))) (finally (rx/dispose! sub))))) + +(t/deftest ^:async 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) + requests (atom []) + state {:current-file-id file-id + :workspace-versions {:data [{:id version-id + :created-at (ct/now) + :created-by "system"}]}} + call! (fn [event] + (reset! requests []) + (->> (ptk/watch event state (rx/empty)) + (rx/subs! (fn [_]))) + (first @requests))] + (await + (mock/with-mocks* + {rp/cmd! (mock/stub (fn [cmd params] + (swap! requests conj [cmd params]) + (rx/of true)))} + (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)))))))))) \ No newline at end of file diff --git a/frontend/test/frontend_tests/plugins/file_test.cljs b/frontend/test/frontend_tests/plugins/file_test.cljs index e6f2780035..a15b5d3c52 100644 --- a/frontend/test/frontend_tests/plugins/file_test.cljs +++ b/frontend/test/frontend_tests/plugins/file_test.cljs @@ -6,8 +6,14 @@ (ns frontend-tests.plugins.file-test (:require + [app.common.time :as ct] + [app.common.uuid :as uuid] + [app.main.repo :as rp] [app.plugins.file :as file] - [cljs.test :as t :include-macros true])) + [app.plugins.register :as r] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.mock :as mock])) (t/deftest file-version-created-at-returns-stored-date (let [created-at (js/Date.) @@ -19,3 +25,92 @@ :label "Version" :created-at created-at})] (t/is (identical? created-at (.-createdAt version))))) + +(defn- make-version + [file-id data] + (file/file-version-proxy (str (uuid/next)) file-id {} data)) + +(defn- record-cmd! + "Mock for `rp/cmd!` that records the calls and answers an observable with + the shape each command expects: a list of team users, and `true` for the + snapshot update (`update-file-snapshot` answers a boolean)." + [calls] + (fn [cmd params] + (swap! calls conj [cmd params]) + (rx/of (if (= :get-team-users cmd) [] true)))) + +(t/deftest ^:async file-version-proxy-sends-the-file-id + ;; The snapshot RPC commands require the `file-id` (they need it for the + ;; partition pruning on `file_change`); the plugin proxy is the other door to + ;; them and has to send it on rename, remove and pin. + (let [file-id (uuid/next) + version-id (uuid/next) + calls (atom [])] + + (await + (mock/with-mocks* + {r/check-permission (constantly true) + rp/cmd! (mock/stub (record-cmd! calls))} + + (t/testing "rename sends the file id" + (reset! calls []) + (let [version (make-version file-id + {:id version-id + :label "Version" + :created-at (ct/now) + :created-by "user"})] + (aset version "label" "renamed") + (let [[cmd params] (last @calls)] + (t/is (= :update-file-snapshot cmd)) + (t/is (= file-id (:file-id params))) + (t/is (= "renamed" (:label params)))))) + + (t/testing "remove sends the file id" + (reset! calls []) + (let [version (make-version file-id + {:id version-id + :label "Version" + :created-at (ct/now) + :created-by "user"})] + (await (.remove version)) + (let [[cmd params] (last @calls)] + (t/is (= :delete-file-snapshot cmd)) + (t/is (= file-id (:file-id params)))))) + + (t/testing "pin sends the file id on both commands" + (reset! calls []) + (let [version (make-version file-id + {:id version-id + :label "Version" + :created-at (ct/now) + :created-by "system"})] + (await (.pin version)) + (t/is (= [[:get-team-users file-id] + [:update-file-snapshot file-id]] + (mapv (fn [[cmd params]] [cmd (:file-id params)]) @calls))))))))) + +(t/deftest ^:async file-version-proxy-pin-resolves-the-pinned-version + ;; `pin` answers with a proxy, not with the boolean the snapshot command + ;; returns: the autosave becomes a user version with a new label. + (let [file-id (uuid/next) + version-id (uuid/next) + created-at (ct/now) + calls (atom [])] + (await + (mock/with-mocks* + {r/check-permission (constantly true) + rp/cmd! (mock/stub (record-cmd! calls))} + (let [version (make-version file-id + {:id version-id + :label "Version" + :created-at created-at + :created-by "system"}) + pinned (await (.pin version))] + (t/is (file/file-version-proxy? pinned)) + (t/is (= [[:get-team-users file-id] + [:update-file-snapshot file-id]] + (mapv (fn [[cmd params]] [cmd (:file-id params)]) @calls))) + (t/is (= (ct/format-inst created-at :localized-date) + (.-label pinned))) + (t/is (false? (.-isAutosave pinned))) + (t/is (identical? created-at (.-createdAt pinned)))))))) diff --git a/operations/2.19.0-file-change-partition.md b/operations/2.19.0-file-change-partition.md new file mode 100644 index 0000000000..8aba631091 --- /dev/null +++ b/operations/2.19.0-file-change-partition.md @@ -0,0 +1,920 @@ +# 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/0158-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. + +## Sizing your instance + +Every number in this procedure comes from your database: run the queries +below, the procedure does not depend on any absolute size. It is plain SQL +and runs on any supported PostgreSQL (>= 15). + +```sql +-- total size of the database and of file_change (heap + TOAST + indexes) +SELECT pg_size_pretty(pg_database_size(current_database())) AS database; +SELECT pg_size_pretty(pg_total_relation_size('file_change')) AS file_change_total, + pg_size_pretty(pg_table_size('file_change')) AS heap_and_toast, + pg_size_pretty(pg_indexes_size('file_change')) AS indexes; +SELECT count(*) AS rows FROM file_change; + +-- dead tuples waiting for vacuum: their space is not copied +SELECT n_dead_tup FROM pg_stat_user_tables WHERE relname = 'file_change'; +``` + +`pg_table_size` (heap + TOAST) is the upper bound of what the copy reads and +writes. Do not estimate it with `pg_stats.avg_width`: that measures what the +heap stores for a column, which for a TOASTed value is the pointer, not the +value. The estimate reads far below the real size (about ten times less on a +small instance, orders of magnitude on a big one). + +Peak disk during the bulk is about the old table (it does not grow) plus the +new table (at most its `pg_table_size`, less because the dead rows are not +copied) plus its six indexes (about the `pg_indexes_size` of the live rows) +plus the WAL the bulk generates (proportional to what it writes: watch the +free space of the `pg_wal` mount while the first batches run). The disk comes +back when the legacy table is dropped, days later. Time the first batches and +extrapolate: the bulk reads heap + TOAST once sequentially and writes about +the same, so its duration is proportional to `pg_table_size`, not to the heap +alone. + +## 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 17 columns: `data_ref_id` and `data_backend` are gone (dead + since the file storage refactor), `data` is kept (the `legacy-db` backend + reads old payloads from it) 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 (see sizing), so the new + table holds at most its heap + TOAST, and less as the dead rows are not + copied. The disk comes back when the legacy table is dropped: the database + shrinks by the size of the old table. + +## 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 every row. 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 +small next to the TOAST, so a delta pass without an index scans the heap and +only detoastes the rows that changed. + +--- + +## Phase 0 — Before anything + +### 0.0 The role of the session + +Every session of this procedure starts with `SET ROLE` to the role that owns +`file_change` (normally `penpot`): + +```sql +SET ROLE penpot; -- replace with the owner of file_change (see 0.6) +``` + +The new table, its 16 partitions, its indexes and its TOAST are created with +the role of the session, and `ALTER TABLE ... OWNER TO` does not recurse to the +partitions. Without this, a procedure run as a superuser leaves the table +owned by that superuser and the backend, which connects as `penpot`, gets +`permission denied` after the cutover. + +### 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', '0158-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 table), 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; 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 sequentially and writes one narrow entry per + row (check its size with `pg_relation_size` after the build): it does not + block the writes either way. Time the build and extrapolate the bulk from + it (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 about the old table (it does not grow) + the new table +-- (at most its `pg_table_size`, see sizing) + the WAL the bulk generates +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 WAL proportional to what it writes, +-- watch the retained WAL while the first batches run. +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 were already moved to `file_data` by +-- migration 0157-migrate-legacy-file-data (on the big instances, by hand, +-- with its runbook 2.19.0-migrate-legacy-file-data.md), so there is nothing +-- to copy here: the `data` column travels with the row to the new table. +-- These two numbers are informational guards. The first one has to be 0 +-- after 0157 ran; if it is not, run 0157 (or its runbook) before starting. +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 travels +-- with the row to the new table. +SELECT count(*) AS revisions_with_data + FROM file_change + WHERE data IS NOT NULL + AND label IS NULL; +``` + +- 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 last delta pass, 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). + +### 0.6 Owner and grants + +```sql +-- the role that has to own every object at the end (see 0.0) +SELECT tableowner FROM pg_tables + WHERE schemaname = current_schema() AND tablename = 'file_change'; + +-- grants added by the operator (monitoring, reporting). This procedure +-- creates a new table: they do not travel with it, so save them to re-apply +-- them after the cutover. +SELECT grantee, privilege_type + FROM information_schema.role_table_grants + WHERE table_schema = current_schema() AND table_name = 'file_change'; +``` + +--- + +## Phase 1 — Create the partitioned table + +The new indexes carry a `_new` suffix because index names are unique per +schema and the old table still holds the canonical ones until the cutover. +The same goes for the primary key of the new table: its index is named after +the constraint. The constraints that have no index (the `CHECK` and the +foreign keys) are named canonically from the start, because a constraint name +is unique per table, not per schema, so it can live with the one of the old +table; the partitions inherit it when they are created. The partitions 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, + data bytea, + 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_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). They take the canonical name +-- already: they have no index, so they do not collide with the old table and +-- the partitions inherit it. +ALTER TABLE file_change_new + ADD CONSTRAINT file_change_file_id_fkey FOREIGN KEY (file_id) + REFERENCES file(id) DEFERRABLE; +ALTER TABLE file_change_new + ADD CONSTRAINT file_change_locked_by_fkey FOREIGN KEY (locked_by) + REFERENCES profile(id) ON DELETE SET NULL DEFERRABLE; +ALTER TABLE file_change_new + ADD CONSTRAINT file_change_profile_id_fkey 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 with the payload living in `file_data` the column +-- no longer tells snapshots apart. 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, + last_at timestamptz +); + +-- for a procedure that already created the table without the mark +ALTER TABLE file_change_copy_state + ADD COLUMN IF NOT EXISTS last_at timestamptz; + +INSERT INTO file_change_copy_state (name, last_id) +VALUES ('bulk', '00000000-0000-0000-0000-000000000000') +ON CONFLICT (name) DO NOTHING; + +INSERT INTO file_change_copy_state (name) VALUES ('delta') +ON CONFLICT (name) DO NOTHING; +``` + +One batch (idempotent). It **always returns one row**: the number of rows it +copied, `0` when there is nothing left to copy. + +```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, data, 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, data, 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, + data = EXCLUDED.data, + 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 + WHERE file_change_new.updated_at IS DISTINCT FROM EXCLUDED.updated_at + RETURNING 1 +), +-- `moved` advances the pointer only when the batch copied something. +-- PostgreSQL executes a data-modifying CTE exactly once even when the final +-- query does not read its output. +moved AS ( + 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 1 +) +SELECT count(*) AS copied FROM copied; +``` + +Repeat this statement until it returns `0`. The `0` comes from the final +`SELECT`, not from a missing row: a batch that finds nothing to copy still +returns it, so it is the signal to stop. + +- The pointer is a `uuid` (random order): the pagination is stable but it does + not group by physical position. The total read is the heap, in + a scattered order. +- **Estimated duration**: time the first batches and extrapolate. The bulk + reads the heap plus the live TOAST once (see sizing) and writes about the + live data. Batches are rows divided by batch size: count your rows in + sizing and divide. +- Pace the batches yourself (a pause between statements) 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 count(*) AS missing + FROM file_change o + WHERE NOT EXISTS (SELECT 1 FROM file_change_new n + WHERE n.file_id = o.file_id + AND n.id = o.id); +``` + +`missing` is zero when no row of the old table is absent from the new one; it +is the check that matters, not the total count. The new table can have **more** +rows than the old one: the GC may have hard deleted a row from the source +after the bulk copied it, and it cleans that row up after the cutover. Rows +written during the bulk are missing until the delta passes pick them up. + +--- + +## Phase 3 — Delta passes + +Every pass copies the rows modified since the **previous pass**. The watermark +lives in `file_change_copy_state.last_at` (the start of the last pass), and the +SQL subtracts one hour of margin, which absorbs the clock drift between the +application (which writes `updated_at`) and the database (which writes +`now()`) and the drift between batches. A row that a pass already copied but +did not change again is not rewritten: the `WHERE` of the `ON CONFLICT` skips +it. + +```sql +-- :bulk_start = the time the bulk started (the margin is applied below). +-- Set it in this session before running the pass: without it the query fails +-- instead of copying nothing and leaving those rows behind for good. +SET penpot.bulk_start = '2026-10-25 08:00:00+02'; + +BEGIN; + +WITH mark AS ( + SELECT COALESCE((SELECT last_at FROM file_change_copy_state WHERE name = 'delta'), + current_setting('penpot.bulk_start')::timestamptz) - INTERVAL '1 hour' AS since +) +INSERT INTO file_change_new (id, file_id, created_at, session_id, revn, data, 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, data, 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, mark + WHERE updated_at > mark.since +ON CONFLICT (file_id, id) DO UPDATE + SET created_at = EXCLUDED.created_at, + session_id = EXCLUDED.session_id, + revn = EXCLUDED.revn, + data = EXCLUDED.data, + 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 + WHERE file_change_new.updated_at IS DISTINCT FROM EXCLUDED.updated_at; + +-- The mark is the start of this transaction (`now()`): the rows changed +-- after it are the window of the next pass. Both statements go in the same +-- transaction on purpose; a mark written later would leave a gap. +UPDATE file_change_copy_state SET last_at = now() WHERE name = 'delta'; + +COMMIT; +``` + +- Repeat the transaction 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 window, so it takes seconds on a quiet instance; without it, + it scans the heap. To see how many rows are left in the window: + +```sql +SELECT count(*) FROM file_change + WHERE updated_at > (SELECT last_at FROM file_change_copy_state WHERE name = 'delta'); +``` + +- 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 GC deletes the pending files in + chunks of 100, and one blocked file aborts the transaction of the whole + chunk: the chunk is logged as `error on chunk deletion` and skipped, its + files wait in the trash, and the next daily run retries the same chunk and + fails again until the cutover. The other chunks keep making progress. **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 +SET ROLE penpot; -- the owner of file_change (see 0.0 and 0.6) +SET penpot.bulk_start = '2026-10-25 08:00:00+02'; + +BEGIN; + +LOCK TABLE file_change IN ACCESS EXCLUSIVE MODE; + +-- The final delta: the rows modified since the last pass (its mark, minus +-- the hour of margin). The mark is the one the last pass left; the fallback +-- is the bulk start. +WITH mark AS ( + SELECT COALESCE((SELECT last_at FROM file_change_copy_state WHERE name = 'delta'), + current_setting('penpot.bulk_start')::timestamptz) - INTERVAL '1 hour' AS since +) +INSERT INTO file_change_new (id, file_id, created_at, session_id, revn, data, 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, data, 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, mark + WHERE updated_at > mark.since +ON CONFLICT (file_id, id) DO UPDATE + SET created_at = EXCLUDED.created_at, + session_id = EXCLUDED.session_id, + revn = EXCLUDED.revn, + data = EXCLUDED.data, + 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 + WHERE file_change_new.updated_at IS DISTINCT FROM EXCLUDED.updated_at; + +-- Check inside the lock: no row of the old table may be missing. The new +-- table can carry extra rows that the GC hard deleted from the source; it +-- cleans them up after the cutover. +SELECT count(*) AS missing + FROM file_change o + WHERE NOT EXISTS (SELECT 1 FROM file_change_new n + WHERE n.file_id = o.file_id + AND n.id = o.id); + +-- The old table frees the canonical names (the same names the migration +-- 0158 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. Only the primary key needs the rename: the +-- `CHECK` and the foreign keys already carry the canonical name (they have no +-- index, so they could take it from the start), and the partitions inherited +-- it. +ALTER TABLE file_change_new RENAME CONSTRAINT file_change_new_pkey + TO file_change_pkey; + +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 +-- :cutover = the time the backend went up on the new table. Set it in this +-- session: without it the queries fail instead of comparing against a stale +-- date. +SET penpot.cutover = '2026-10-25 08:00:00+02'; + +-- the rows written after the cutover: this one grows with every save +SELECT count(*) FROM file_change + WHERE updated_at > current_setting('penpot.cutover')::timestamptz + +-- the rows that went to the legacy table: this one has to stay at 0 +SELECT count(*) FROM file_change_legacy + WHERE updated_at > current_setting('penpot.cutover')::timestamptz; +``` + +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`, carrying `data` too). 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 `0158`. Every row of this + query has to show `got = expected`: + +```sql +SELECT 'columns' AS what, count(*) AS got, 17 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 || '_%' +UNION ALL +SELECT 'partition constraints out of name', count(*), 0 + FROM pg_constraint c + JOIN pg_class t ON t.oid = c.conrelid + WHERE t.relname ~ '^file_change_[0-9]{2}$' + AND c.contype <> 'n' + AND c.conname LIKE 'file_change_new%'; +``` + + The `NOT NULL` constraints (`contype = 'n'`, tracked in `pg_constraint` from + PostgreSQL 17) are excluded from the last check: their name is generated by + the column and changes with the server version, so they are not part of the + parity. The `CHECK`, the primary key and the foreign keys are. + + 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; + +-- every table, partition and index of the family shares the owner of the +-- table: a session without `SET ROLE` (0.0) shows here +SELECT count(*) AS out_of_owner + FROM pg_class c + WHERE c.relname LIKE 'file_change%' + AND c.relkind IN ('r', 'p', 'i') + AND pg_get_userbyid(c.relowner) <> + (SELECT tableowner FROM pg_tables + WHERE schemaname = current_schema() AND tablename = 'file_change'); +``` + + The owner check has to return `0`. If it does not, the session that + created the table did not use `SET ROLE`: fix it before opening the + service (re-own the table, the 16 partitions and their indexes) and + re-apply the grants saved in 0.6. + +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: each of the 16 partitions has its own TOAST (check their + sizes with `pg_total_relation_size` per partition); 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` (the team snapshot + quota and the telemetry counts) read all 16 partitions. To know their + cost on your instance, time them before and after with + `EXPLAIN (ANALYZE, BUFFERS)` and compare: + + - 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 scans the whole heap, the same before and after: the + partitioned run pays a small cost per partition on top. 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: the database shrinks by about the +size of the old table (see sizing). + +--- + +## Risks + +| Risk | Mitigation | +|---|---| +| Massive WAL during the bulk (proportional to the copy; see sizing) | 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) holds at most the + `pg_table_size` of the old table (see sizing). +- After the `DROP` of the old table: the database shrinks by about the size + that table had. +- 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. diff --git a/operations/README.md b/operations/README.md new file mode 100644 index 0000000000..be1036e7c6 --- /dev/null +++ b/operations/README.md @@ -0,0 +1,16 @@ +# 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 | Version | +|---|---| +| [Migrate legacy file payloads to `file_data`](2.19.0-migrate-legacy-file-data.md) | 2.19.0 | +| [Partition `file_change` by `file_id`](2.19.0-file-change-partition.md) | 2.19.0 |