From 575b68d2c906617ae94864da039154c85af9d97c Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Mon, 12 Oct 2026 13:11:29 +0000 Subject: [PATCH] :bug: Unmark the storage objects that a restore brings back Restoring a file only un-marked the database rows (`file`, `file_media_object`, `file_change`, `file_data`, `file_thumbnail`, `file_tagged_object_thumbnail`), not the storage objects those rows reference. An object that was already marked for deletion stayed marked while a live row pointed at it, and those foreign keys are `NO ACTION`: the storage GC cannot delete it and skips it forever (and before the GC learned to skip, it cost the whole nightly run). Restoring a team had the same gap with its font variants. Both restores now unmark the storage objects of the rows they bring back: the media and the thumbnail of the file media objects, the media of the file and object thumbnails, the `storage-ref-id` of the file data, and the four files of a font variant. Objects referenced by other rows are left alone. AI-assisted-by: deepseek-v4.1-flash --- backend/src/app/rpc/commands/files.clj | 42 +++++- backend/src/app/srepl/main.clj | 14 ++ .../restore_storage_objects_test.clj | 126 ++++++++++++++++++ 3 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 backend/test/backend_tests/restore_storage_objects_test.clj diff --git a/backend/src/app/rpc/commands/files.clj b/backend/src/app/rpc/commands/files.clj index b7d04a29a3..c5169643bc 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -1320,6 +1320,40 @@ "UPDATE file_tagged_object_thumbnail SET deleted_at = null WHERE file_id = ANY(?::uuid[])") +(def ^:private sql:restore-storage-objects + "WITH ids AS (SELECT unnest(?::uuid[]) AS file_id) + + UPDATE storage_object + SET deleted_at = null + WHERE deleted_at IS NOT NULL + AND (id IN (SELECT media_id + FROM file_media_object AS fmo + JOIN ids ON (ids.file_id = fmo.file_id) + WHERE fmo.deleted_at IS NULL + AND fmo.media_id IS NOT NULL + UNION + SELECT thumbnail_id + FROM file_media_object AS fmo + JOIN ids ON (ids.file_id = fmo.file_id) + WHERE fmo.deleted_at IS NULL + AND fmo.thumbnail_id IS NOT NULL + UNION + SELECT ft.media_id + FROM file_thumbnail AS ft + JOIN ids ON (ids.file_id = ft.file_id) + WHERE ft.deleted_at IS NULL + AND ft.media_id IS NOT NULL + UNION + SELECT fot.media_id + FROM file_tagged_object_thumbnail AS fot + JOIN ids ON (ids.file_id = fot.file_id) + WHERE fot.deleted_at IS NULL + AND fot.media_id IS NOT NULL) + OR id::text IN (SELECT fd.metadata->>'storage-ref-id' + FROM file_data AS fd + JOIN ids ON (ids.file_id = fd.file_id) + WHERE fd.deleted_at IS NULL))") + (defn- restore-files [conn file-ids] (let [file-ids (db/create-array conn "uuid" file-ids)] @@ -1328,7 +1362,13 @@ (db/exec-one! conn [sql:restore-file-changes file-ids]) (db/exec-one! conn [sql:restore-file-data file-ids]) (db/exec-one! conn [sql:restore-file-thumbnails file-ids]) - (db/exec-one! conn [sql:restore-file-tagged-object-thumbnails file-ids]))) + (db/exec-one! conn [sql:restore-file-tagged-object-thumbnails file-ids]) + + ;; NOTE: the storage objects referenced by the rows we just brought back + ;; must stop being marked for deletion. The storage GC cannot delete an + ;; object that a live row references (those foreign keys are `NO ACTION`), + ;; so a marked object that comes back to life is skipped forever otherwise. + (db/exec-one! conn [sql:restore-storage-objects file-ids]))) (def ^:private sql:restore-projects "UPDATE project SET deleted_at = null WHERE id = ANY(?::uuid[])") diff --git a/backend/src/app/srepl/main.clj b/backend/src/app/srepl/main.clj index 11b2732c19..6f497e9755 100644 --- a/backend/src/app/srepl/main.clj +++ b/backend/src/app/srepl/main.clj @@ -720,6 +720,15 @@ :id team-id}))) :deleted)) +(def ^:private sql:restore-font-file-objects + "UPDATE storage_object + SET deleted_at = null + WHERE deleted_at IS NOT NULL + AND id IN (SELECT unnest(array[woff1_file_id, woff2_file_id, otf_file_id, ttf_file_id]) + FROM team_font_variant + WHERE team_id = ?::uuid + AND deleted_at IS NULL)") + (defn- restore-team* [{:keys [::db/conn] :as cfg} team-id] (db/update! conn :team @@ -730,6 +739,11 @@ {:deleted-at nil} {:team-id team-id}) + ;; Same as in `restore-files`: the font objects of the variants we just + ;; brought back must stop being marked for deletion, or the storage GC will + ;; skip them forever (those foreign keys are `NO ACTION`). + (db/exec-one! conn [sql:restore-font-file-objects team-id]) + (doseq [{:keys [id]} (db/query conn :project {:team-id team-id} {::sql/columns [:id]})] diff --git a/backend/test/backend_tests/restore_storage_objects_test.clj b/backend/test/backend_tests/restore_storage_objects_test.clj new file mode 100644 index 0000000000..5545270648 --- /dev/null +++ b/backend/test/backend_tests/restore_storage_objects_test.clj @@ -0,0 +1,126 @@ +;; 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.restore-storage-objects-test + (:require + [app.common.time :as ct] + [app.common.uuid :as uuid] + [app.db :as db] + [app.rpc.commands.files :as files] + [app.srepl.main :as srepl] + [app.storage :as sto] + [app.system :as sys] + [backend-tests.helpers :as th] + [clojure.test :as t])) + +(t/use-fixtures :once th/state-init) +(t/use-fixtures :each th/database-reset) + +;; Restoring a file brings its rows back to life, so the storage objects those +;; rows reference must stop being marked for deletion: otherwise the storage GC +;; hits the foreign key and skips them forever. + +(defn- put-object! + [bucket] + (sto/put-object! (::sto/storage th/*system*) + {::sto/content (sto/content "sobject") + :content-type "text/plain" + :bucket bucket})) + +(defn- find-object + [id] + (th/db-get :storage-object {:id id} {::db/remove-deleted false})) + +(defn- mark-object! + [id] + (th/db-update! :storage-object {:deleted-at (ct/in-past {:days 1})} {:id id})) + +(defn- restore-file! + [file-id] + (db/tx-run! th/*system* + (fn [{:keys [::db/conn]}] + (#'files/restore-files conn [file-id])))) + +(t/deftest restoring-a-file-unmarks-the-objects-its-rows-reference + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile)}) + + ;; The media object's thumbnail points at an object of another bucket: + ;; that is the anomaly found in production. The marking task decides + ;; with the table of the object's bucket, so it does not see that + ;; reference and ends up marking an object that a live row still uses. + media-id (:id (put-object! "file-media-object")) + thumb-id (:id (put-object! "file-object-thumbnail")) + bystander (:id (put-object! "file-object-thumbnail")) + mobject (th/create-file-media-object* {:file-id (:id file) + :media-id media-id})] + + (th/db-update! :file-media-object + {:thumbnail-id thumb-id + :deleted-at (ct/in-past {:days 1})} + {:id (:id mobject)}) + + (t/testing "the fixture reproduces the anomaly" + (t/is (= "file-object-thumbnail" + (:bucket (meta (sto/get-object (::sto/storage th/*system*) thumb-id)))))) + + ;; The objects end up marked as deleted, as the marking task leaves them. + (mark-object! media-id) + (mark-object! thumb-id) + (mark-object! bystander) + + (restore-file! (:id file)) + + (t/testing "the objects referenced by the restored rows are alive again" + (t/is (nil? (:deleted-at (find-object media-id)))) + (t/is (nil? (:deleted-at (find-object thumb-id))))) + + (t/testing "objects of other rows are left alone" + (t/is (some? (:deleted-at (find-object bystander))))))) + +(t/deftest restoring-a-file-unmarks-the-objects-of-its-data + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile)}) + sobject (:id (put-object! "file-data")) + data-id (uuid/next)] + + (th/db-update! :file {:deleted-at (ct/in-past {:days 1})} {:id (:id file)}) + (th/db-insert! :file-data + {:file-id (:id file) + :id data-id + :type "main" + :backend "fs" + :deleted-at (ct/in-past {:days 1}) + :metadata (db/json {"storage-ref-id" (str sobject)})}) + (mark-object! sobject) + + (restore-file! (:id file)) + + (t/is (nil? (:deleted-at (find-object sobject)))))) + +(t/deftest restoring-a-team-unmarks-the-objects-of-its-fonts + (let [profile (th/create-profile* 1) + team (th/create-team* 1 {:profile-id (:id profile)}) + sobject (:id (put-object! "team-font-variant"))] + + (th/db-insert! :team-font-variant + {:id (uuid/next) + :team-id (:id team) + :profile-id (:id profile) + :font-id (uuid/next) + :font-family "somefont" + :font-weight 400 + :font-style "normal" + :deleted-at (ct/in-past {:days 1}) + :woff1-file-id sobject}) + (mark-object! sobject) + + (with-redefs [sys/system th/*system*] + (srepl/restore-team! (:id team))) + + (t/is (nil? (:deleted-at (find-object sobject))))))