diff --git a/backend/scripts/repl b/backend/scripts/repl index 057018a11..0debeece2 100755 --- a/backend/scripts/repl +++ b/backend/scripts/repl @@ -24,6 +24,7 @@ export PENPOT_FLAGS="\ enable-rpc-climit \ enable-rpc-rlimit \ enable-soft-rpc-rlimit \ + enable-file-snapshot \ enable-webhooks \ enable-access-tokens \ enable-file-validation \ diff --git a/backend/scripts/start-dev b/backend/scripts/start-dev index 55f5e835a..564151bef 100755 --- a/backend/scripts/start-dev +++ b/backend/scripts/start-dev @@ -17,6 +17,7 @@ export PENPOT_FLAGS="\ disable-secure-session-cookies \ enable-rpc-climit \ enable-smtp \ + enable-file-snapshot \ enable-access-tokens \ enable-file-validation \ enable-file-schema-validation"; diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index 602da9d24..d1315d48b 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -42,8 +42,9 @@ :rpc-rlimit-config "resources/rlimit.edn" :rpc-climit-config "resources/climit.edn" - :file-change-snapshot-every 5 - :file-change-snapshot-timeout "3h" + :file-snapshot-total 10 + :file-snapshot-every 5 + :file-snapshot-timeout "3h" :public-uri "http://localhost:3449" :host "localhost" @@ -100,8 +101,9 @@ [:telemetry-uri {:optional true} :string] [:telemetry-with-taiga {:optional true} :boolean] ;; DELETE - [:file-change-snapshot-every {:optional true} :int] - [:file-change-snapshot-timeout {:optional true} ::dt/duration] + [:file-snapshot-total {:optional true} :int] + [:file-snapshot-every {:optional true} :int] + [:file-snapshot-timeout {:optional true} ::dt/duration] [:media-max-file-size {:optional true} :int] [:deletion-delay {:optional true} ::dt/duration] ;; REVIEW diff --git a/backend/src/app/rpc/commands/files_update.clj b/backend/src/app/rpc/commands/files_update.clj index f46764129..76b621b3c 100644 --- a/backend/src/app/rpc/commands/files_update.clj +++ b/backend/src/app/rpc/commands/files_update.clj @@ -123,12 +123,13 @@ (feat.fdata/persist-pointers! cfg id) result)))) -(declare get-lagged-changes) -(declare send-notifications!) -(declare update-file) -(declare update-file*) -(declare update-file-data) -(declare take-snapshot?) +(declare ^:private delete-old-snapshots!) +(declare ^:private get-lagged-changes) +(declare ^:private send-notifications!) +(declare ^:private take-snapshot?) +(declare ^:private update-file) +(declare ^:private update-file*) +(declare ^:private update-file-data) ;; If features are specified from params and the final feature ;; set is different than the persisted one, update it on the @@ -238,12 +239,15 @@ :created-at created-at :file-id (:id file) :revn (:revn file) + :label (::snapshot-label file) + :data (::snapshot-data file) :features (db/create-array conn "text" (:features file)) - :data (when (take-snapshot? file) - (:data file)) :changes (blob/encode changes)} {::db/return-keys false}) + (when (::snapshot-data file) + (delete-old-snapshots! cfg file)) + (db/update! conn :file {:revn (:revn file) :data (:data file) @@ -286,7 +290,6 @@ (-> data (blob/decode) (assoc :id (:id file))))) - ;; For avoid unnecesary overhead of creating multiple pointers ;; and handly internally with objects map in their worst ;; case (when probably all shapes and all pointers will be @@ -322,8 +325,27 @@ file (-> (files/check-version! file) (update :revn inc) (update :data cpc/process-changes changes) - (update :data d/without-nils))] + (update :data d/without-nils)) + file (if (take-snapshot? file) + (let [tpoint (dt/tpoint) + snapshot (-> (:data file) + (feat.fdata/process-pointers deref) + (feat.fdata/process-objects (partial into {})) + (blob/encode)) + elapsed (tpoint) + label (str "internal/snapshot/" (:revn file))] + + (l/trc :hint "take snapshot" + :file-id (str (:id file)) + :revn (:revn file) + :label label + :elapsed (dt/format-duration elapsed)) + + (-> file + (assoc ::snapshot-data snapshot) + (assoc ::snapshot-label label))) + file)] (binding [pmap/*tracked* nil] (when (contains? cf/flags :soft-file-validation) @@ -353,13 +375,42 @@ (defn- take-snapshot? "Defines the rule when file `data` snapshot should be saved." [{:keys [revn modified-at] :as file}] - (let [freq (or (cf/get :file-change-snapshot-every) 20) - timeout (or (cf/get :file-change-snapshot-timeout) - (dt/duration {:hours 1}))] - (or (= 1 freq) - (zero? (mod revn freq)) - (> (inst-ms (dt/diff modified-at (dt/now))) - (inst-ms timeout))))) + (when (contains? cf/flags :file-snapshot) + (let [freq (or (cf/get :file-snapshot-every) 20) + timeout (or (cf/get :file-snapshot-timeout) + (dt/duration {:hours 1}))] + + (or (= 1 freq) + (zero? (mod revn freq)) + (> (inst-ms (dt/diff modified-at (dt/now))) + (inst-ms timeout)))))) + +;; Get the latest available snapshots without exceeding the total +;; snapshot limit. +(def ^:private sql:get-latest-snapshots + "SELECT fch.id, fch.created_at + FROM file_change AS fch + WHERE fch.file_id = ? + AND fch.label LIKE 'internal/%' + ORDER BY fch.created_at DESC + LIMIT ?") + +;; Mark all snapshots that are outside the allowed total threshold +;; available for the GC. +(def ^:private sql:delete-snapshots + "UPDATE file_change + SET label = NULL + WHERE file_id = ? + AND label IS NOT NULL + AND created_at < ?") + +(defn- delete-old-snapshots! + [{:keys [::db/conn] :as cfg} {:keys [id] :as file}] + (when-let [snapshots (not-empty (db/exec! conn [sql:get-latest-snapshots id + (cf/get :file-snapshot-total 10)]))] + (let [last-date (-> snapshots peek :created-at) + result (db/exec-one! conn [sql:delete-snapshots id last-date])] + (l/trc :hint "delete old snapshots" :file-id (str id) :total (db/get-update-count result))))) (def ^:private sql:lagged-changes diff --git a/backend/src/app/tasks/file_gc.clj b/backend/src/app/tasks/file_gc.clj index 5430b0c98..79f5ff8b9 100644 --- a/backend/src/app/tasks/file_gc.clj +++ b/backend/src/app/tasks/file_gc.clj @@ -314,7 +314,7 @@ 0 (get-candidates cfg))] - (l/inf :hint "task finished" + (l/inf :hint "finished" :min-age (dt/format-duration min-age) :processed total) diff --git a/backend/test/backend_tests/helpers.clj b/backend/test/backend_tests/helpers.clj index 3b862d6dd..8380ea13e 100644 --- a/backend/test/backend_tests/helpers.clj +++ b/backend/test/backend_tests/helpers.clj @@ -61,7 +61,7 @@ (def default {:database-uri "postgresql://postgres/penpot_test" :redis-uri "redis://redis/1" - :file-change-snapshot-every 1}) + :file-snapshot-every 1}) (def config (cf/read-config :prefix "penpot-test" @@ -76,6 +76,7 @@ :enable-feature-fdata-pointer-map :enable-feature-fdata-objets-map :enable-feature-components-v2 + :enable-file-snapshot :disable-file-validation]) (defn state-init diff --git a/backend/test/backend_tests/rpc_file_test.clj b/backend/test/backend_tests/rpc_file_test.clj index d28213879..5d1fe1824 100644 --- a/backend/test/backend_tests/rpc_file_test.clj +++ b/backend/test/backend_tests/rpc_file_test.clj @@ -166,6 +166,10 @@ :name "test" :id page-id}]) + ;; Check the number of fragments before adding the page + (let [rows (th/db-query :file-data-fragment {:file-id (:id file)})] + (t/is (= 3 (count rows)))) + ;; The file-gc should mark for remove unused fragments (let [res (th/run-task! :file-gc {:min-age 0})] (t/is (= 1 (:processed res)))) @@ -176,11 +180,11 @@ ;; The objects-gc should remove unused fragments (let [res (th/run-task! :objects-gc {:min-age 0})] - (t/is (= 1 (:processed res)))) + (t/is (= 3 (:processed res)))) ;; Check the number of fragments (let [rows (th/db-query :file-data-fragment {:file-id (:id file)})] - (t/is (= 4 (count rows)))) + (t/is (= 2 (count rows)))) ;; Add shape to page that should add a new fragment (update-file! @@ -203,7 +207,7 @@ ;; Check the number of fragments (let [rows (th/db-query :file-data-fragment {:file-id (:id file)})] - (t/is (= 5 (count rows)))) + (t/is (= 3 (count rows)))) ;; The file-gc should mark for remove unused fragments (let [res (th/run-task! :file-gc {:min-age 0})] @@ -211,13 +215,13 @@ ;; The objects-gc should remove unused fragments (let [res (th/run-task! :objects-gc {:min-age 0})] - (t/is (= 1 (:processed res)))) + (t/is (= 3 (:processed res)))) ;; Check the number of fragments; should be 3 because changes ;; are also holding pointers to fragments; (let [rows (th/db-query :file-data-fragment {:file-id (:id file) :deleted-at nil})] - (t/is (= 6 (count rows)))) + (t/is (= 2 (count rows)))) ;; Lets proceed to delete all changes (th/db-delete! :file-change {:file-id (:id file)}) @@ -233,11 +237,11 @@ ;; Check the number of fragments; (let [rows (th/db-query :file-data-fragment {:file-id (:id file)})] ;; (pp/pprint rows) - (t/is (= 8 (count rows))) + (t/is (= 4 (count rows))) (t/is (= 2 (count (remove (comp some? :deleted-at) rows))))) (let [res (th/run-task! :objects-gc {:min-age 0})] - (t/is (= 6 (:processed res)))) + (t/is (= 2 (:processed res)))) (let [rows (th/db-query :file-data-fragment {:file-id (:id file)})] (t/is (= 2 (count rows))))))) @@ -338,7 +342,7 @@ (t/is (= 1 (count (remove (comp some? :deleted-at) rows))))) (let [res (th/run-task! :objects-gc {:min-age 0})] - (t/is (= 2 (:processed res)))) + (t/is (= 3 (:processed res)))) ;; check file media objects (let [rows (th/db-query :file-media-object {:file-id (:id file)})] @@ -367,7 +371,7 @@ (t/is (= 1 (:processed res)))) (let [res (th/run-task! :objects-gc {:min-age 0})] - (t/is (= 2 (:processed res)))) + (t/is (= 3 (:processed res)))) ;; Now that file-gc have deleted the file-media-object usage, ;; lets execute the touched-gc task, we should see that two of @@ -494,11 +498,11 @@ (t/is (= 1 (:processed res)))) (let [res (th/run-task! :objects-gc {:min-age 0})] - (t/is (= 1 (:processed res)))) + (t/is (= 2 (:processed res)))) (let [rows (th/db-query :file-data-fragment {:file-id (:id file) :deleted-at nil})] - (t/is (= (count rows) 2))) + (t/is (= (count rows) 1))) ;; retrieve file and check trimmed attribute (let [row (th/db-get :file {:id (:id file)})] @@ -535,11 +539,11 @@ (t/is (= 1 (:processed res)))) (let [res (th/run-task! :objects-gc {:min-age 0})] - (t/is (= 6 (:processed res)))) + (t/is (= 7 (:processed res)))) (let [rows (th/db-query :file-data-fragment {:file-id (:id file) :deleted-at nil})] - (t/is (= (count rows) 3))) + (t/is (= (count rows) 1))) ;; Now that file-gc have deleted the file-media-object usage, ;; lets execute the touched-gc task, we should see that two of @@ -702,7 +706,7 @@ ;; thumbnail lets execute the objects-gc task which remove ;; the rows and mark as touched the storage object rows (let [res (th/run-task! :objects-gc {:min-age 0})] - (t/is (= 3 (:processed res)))) + (t/is (= 5 (:processed res)))) ;; Now that objects-gc have deleted the object thumbnail lets ;; execute the touched-gc task @@ -732,7 +736,7 @@ (let [res (th/run-task! :objects-gc {:min-age 0})] ;; (pp/pprint res) - (t/is (= 2 (:processed res)))) + (t/is (= 3 (:processed res)))) ;; We still have th storage objects in the table (let [rows (th/db-query :storage-object {:deleted-at nil})] @@ -1168,7 +1172,7 @@ (t/is (= 1 (count (remove :deleted-at rows))))) (let [res (th/run-task! :objects-gc {:min-age 0})] - (t/is (= 3 (:processed res)))) + (t/is (= 4 (:processed res)))) (let [rows (th/db-query :file-tagged-object-thumbnail {:file-id (:id file)})] ;; (app.common.pprint/pprint rows) diff --git a/backend/test/backend_tests/rpc_file_thumbnails_test.clj b/backend/test/backend_tests/rpc_file_thumbnails_test.clj index e5cd918b1..c73941aff 100644 --- a/backend/test/backend_tests/rpc_file_thumbnails_test.clj +++ b/backend/test/backend_tests/rpc_file_thumbnails_test.clj @@ -118,7 +118,7 @@ (t/is (= 1 (:processed result)))) (let [result (th/run-task! :objects-gc {:min-age 0})] - (t/is (= 2 (:processed result)))) + (t/is (= 3 (:processed result)))) ;; check if row2 related thumbnail row still exists (let [[row :as rows] (th/db-query :file-tagged-object-thumbnail