diff --git a/.serena/memories/backend/subtleties.md b/.serena/memories/backend/subtleties.md index cee764853a..b6db941801 100644 --- a/.serena/memories/backend/subtleties.md +++ b/.serena/memories/backend/subtleties.md @@ -81,4 +81,5 @@ - Tell a snapshot apart by `label IS NOT NULL` (with `created_by` / `deleted_at` as needed), never by `file_change.data IS NOT NULL`: the label holds with every file data backend, and the payload column stopped meaning "this is a snapshot" when the payload moved out. Queries that identify versions must use the label. - The version quota counts of `app.rpc.quotes` (`sql:get-snapshots-per-file`, `sql:get-snapshots-per-team`) are served by the partial index `file_change__user_snapshots__idx`, whose predicate mirrors their where clause; changing one without the other drops the index only scan and the count walks the whole table again. - Revisions live only in `file_change` and have no `file_data` counterpart. They are born already scheduled for deletion one hour later (`app.rpc.commands.files-update`) and the snapshot paths cannot reach one, because they resolve their row through a `label IS NOT NULL` filter. -- A deletion date is assigned once and never moved: `restore-files` (`app.rpc.commands.files`) unmarks only durable history, the `:file` method of `app.tasks.delete-object` dates only rows that have no date yet, and `objects-gc` collects only rows that carry one. Unmarking a revision leaves it invisible to `objects-gc` for ever, because nothing sets that date again. \ No newline at end of file +- A deletion date is assigned once and never moved: `restore-files` (`app.rpc.commands.files`) unmarks only durable history, the `:file` method of `app.tasks.delete-object` dates only rows that have no date yet, and `objects-gc` collects only rows that carry one. Unmarking a revision leaves it invisible to `objects-gc` for ever, because nothing sets that date again. +- Binfile v1/v3 import and the v3 export `get-file` step run inside one long transaction. They raise `idle_in_transaction_session_timeout` to 30 min (`app.binfile.common/transaction-idle-timeout-ms`) instead of disabling it, so a connection left idle while the ZIP is decoded or the file data is realized is bounded; `statement_timeout` stays at 300 s. diff --git a/backend/src/app/binfile/common.clj b/backend/src/app/binfile/common.clj index 7fe8e6a532..34e93ae686 100644 --- a/backend/src/app/binfile/common.clj +++ b/backend/src/app/binfile/common.clj @@ -660,10 +660,28 @@ data library-ids))) -(defn disable-database-timeouts! +(def transaction-idle-timeout-ms + "Idle-in-transaction guard for the long binfile transactions: 30 + minutes. The import and the export of a file run inside one + transaction that can go quiet while the ZIP is decoded and the file + data is realized. The longest operation observed in production is + under 8 minutes, so this leaves room for bigger files and still + aborts a connection that is left idle for half an hour." + 1800000) + +(defn set-transaction-idle-timeout + "Set `idle_in_transaction_session_timeout` for the current + transaction. Must be called inside a transaction: `SET LOCAL` is + scoped to it." [cfg] (let [conn (db/get-connection cfg)] - (db/exec-one! conn ["SET LOCAL idle_in_transaction_session_timeout = 0"]) + (db/exec-one! conn [(str "SET LOCAL idle_in_transaction_session_timeout = " + transaction-idle-timeout-ms)]))) + +(defn configure-import-transaction + [cfg] + (set-transaction-idle-timeout cfg) + (let [conn (db/get-connection cfg)] (db/exec-one! conn ["SET CONSTRAINTS ALL DEFERRED"]))) (defn invalidate-thumbnails diff --git a/backend/src/app/binfile/v1.clj b/backend/src/app/binfile/v1.clj index fd340a487d..a8b1ea32c3 100644 --- a/backend/src/app/binfile/v1.clj +++ b/backend/src/app/binfile/v1.clj @@ -455,7 +455,7 @@ (defn- read-import-v1 [{:keys [::db/conn ::bfc/project-id ::bfc/profile-id ::bfc/input] :as cfg}] - (bfc/disable-database-timeouts! cfg) + (bfc/configure-import-transaction cfg) (pu/with-open [input (zstd-input-stream input) input (io/data-input-stream input)] diff --git a/backend/src/app/binfile/v3.clj b/backend/src/app/binfile/v3.clj index cdc3dd66bb..69c3e4781e 100644 --- a/backend/src/app/binfile/v3.clj +++ b/backend/src/app/binfile/v3.clj @@ -228,6 +228,7 @@ (let [detach? (and (not embed-assets) (not include-libraries))] (db/tx-run! cfg (fn [cfg] + (bfc/set-transaction-idle-timeout cfg) (cond-> (bfc/get-file cfg file-id {:realize? true :include-deleted? true @@ -934,7 +935,7 @@ (defn- import-files* [{:keys [::manifest] :as cfg}] - (bfc/disable-database-timeouts! cfg) + (bfc/configure-import-transaction cfg) (vswap! bfc/*state* update :index bfc/update-index (:files manifest) :id) @@ -962,7 +963,7 @@ :hint "unable to perform in-place update with binfile containing more than 1 file" :manifest manifest)) - (bfc/disable-database-timeouts! cfg) + (bfc/configure-import-transaction cfg) (let [ref-file (bfc/get-minimal-file cfg file-id ::db/for-update true) file (first (get manifest :files)) diff --git a/backend/test/backend_tests/binfile_test.clj b/backend/test/backend_tests/binfile_test.clj index 42edaaf4b5..61cf253c8e 100644 --- a/backend/test/backend_tests/binfile_test.clj +++ b/backend/test/backend_tests/binfile_test.clj @@ -390,3 +390,20 @@ d)))] (t/is (= :validation (:type out))) (t/is (= :max-file-size-reached (:code out)))))) + +(def ^:private sql:current-idle-timeout + "SELECT current_setting('idle_in_transaction_session_timeout')::interval + = interval '30 minutes' AS ok") + +(t/deftest binfile-transactions-use-a-finite-idle-timeout + (db/tx-run! th/*system* + (fn [{::db/keys [conn] :as cfg}] + (t/testing "export transaction" + (bfc/set-transaction-idle-timeout cfg) + (t/is (true? (-> (db/exec-one! conn [sql:current-idle-timeout]) + :ok)))) + + (t/testing "import transaction" + (bfc/configure-import-transaction cfg) + (t/is (true? (-> (db/exec-one! conn [sql:current-idle-timeout]) + :ok)))))))