diff --git a/example.env b/example.env index 1c46e18e..30c4093e 100644 --- a/example.env +++ b/example.env @@ -80,6 +80,13 @@ OXICLOUD_SERVER_HOST=127.0.0.1 # higher = less background DB work. Minimum enforced: 30s. #OXICLOUD_STORAGE_USAGE_RECONCILE_SECS=600 +# How often (milliseconds) the background job drains storage.tree_etag_dirty +# and bumps folder tree ETags (default: 500). Write paths only enqueue bump +# requests — this is the upper bound on how stale an ancestor folder's ETag +# (WebDAV/NextCloud collection sync signal) can be after a change. Lower = +# fresher sync detection, higher = fewer background UPDATEs. Minimum: 100. +#OXICLOUD_TREE_ETAG_FLUSH_MS=500 + # Allow multiple processes to bind to the same port (SO_REUSEPORT). # DISABLED by default — leaving this off means a second accidental instance # will fail immediately with "address already in use", which is the safe behaviour. diff --git a/migrations/20260627000000_async_tree_etag_queue.sql b/migrations/20260627000000_async_tree_etag_queue.sql new file mode 100644 index 00000000..998258f4 --- /dev/null +++ b/migrations/20260627000000_async_tree_etag_queue.sql @@ -0,0 +1,196 @@ +-- Asynchronous tree-ETag bumps: enqueue-only triggers + background flusher. +-- +-- The statement-level triggers from `20260626000000_tree_etag_statement_triggers` +-- still updated ancestor folder rows inside the writer's own transaction, +-- locking them with `SELECT … ORDER BY id FOR UPDATE`. That lock conflicts +-- with the FOR KEY SHARE lock the FK RI check on files.folder_id takes on +-- the parent folder row at INSERT time — a shared-to-exclusive upgrade on +-- the same hot row. Two concurrent uploads into one folder form a mutual +-- cycle; PostgreSQL's deadlock detector (deadlock_timeout = 1 s) aborts one +-- victim per cycle. Measured on a real bulk upload: 363 deadlocks, 31 % of +-- uploads failed with HTTP 500, throughput collapsed from a demonstrated +-- ~185 files/s burst to ~1 resolved transaction per second. +-- +-- The fix removes ALL folder-row locking from user-facing write paths: +-- +-- * The four bump functions now only INSERT the affected lpath targets +-- into `storage.tree_etag_dirty` — a plain heap append with no unique +-- constraints, taking zero shared row locks. Parallel uploads now only +-- share FOR KEY SHARE locks on the parent folder (mutually compatible), +-- so N-way concurrent writes cannot conflict, by construction. +-- * A single background task in the app (`TreeEtagFlushService`, on the +-- maintenance pool) drains the queue every ~500 ms and applies ONE +-- batched ancestor UPDATE with deterministic id-order locking. +-- +-- Semantics preserved from 20260626000000 (see that migration's header): +-- * depth guard: bumps fired from inside another trigger's DML +-- (FK cascades, the lpath cascade rewrite) are skipped; +-- * UPDATE value filters: only DAV-observable column changes count +-- (the EXIF media_sort_date sync never bumps); +-- * file moves cover the OLD parent chain as well as the NEW one; +-- * folder events bump STRICT ancestors only (self-exclusion). +-- +-- Semantic delta, decided deliberately: ancestor ETags become eventually +-- consistent (≤ ~1 flush interval after commit) instead of same-transaction. +-- No reader requires same-transaction freshness: collection ETags are only +-- compared across successive PROPFIND polls (seconds apart), and no mutation +-- response embeds a freshly bumped ANCESTOR etag (folder create/rename/move +-- responses return the row's OWN tree_modified_at, which bumps never touch). +-- The flusher orders its bump strictly after the writer's commit, so an ETag +-- can never change before the content that caused it is visible. + +-- ── Dirty queue ────────────────────────────────────────────────────── +-- Each row is one "bump request", dual-keyed: +-- * `lpath` — the target chain CAPTURED at mutation time. Covers +-- chains whose folders are deleted or moved away by +-- flush time (the old location's surviving ancestors +-- still get their bump — unrecoverable from an id). +-- * `folder_id` — the same target folder's id, re-resolved to its +-- CURRENT lpath at flush time. Covers the converse +-- race: a folder MOVED inside the flush window has its +-- whole subtree's lpaths rewritten, so the captured +-- lpath no longer prefix-matches it and the pending +-- bump would otherwise be silently lost (a sync client +-- would never discover the change). +-- The flusher bumps the inclusive ancestor closure of the UNION of both. +-- File events target their parent folder; folder events target their +-- PARENT (subpath drops the last label), preserving self-exclusion — +-- one uniform inclusive queue semantic. +-- +-- Append-only between flushes; the flusher deletes what it processes. +-- Logged (not UNLOGGED): a crash must not lose bumps. Duplicates are +-- expected and welcome; the flusher dedups. No FK on folder_id and no +-- indexes beyond the PK: enqueue must stay a pure append, +-- contention-free under any write concurrency. +CREATE TABLE IF NOT EXISTS storage.tree_etag_dirty ( + id BIGINT GENERATED ALWAYS AS IDENTITY PRIMARY KEY, + lpath ltree NOT NULL, + folder_id UUID +); + +-- Self-heal for databases that applied an earlier draft of THIS +-- migration (folder_id added after initial rollout of the queue). +ALTER TABLE storage.tree_etag_dirty ADD COLUMN IF NOT EXISTS folder_id UUID; + +-- ── File side: INSERT / DELETE ─────────────────────────────────────── +-- Both triggers alias their transition table to `changed_rows`; one body +-- serves both events. Reading fo.lpath is a plain MVCC read — no locks. +-- Root-level files (folder_id IS NULL) have no ancestors; a parent row +-- deleted in the same statement drops out of the JOIN (the folder-side +-- trigger of the outer statement covers the surviving ancestors). +CREATE OR REPLACE FUNCTION storage.bump_tree_from_files_stmt() +RETURNS TRIGGER LANGUAGE plpgsql AS $$ +BEGIN + IF pg_trigger_depth() > 1 THEN + RETURN NULL; + END IF; + + INSERT INTO storage.tree_etag_dirty (lpath, folder_id) + SELECT DISTINCT fo.lpath, fo.id + FROM (SELECT DISTINCT folder_id + FROM changed_rows + WHERE folder_id IS NOT NULL) c + JOIN storage.folders fo ON fo.id = c.folder_id; + + RETURN NULL; +END; +$$; + +-- ── File side: UPDATE ──────────────────────────────────────────────── +-- Union of OLD and NEW parent chains so a move invalidates both the +-- source and the destination collection ETags. The value filter keeps +-- the 20260626000000 semantics: only DAV-observable changes enqueue +-- (PostgreSQL forbids `AFTER UPDATE OF ` with transition tables, +-- so the filter lives here). +CREATE OR REPLACE FUNCTION storage.bump_tree_from_files_stmt_upd() +RETURNS TRIGGER LANGUAGE plpgsql AS $$ +BEGIN + IF pg_trigger_depth() > 1 THEN + RETURN NULL; + END IF; + + WITH changed AS ( + SELECT o.folder_id AS old_folder_id, n.folder_id AS new_folder_id + FROM old_rows o + JOIN new_rows n USING (id) + WHERE (o.name, o.folder_id, o.blob_hash, o.size, + o.mime_type, o.is_trashed, o.updated_at) + IS DISTINCT FROM + (n.name, n.folder_id, n.blob_hash, n.size, + n.mime_type, n.is_trashed, n.updated_at) + ) + INSERT INTO storage.tree_etag_dirty (lpath, folder_id) + SELECT DISTINCT fo.lpath, fo.id + FROM (SELECT old_folder_id AS folder_id + FROM changed WHERE old_folder_id IS NOT NULL + UNION + SELECT new_folder_id + FROM changed WHERE new_folder_id IS NOT NULL) c + JOIN storage.folders fo ON fo.id = c.folder_id; + + RETURN NULL; +END; +$$; + +-- ── Folder side: INSERT / DELETE ───────────────────────────────────── +-- Strict ancestors only: enqueue the PARENT's lpath (subpath drops the +-- last label), so the inclusive queue semantic covers parent + ancestors +-- but never the folder itself. Root folders (nlevel = 1) enqueue nothing. +-- The lpath value is captured HERE, at mutation time — by flush time the +-- row may be gone (purge) or rewritten (move), and the OLD chain would +-- be unrecoverable from a folder id. +CREATE OR REPLACE FUNCTION storage.bump_tree_from_folders_stmt() +RETURNS TRIGGER LANGUAGE plpgsql AS $$ +BEGIN + IF pg_trigger_depth() > 1 THEN + RETURN NULL; + END IF; + + INSERT INTO storage.tree_etag_dirty (lpath, folder_id) + SELECT DISTINCT subpath(lpath, 0, nlevel(lpath) - 1), parent_id + FROM changed_rows + WHERE lpath IS NOT NULL + AND nlevel(lpath) > 1; + + RETURN NULL; +END; +$$; + +-- ── Folder side: UPDATE ────────────────────────────────────────────── +-- Union of OLD and NEW parent chains (a move bumps the chain it left and +-- the chain it joined), same value filter as 20260626000000: descendant +-- path/lpath rewrites by `trg_folders_cascade_path` change none of the +-- compared columns and additionally run at depth 2. +CREATE OR REPLACE FUNCTION storage.bump_tree_from_folders_stmt_upd() +RETURNS TRIGGER LANGUAGE plpgsql AS $$ +BEGIN + IF pg_trigger_depth() > 1 THEN + RETURN NULL; + END IF; + + WITH changed AS ( + SELECT o.lpath AS old_lpath, o.parent_id AS old_parent_id, + n.lpath AS new_lpath, n.parent_id AS new_parent_id + FROM old_rows o + JOIN new_rows n USING (id) + WHERE (o.name, o.parent_id, o.is_trashed, o.updated_at) + IS DISTINCT FROM + (n.name, n.parent_id, n.is_trashed, n.updated_at) + ) + INSERT INTO storage.tree_etag_dirty (lpath, folder_id) + SELECT DISTINCT subpath(c.lpath, 0, nlevel(c.lpath) - 1), c.parent_id + FROM (SELECT old_lpath AS lpath, old_parent_id AS parent_id + FROM changed WHERE old_lpath IS NOT NULL + UNION + SELECT new_lpath, new_parent_id + FROM changed WHERE new_lpath IS NOT NULL) c + WHERE nlevel(c.lpath) > 1; + + RETURN NULL; +END; +$$; + +-- The six statement-level triggers from 20260626000000 keep their names +-- and wiring — they reference these functions by name, so replacing the +-- bodies above is the whole swap. `trg_folders_cascade_path` and the +-- BEFORE path triggers are untouched. diff --git a/src/common/config.rs b/src/common/config.rs index f3c89561..f620dad3 100644 --- a/src/common/config.rs +++ b/src/common/config.rs @@ -248,6 +248,12 @@ pub struct StorageConfig { /// quota fresh for all mutations without recomputing on the request path. /// Default: 600 (10 min). Env: `OXICLOUD_STORAGE_USAGE_RECONCILE_SECS`. pub usage_reconcile_secs: u64, + /// Interval (milliseconds) of the background job that drains + /// `storage.tree_etag_dirty` and bumps folder `tree_modified_at` + /// (collection ETags). Write paths only enqueue — this is the upper + /// bound on how stale an ancestor folder's ETag can be after a change. + /// Default: 500. Env: `OXICLOUD_TREE_ETAG_FLUSH_MS`. + pub tree_etag_flush_ms: u64, /// Which blob storage backend to use (`local`, `s3`, or `azure`). pub backend: StorageBackendType, /// S3-compatible backend configuration (used when `backend == S3`). @@ -390,6 +396,7 @@ impl Default for StorageConfig { upload_temp_dir: None, chunk_dir: None, usage_reconcile_secs: 600, // 10 minutes + tree_etag_flush_ms: 500, backend: StorageBackendType::Local, s3: None, azure: None, @@ -1291,6 +1298,13 @@ impl AppConfig { config.storage.usage_reconcile_secs = val; } + // Tree-ETag dirty-queue flush cadence + if let Ok(ms) = env::var("OXICLOUD_TREE_ETAG_FLUSH_MS").map(|v| v.parse::()) + && let Ok(val) = ms + { + config.storage.tree_etag_flush_ms = val; + } + // Storage backend selection if let Ok(backend) = env::var("OXICLOUD_STORAGE_BACKEND") { match backend.to_lowercase().as_str() { diff --git a/src/common/di.rs b/src/common/di.rs index 5a07193f..9e694792 100644 --- a/src/common/di.rs +++ b/src/common/di.rs @@ -657,6 +657,25 @@ impl AppServiceFactory { service } + /// Starts the tree-ETag flush job (requires database). + /// + /// The statement triggers on `storage.files`/`storage.folders` only + /// enqueue bump requests into `storage.tree_etag_dirty` (so user-facing + /// writes take no folder-row locks); this job is the single drainer that + /// turns them into `tree_modified_at` updates. It must run whenever the + /// database is up — the triggers are always installed, and an undrained + /// queue grows unboundedly while folder ETags freeze. Fire-and-forget on + /// the maintenance pool, like the trash cleanup job. + fn start_tree_etag_flush_job(&self, maintenance_pool: &Arc) { + let service = + crate::infrastructure::services::tree_etag_flush_service::TreeEtagFlushService::new( + maintenance_pool.clone(), + self.config.storage.tree_etag_flush_ms, + ); + service.start_flush_job(); + tracing::info!("Tree-ETag flush service initialized"); + } + /// Builds the complete AppState using all factory services. /// /// This is the main entry point that replaces all manual logic in `main.rs`. @@ -745,6 +764,8 @@ impl AppServiceFactory { storage_usage_service = Some(self.create_storage_usage_service(&repos, &pool, &maintenance_pool)); + self.start_tree_etag_flush_job(&maintenance_pool); + // User-lifecycle dispatcher. Hook order is registration order; // document dependencies inline if/when any arise. Today: // 1. AuditLifecycleHook — fires first so the diff --git a/src/domain/entities/folder.rs b/src/domain/entities/folder.rs index b08c0c76..6607b917 100644 --- a/src/domain/entities/folder.rs +++ b/src/domain/entities/folder.rs @@ -227,11 +227,15 @@ impl Folder { self.owner_id } - /// Latest descendant-write timestamp, maintained by a Postgres - /// trigger that walks the ltree ancestor chain on every file or - /// folder write inside this folder's subtree. See migration - /// `20260625000000_folder_tree_modified_at.sql` for the trigger - /// definition. + /// Latest descendant-write timestamp. Statement-level Postgres + /// triggers enqueue every file/folder write into + /// `storage.tree_etag_dirty`; the background `TreeEtagFlushService` + /// drains the queue (default every 500 ms) and bumps the ltree + /// ancestor chain in one batched UPDATE. Eventually consistent: + /// the bump lands within ~one flush interval AFTER the change is + /// committed, never before. See migration + /// `20260627000000_async_tree_etag_queue.sql` for the rationale + /// (user-facing writes must take zero folder-row locks). pub fn tree_modified_at(&self) -> u64 { self.tree_modified_at } @@ -257,19 +261,26 @@ impl Folder { /// - The 16-char UUID prefix gives the folder its identity /// component — keeps two empty same-mtime folders distinct. /// - `tree_modified_at` (Unix seconds) is the actual signal: - /// bumped by trigger whenever ANY descendant (file or - /// sub-folder, at any depth) is created, modified, deleted, - /// or moved. This is the contract NextCloud's sync engine - /// relies on — "did anything change inside this collection - /// since I last looked?". Until this column existed, the - /// answer was always "no" because the folder UUID never - /// changed; clients had to do periodic deep PROPFIND walks - /// to discover web-uploaded files. + /// bumped whenever ANY descendant (file or sub-folder, at any + /// depth) is created, modified, deleted, or moved. This is the + /// contract NextCloud's sync engine relies on — "did anything + /// change inside this collection since I last looked?". Until + /// this column existed, the answer was always "no" because the + /// folder UUID never changed; clients had to do periodic deep + /// PROPFIND walks to discover web-uploaded files. + /// - The bump is asynchronous: triggers enqueue, the + /// `TreeEtagFlushService` applies (≤ ~one flush interval after + /// commit, monotonic — two flushes in the same wall-clock + /// second still yield distinct values). Clients only compare + /// etags across successive polls, so the short lag is + /// unobservable; what matters is the bump never precedes the + /// change becoming visible. /// - Renaming the folder itself does NOT change the etag's - /// identity portion (UUID is stable across renames). The - /// trigger does bump `tree_modified_at` on rename via the - /// folder-side trigger, so the etag still changes — which is - /// correct, the parent collection's listing changed. + /// identity portion (UUID is stable across renames). A rename + /// does enqueue the ancestor chain, so the PARENT's etag still + /// changes — which is correct, the parent collection's listing + /// changed; the folder's own value stays untouched + /// (self-exclusion). pub fn compute_etag(id: &str, tree_modified_at: u64) -> String { let prefix: String = id.chars().take(16).collect(); format!("{}-{}", prefix, tree_modified_at) diff --git a/src/infrastructure/repositories/pg/file_blob_write_repository.rs b/src/infrastructure/repositories/pg/file_blob_write_repository.rs index 224b89bc..185509bf 100644 --- a/src/infrastructure/repositories/pg/file_blob_write_repository.rs +++ b/src/infrastructure/repositories/pg/file_blob_write_repository.rs @@ -20,6 +20,7 @@ use crate::domain::entities::file::File; use crate::domain::services::path_service::StoragePath; use super::folder_db_repository::FolderDbRepository; +use super::transaction_utils::retry_on_deadlock; use crate::infrastructure::services::dedup_service::DedupService; /// File write repository backed by PostgreSQL metadata + blob storage. @@ -157,24 +158,28 @@ impl FileBlobWriteRepository { modified_at: Option, ) -> Result<(String, i64), DomainError> { // Atomic CTE: capture old hash then update in one round-trip, no TOCTOU. - let (old_hash, updated_at) = match sqlx::query_as::<_, (String, i64)>( - r#" - WITH old AS ( - SELECT id, blob_hash FROM storage.files WHERE id = $3::uuid FOR UPDATE + // Deadlock victims (40P01) retry before the compensation below runs — + // a successful retry must keep the new blob reference alive. + let (old_hash, updated_at) = match retry_on_deadlock("files.swap_blob_hash", || { + sqlx::query_as::<_, (String, i64)>( + r#" + WITH old AS ( + SELECT id, blob_hash FROM storage.files WHERE id = $3::uuid FOR UPDATE + ) + UPDATE storage.files f + SET blob_hash = $1, size = $2, + updated_at = COALESCE(to_timestamp($4), NOW()) + FROM old + WHERE f.id = old.id + RETURNING old.blob_hash, EXTRACT(EPOCH FROM f.updated_at)::bigint + "#, ) - UPDATE storage.files f - SET blob_hash = $1, size = $2, - updated_at = COALESCE(to_timestamp($4), NOW()) - FROM old - WHERE f.id = old.id - RETURNING old.blob_hash, EXTRACT(EPOCH FROM f.updated_at)::bigint - "#, - ) - .bind(new_hash) - .bind(new_size) - .bind(file_id) - .bind(modified_at.map(|t| t as f64)) - .fetch_optional(self.pool.as_ref()) + .bind(new_hash) + .bind(new_size) + .bind(file_id) + .bind(modified_at.map(|t| t as f64)) + .fetch_optional(self.pool.as_ref()) + }) .await { Ok(Some(row)) => row, @@ -236,23 +241,30 @@ impl FileBlobWriteRepository { let is_new_blob = !dedup_result.was_deduplicated(); let blob_hash = dedup_result.hash().to_string(); - let row = match sqlx::query_as::<_, (String, i64, i64)>( - r#" - INSERT INTO storage.files (name, folder_id, user_id, blob_hash, size, mime_type, category_order) - VALUES ($1, $2::uuid, $3, $4, $5, $6, $7) - RETURNING id::text, - EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint - "#, - ) - .bind(&name) - .bind(&folder_id) - .bind(user_id) - .bind(&blob_hash) - .bind(size as i64) - .bind(&content_type) - .bind(category_order_for(&name, &content_type)) - .fetch_one(self.pool.as_ref()) + // Deadlock victims (40P01) retry before the compensation below runs — + // a successful retry must keep the blob reference alive. The final + // attempt's error falls through untouched so the 23505 mapping holds + // (a retried INSERT can legitimately lose to a concurrent identical + // upload). + let row = match retry_on_deadlock("files.insert", || { + sqlx::query_as::<_, (String, i64, i64)>( + r#" + INSERT INTO storage.files (name, folder_id, user_id, blob_hash, size, mime_type, category_order) + VALUES ($1, $2::uuid, $3, $4, $5, $6, $7) + RETURNING id::text, + EXTRACT(EPOCH FROM created_at)::bigint, + EXTRACT(EPOCH FROM updated_at)::bigint + "#, + ) + .bind(&name) + .bind(&folder_id) + .bind(user_id) + .bind(&blob_hash) + .bind(size as i64) + .bind(&content_type) + .bind(category_order_for(&name, &content_type)) + .fetch_one(self.pool.as_ref()) + }) .await { Ok(row) => row, @@ -372,46 +384,48 @@ impl FileWritePort for FileBlobWriteRepository { // Single round-trip; blob content is NOT copied (dedup makes this zero-copy). let target_fid = target_folder_id.clone(); - let row = sqlx::query_as::< - _, - ( - String, - String, - Option, - i64, - String, - i64, - i64, - String, - ), - >( - r#" - WITH src AS ( - SELECT name, folder_id, user_id, blob_hash, size, mime_type, category_order - FROM storage.files - WHERE id = $1::uuid AND NOT is_trashed - ), - new_file AS ( - INSERT INTO storage.files (name, folder_id, user_id, blob_hash, size, mime_type, category_order) - SELECT name, - COALESCE($2::uuid, folder_id), - user_id, - blob_hash, - size, - mime_type, - category_order - FROM src - RETURNING id::text, name, folder_id::text, size, mime_type, - EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint, - blob_hash + let row = retry_on_deadlock("files.copy", || { + sqlx::query_as::< + _, + ( + String, + String, + Option, + i64, + String, + i64, + i64, + String, + ), + >( + r#" + WITH src AS ( + SELECT name, folder_id, user_id, blob_hash, size, mime_type, category_order + FROM storage.files + WHERE id = $1::uuid AND NOT is_trashed + ), + new_file AS ( + INSERT INTO storage.files (name, folder_id, user_id, blob_hash, size, mime_type, category_order) + SELECT name, + COALESCE($2::uuid, folder_id), + user_id, + blob_hash, + size, + mime_type, + category_order + FROM src + RETURNING id::text, name, folder_id::text, size, mime_type, + EXTRACT(EPOCH FROM created_at)::bigint, + EXTRACT(EPOCH FROM updated_at)::bigint, + blob_hash + ) + SELECT * FROM new_file + "#, ) - SELECT * FROM new_file - "#, - ) - .bind(file_id) - .bind(&target_fid) - .fetch_optional(self.pool.as_ref()) + .bind(file_id) + .bind(&target_fid) + .fetch_optional(self.pool.as_ref()) + }) .await .map_err(|e| { if let sqlx::Error::Database(ref db_err) = e @@ -557,23 +571,25 @@ impl FileWritePort for FileBlobWriteRepository { // The write-behind cache will call update_file_content later. let placeholder_hash = "0000000000000000000000000000000000000000000000000000000000000000"; - let row = sqlx::query_as::<_, (String, i64, i64)>( - r#" - INSERT INTO storage.files (name, folder_id, user_id, blob_hash, size, mime_type, category_order) - VALUES ($1, $2::uuid, $3, $4, $5, $6, $7) - RETURNING id::text, - EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint - "#, - ) - .bind(&name) - .bind(&folder_id) - .bind(user_id) - .bind(placeholder_hash) - .bind(size as i64) - .bind(&content_type) - .bind(category_order_for(&name, &content_type)) - .fetch_one(self.pool.as_ref()) + let row = retry_on_deadlock("files.insert_deferred", || { + sqlx::query_as::<_, (String, i64, i64)>( + r#" + INSERT INTO storage.files (name, folder_id, user_id, blob_hash, size, mime_type, category_order) + VALUES ($1, $2::uuid, $3, $4, $5, $6, $7) + RETURNING id::text, + EXTRACT(EPOCH FROM created_at)::bigint, + EXTRACT(EPOCH FROM updated_at)::bigint + "#, + ) + .bind(&name) + .bind(&folder_id) + .bind(user_id) + .bind(placeholder_hash) + .bind(size as i64) + .bind(&content_type) + .bind(category_order_for(&name, &content_type)) + .fetch_one(self.pool.as_ref()) + }) .await .map_err(|e| DomainError::internal_error("FileBlobWrite", format!("deferred: {e}")))?; diff --git a/src/infrastructure/repositories/pg/folder_db_repository.rs b/src/infrastructure/repositories/pg/folder_db_repository.rs index ea6b3466..521388d9 100644 --- a/src/infrastructure/repositories/pg/folder_db_repository.rs +++ b/src/infrastructure/repositories/pg/folder_db_repository.rs @@ -12,6 +12,7 @@ use sqlx::PgPool; use std::sync::Arc; use uuid::Uuid; +use super::transaction_utils::retry_on_deadlock; use crate::application::dtos::folder_dto::{FolderResourceCursor, FolderResourceRow}; use crate::common::errors::DomainError; use crate::domain::entities::folder::Folder; @@ -450,20 +451,25 @@ impl FolderRepository for FolderDbRepository { // The BEFORE UPDATE trigger recomputes path/lpath for this row; // the AFTER UPDATE cascade trigger then batch-updates all // descendants in a single UPDATE using the GiST lpath index. - let row = sqlx::query_as::<_, FolderRow>( - r#" - UPDATE storage.folders - SET name = $1, updated_at = NOW() - WHERE id = $2::uuid AND NOT is_trashed - RETURNING id::text, name, path, parent_id::text, user_id, - EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint, - EXTRACT(EPOCH FROM tree_modified_at)::bigint - "#, - ) - .bind(&new_name) - .bind(id) - .fetch_optional(self.pool()) + // That multi-row rewrite can deadlock against the tree-ETag + // flusher's id-ordered ancestor bump — retry instead of failing + // the user's operation (40P01 only; 23505 still maps below). + let row = retry_on_deadlock("folders.rename", || { + sqlx::query_as::<_, FolderRow>( + r#" + UPDATE storage.folders + SET name = $1, updated_at = NOW() + WHERE id = $2::uuid AND NOT is_trashed + RETURNING id::text, name, path, parent_id::text, user_id, + EXTRACT(EPOCH FROM created_at)::bigint, + EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint + "#, + ) + .bind(&new_name) + .bind(id) + .fetch_optional(self.pool()) + }) .await .map_err(|e| { if let sqlx::Error::Database(ref db_err) = e @@ -486,20 +492,23 @@ impl FolderRepository for FolderDbRepository { // The BEFORE UPDATE trigger recomputes path/lpath for this row; // the AFTER UPDATE cascade trigger then batch-updates all // descendants in a single UPDATE using the GiST lpath index. - let row = sqlx::query_as::<_, FolderRow>( - r#" - UPDATE storage.folders - SET parent_id = $1::uuid, updated_at = NOW() - WHERE id = $2::uuid AND NOT is_trashed - RETURNING id::text, name, path, parent_id::text, user_id, - EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint, - EXTRACT(EPOCH FROM tree_modified_at)::bigint - "#, - ) - .bind(new_parent_id) - .bind(id) - .fetch_optional(self.pool()) + // Retried on deadlock vs the tree-ETag flusher (see rename_folder). + let row = retry_on_deadlock("folders.move", || { + sqlx::query_as::<_, FolderRow>( + r#" + UPDATE storage.folders + SET parent_id = $1::uuid, updated_at = NOW() + WHERE id = $2::uuid AND NOT is_trashed + RETURNING id::text, name, path, parent_id::text, user_id, + EXTRACT(EPOCH FROM created_at)::bigint, + EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint + "#, + ) + .bind(new_parent_id) + .bind(id) + .fetch_optional(self.pool()) + }) .await .map_err(|e| DomainError::internal_error("FolderDb", format!("move: {e}")))? .ok_or_else(|| DomainError::not_found("Folder", id))?; @@ -511,24 +520,30 @@ impl FolderRepository for FolderDbRepository { // Delete all files whose folder is anywhere in the subtree. // Uses the GiST-indexed ltree `<@` operator — O(log N) vs the // O(depth × N) recursive CTE it replaces. - sqlx::query( - "DELETE FROM storage.files \ - WHERE folder_id IN ( \ - SELECT id FROM storage.folders \ - WHERE lpath <@ (SELECT lpath FROM storage.folders WHERE id = $1::uuid) \ - )", - ) - .bind(id) - .execute(self.pool()) + // Both statements retried on deadlock vs the tree-ETag flusher's + // id-ordered ancestor bump (multi-row exclusive locks). + retry_on_deadlock("folders.delete_files", || { + sqlx::query( + "DELETE FROM storage.files \ + WHERE folder_id IN ( \ + SELECT id FROM storage.folders \ + WHERE lpath <@ (SELECT lpath FROM storage.folders WHERE id = $1::uuid) \ + )", + ) + .bind(id) + .execute(self.pool()) + }) .await .map_err(|e| DomainError::internal_error("FolderDb", format!("delete files: {e}")))?; // Then delete the folder (CASCADE will remove descendant folders) - let result = sqlx::query("DELETE FROM storage.folders WHERE id = $1::uuid") - .bind(id) - .execute(self.pool()) - .await - .map_err(|e| DomainError::internal_error("FolderDb", format!("delete: {e}")))?; + let result = retry_on_deadlock("folders.delete", || { + sqlx::query("DELETE FROM storage.folders WHERE id = $1::uuid") + .bind(id) + .execute(self.pool()) + }) + .await + .map_err(|e| DomainError::internal_error("FolderDb", format!("delete: {e}")))?; if result.rows_affected() == 0 { return Err(DomainError::not_found("Folder", id)); @@ -570,22 +585,24 @@ impl FolderRepository for FolderDbRepository { // Child files and sub-folders are implicitly hidden because their // ancestor is trashed — list queries already filter NOT is_trashed, // and folder navigation won't reach a trashed folder's children. - let result = sqlx::query_scalar::<_, i64>( - r#" - WITH trash_folder AS ( - UPDATE storage.folders - SET is_trashed = TRUE, - trashed_at = NOW(), - original_parent_id = parent_id, - updated_at = NOW() - WHERE id = $1::uuid AND NOT is_trashed - RETURNING 1 + let result = retry_on_deadlock("folders.trash", || { + sqlx::query_scalar::<_, i64>( + r#" + WITH trash_folder AS ( + UPDATE storage.folders + SET is_trashed = TRUE, + trashed_at = NOW(), + original_parent_id = parent_id, + updated_at = NOW() + WHERE id = $1::uuid AND NOT is_trashed + RETURNING 1 + ) + SELECT COUNT(*) FROM trash_folder + "#, ) - SELECT COUNT(*) FROM trash_folder - "#, - ) - .bind(folder_id) - .fetch_one(self.pool()) + .bind(folder_id) + .fetch_one(self.pool()) + }) .await .map_err(|e| DomainError::internal_error("FolderDb", format!("trash: {e}")))?; @@ -607,23 +624,25 @@ impl FolderRepository for FolderDbRepository { // The BEFORE UPDATE trigger recomputes path/lpath when // original_parent_id is restored; the cascade trigger // batch-updates all descendants via the GiST lpath index. - let result = sqlx::query_scalar::<_, i64>( - r#" - WITH restore_folder AS ( - UPDATE storage.folders - SET is_trashed = FALSE, - trashed_at = NULL, - parent_id = COALESCE(original_parent_id, parent_id), - original_parent_id = NULL, - updated_at = NOW() - WHERE id = $1::uuid AND is_trashed - RETURNING 1 + let result = retry_on_deadlock("folders.restore", || { + sqlx::query_scalar::<_, i64>( + r#" + WITH restore_folder AS ( + UPDATE storage.folders + SET is_trashed = FALSE, + trashed_at = NULL, + parent_id = COALESCE(original_parent_id, parent_id), + original_parent_id = NULL, + updated_at = NOW() + WHERE id = $1::uuid AND is_trashed + RETURNING 1 + ) + SELECT COUNT(*) FROM restore_folder + "#, ) - SELECT COUNT(*) FROM restore_folder - "#, - ) - .bind(folder_id) - .fetch_one(self.pool()) + .bind(folder_id) + .fetch_one(self.pool()) + }) .await .map_err(|e| DomainError::internal_error("FolderDb", format!("restore: {e}")))?; @@ -636,25 +655,30 @@ impl FolderRepository for FolderDbRepository { async fn delete_folder_permanently(&self, folder_id: &str) -> Result<(), DomainError> { // Delete all files whose folder is anywhere in the subtree - // (GiST ltree index, same pattern as delete_folder). - sqlx::query( - "DELETE FROM storage.files \ - WHERE folder_id IN ( \ - SELECT id FROM storage.folders \ - WHERE lpath <@ (SELECT lpath FROM storage.folders WHERE id = $1::uuid) \ - )", - ) - .bind(folder_id) - .execute(self.pool()) + // (GiST ltree index, same pattern as delete_folder — both + // statements retried on deadlock vs the tree-ETag flusher). + retry_on_deadlock("folders.perm_delete_files", || { + sqlx::query( + "DELETE FROM storage.files \ + WHERE folder_id IN ( \ + SELECT id FROM storage.folders \ + WHERE lpath <@ (SELECT lpath FROM storage.folders WHERE id = $1::uuid) \ + )", + ) + .bind(folder_id) + .execute(self.pool()) + }) .await .map_err(|e| DomainError::internal_error("FolderDb", format!("perm delete files: {e}")))?; // Then permanently delete folder — CASCADE handles descendant folders - let result = sqlx::query("DELETE FROM storage.folders WHERE id = $1::uuid") - .bind(folder_id) - .execute(self.pool()) - .await - .map_err(|e| DomainError::internal_error("FolderDb", format!("perm delete: {e}")))?; + let result = retry_on_deadlock("folders.perm_delete", || { + sqlx::query("DELETE FROM storage.folders WHERE id = $1::uuid") + .bind(folder_id) + .execute(self.pool()) + }) + .await + .map_err(|e| DomainError::internal_error("FolderDb", format!("perm delete: {e}")))?; if result.rows_affected() == 0 { return Err(DomainError::not_found("Folder", folder_id)); diff --git a/src/infrastructure/repositories/pg/mod.rs b/src/infrastructure/repositories/pg/mod.rs index 6a1aaf22..98f49aee 100644 --- a/src/infrastructure/repositories/pg/mod.rs +++ b/src/infrastructure/repositories/pg/mod.rs @@ -16,7 +16,7 @@ mod session_pg_repository; mod settings_pg_repository; mod share_pg_repository; mod subject_group_pg_repository; -mod transaction_utils; +pub(crate) mod transaction_utils; mod user_pg_repository; // ── Blob-storage repositories ── diff --git a/src/infrastructure/repositories/pg/transaction_utils.rs b/src/infrastructure/repositories/pg/transaction_utils.rs index bfc5ee82..7d7b6d99 100644 --- a/src/infrastructure/repositories/pg/transaction_utils.rs +++ b/src/infrastructure/repositories/pg/transaction_utils.rs @@ -1,6 +1,7 @@ use sqlx::{Error as SqlxError, PgPool, Postgres, Transaction}; +use std::future::Future; use std::sync::Arc; -use tracing::{debug, error, info}; +use tracing::{debug, error, info, warn}; /// Helper function to execute database operations in a transaction /// Takes a database pool and a closure that will be executed within a transaction @@ -56,3 +57,128 @@ where } } } + +/// True when the error is a PostgreSQL deadlock abort (SQLSTATE `40P01`). +/// +/// Deadlock victims are safe to re-run when the statement is a single +/// autocommit round-trip: the aborted implicit transaction left nothing +/// behind, and PostgreSQL chose this session as the victim precisely so +/// the competing transaction could finish — a retry usually succeeds +/// immediately. +pub fn is_deadlock(err: &SqlxError) -> bool { + matches!(err, SqlxError::Database(db) if db.code().as_deref() == Some("40P01")) +} + +/// Re-run `op` while it fails with an error matching `should_retry`, up to +/// 3 retries with a short growing backoff. Errors that don't match the +/// predicate — and the final attempt's error — are returned untouched, so +/// callers' existing error mapping (e.g. `23505` → already-exists) still +/// sees exactly what it expects. +pub async fn retry_when( + operation_name: &str, + should_retry: P, + op: F, +) -> Result +where + F: Fn() -> Fut, + Fut: Future>, + P: Fn(&E) -> bool, +{ + const BACKOFF_MS: [u64; 3] = [10, 50, 150]; + + let mut attempt = 0; + loop { + match op().await { + Ok(value) => return Ok(value), + Err(e) if attempt < BACKOFF_MS.len() && should_retry(&e) => { + warn!( + "Retryable failure on {} (attempt {}/{}), backing off {}ms", + operation_name, + attempt + 1, + BACKOFF_MS.len() + 1, + BACKOFF_MS[attempt] + ); + tokio::time::sleep(std::time::Duration::from_millis(BACKOFF_MS[attempt])).await; + attempt += 1; + } + Err(e) => return Err(e), + } + } +} + +/// [`retry_when`] specialised to PostgreSQL deadlocks (`40P01`) — the only +/// transient SQLSTATE our single-statement write paths can hit. +pub async fn retry_on_deadlock(operation_name: &str, op: F) -> Result +where + F: Fn() -> Fut, + Fut: Future>, +{ + retry_when(operation_name, is_deadlock, op).await +} + +#[cfg(test)] +mod tests { + use super::retry_when; + use std::sync::atomic::{AtomicU32, Ordering}; + + #[derive(Debug, PartialEq)] + enum FakeError { + Transient, + Fatal, + } + + #[tokio::test] + async fn retries_transient_errors_until_success() { + let calls = AtomicU32::new(0); + let result = retry_when( + "test", + |e| *e == FakeError::Transient, + || { + let n = calls.fetch_add(1, Ordering::SeqCst); + async move { + if n < 2 { + Err(FakeError::Transient) + } else { + Ok(n) + } + } + }, + ) + .await; + assert_eq!(result, Ok(2)); + assert_eq!(calls.load(Ordering::SeqCst), 3); + } + + #[tokio::test] + async fn gives_up_after_max_attempts_returning_last_error() { + let calls = AtomicU32::new(0); + let result: Result<(), _> = retry_when( + "test", + |e| *e == FakeError::Transient, + || { + calls.fetch_add(1, Ordering::SeqCst); + async { Err(FakeError::Transient) } + }, + ) + .await; + assert_eq!(result, Err(FakeError::Transient)); + // 1 initial attempt + 3 retries + assert_eq!(calls.load(Ordering::SeqCst), 4); + } + + #[tokio::test] + async fn non_matching_errors_are_not_retried() { + let calls = AtomicU32::new(0); + let result: Result<(), _> = retry_when( + "test", + |e| *e == FakeError::Transient, + || { + calls.fetch_add(1, Ordering::SeqCst); + async { Err(FakeError::Fatal) } + }, + ) + .await; + assert_eq!(result, Err(FakeError::Fatal)); + assert_eq!(calls.load(Ordering::SeqCst), 1); + } +} diff --git a/src/infrastructure/services/mod.rs b/src/infrastructure/services/mod.rs index 3accf192..73144245 100644 --- a/src/infrastructure/services/mod.rs +++ b/src/infrastructure/services/mod.rs @@ -29,6 +29,7 @@ pub mod thumbnail_service; #[cfg(test)] mod thumbnail_service_test; pub mod trash_cleanup_service; +pub mod tree_etag_flush_service; pub mod webdav_lock_service; pub mod wopi_discovery_service; pub mod zip_service; diff --git a/src/infrastructure/services/tree_etag_flush_service.rs b/src/infrastructure/services/tree_etag_flush_service.rs new file mode 100644 index 00000000..81572aa2 --- /dev/null +++ b/src/infrastructure/services/tree_etag_flush_service.rs @@ -0,0 +1,173 @@ +use sqlx::PgPool; +use std::sync::Arc; +use tracing::{debug, error, info, instrument}; + +use crate::infrastructure::repositories::pg::transaction_utils::retry_on_deadlock; + +/// Background drainer for `storage.tree_etag_dirty` — the asynchronous half +/// of the tree-ETag bump (see migration `20260627000000_async_tree_etag_queue`). +/// +/// The statement triggers on `storage.files` / `storage.folders` only append +/// "bump request" lpaths to the queue, taking zero folder-row locks, so any +/// number of concurrent uploads never conflict with each other. This service +/// turns those requests into actual `tree_modified_at` updates: every +/// `interval_ms` it drains the queue and bumps the deduplicated ancestor +/// closure in ONE batched UPDATE with deterministic id-order locking. +/// +/// Correctness invariants this relies on: +/// * The drain (DELETE) and the bump (UPDATE) are one statement — one +/// transaction. A crash mid-flush rolls both back and the queue rows +/// survive; the first flush after startup drains any leftovers, so no +/// bump is ever lost (a lost bump means sync clients never discover +/// the change). +/// * Queue rows are dual-keyed (`lpath` captured at mutation time, +/// `folder_id` re-resolved at flush time) and the target set is the +/// UNION of both — so neither a folder deleted/moved away (captured +/// lpath wins) nor a folder re-rooted by a move inside the flush +/// window (resolved lpath wins) can lose its pending bump. +/// * The bump is monotonic per folder: +/// `GREATEST(NOW(), tree_modified_at + interval '1 second')`. Folder +/// ETags have whole-second granularity (`{id}-{epoch_seconds}`, see +/// `Folder::compute_etag`); two flushes inside the same wall-clock +/// second must still produce two distinct ETags or a client polling +/// between them would permanently miss the second change. +/// * The flusher is the ONLY writer of `tree_modified_at`, runs as a +/// single instance, and locks victims in id order — so it cannot +/// deadlock against itself. It can still lose a race against a folder +/// move's descendant lpath cascade, which is why the statement runs +/// under [`retry_on_deadlock`]; a retried flush re-drains the intact +/// queue, invisible to users. +pub struct TreeEtagFlushService { + pool: Arc, + interval_ms: u64, +} + +/// Rows drained from the queue per statement. Bounds the lock footprint of +/// one flush under burst load (bulk trash purge, recursive copy); leftovers +/// are picked up by the in-tick drain loop or the next tick. +const DRAIN_BATCH: i64 = 5_000; + +/// Max drain statements per tick, so a pathological backlog cannot +/// monopolise the maintenance connection forever within one tick. +const MAX_BATCHES_PER_TICK: u32 = 8; + +impl TreeEtagFlushService { + pub fn new(pool: Arc, interval_ms: u64) -> Self { + Self { + pool, + // Floor the cadence so a misconfiguration can't busy-loop the + // maintenance pool. + interval_ms: interval_ms.max(100), + } + } + + /// Spawn the flush loop. Fire-and-forget: the loop logs and survives + /// every error (an exited loop would silently freeze all folder ETags), + /// and the first flush runs immediately to drain rows left over from a + /// previous run. + #[instrument(skip(self))] + pub fn start_flush_job(&self) { + let pool = self.pool.clone(); + let interval_ms = self.interval_ms; + info!( + "Starting tree-ETag flush job (every {}ms, batch {})", + interval_ms, DRAIN_BATCH + ); + tokio::spawn(async move { + let mut ticker = tokio::time::interval(std::time::Duration::from_millis(interval_ms)); + // If a flush overruns the interval, fire the next one a full + // interval later instead of bursting to catch up. + ticker.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Delay); + loop { + ticker.tick().await; + match Self::flush(&pool).await { + Ok((0, _)) => {} + Ok((drained, bumped)) => { + debug!( + "Tree-ETag flush: drained {} queue row(s), bumped {} folder(s)", + drained, bumped + ); + } + Err(e) => { + error!("Tree-ETag flush failed (queue preserved, will retry): {e}"); + } + } + } + }); + } + + /// Drain the dirty queue and bump the ancestor closure. Returns + /// `(queue rows drained, folder rows bumped)` summed over the batches + /// processed this tick. + async fn flush(pool: &PgPool) -> Result<(i64, i64), sqlx::Error> { + let mut total_drained = 0i64; + let mut total_bumped = 0i64; + + for _ in 0..MAX_BATCHES_PER_TICK { + let (drained, bumped) = retry_on_deadlock("tree_etag_flush", || { + sqlx::query_as::<_, (i64, i64)>( + r#" + WITH drained AS ( + DELETE FROM storage.tree_etag_dirty + WHERE id IN (SELECT id + FROM storage.tree_etag_dirty + ORDER BY id + LIMIT $1) + RETURNING lpath, folder_id + ), + targets AS ( + -- Captured chain: covers target folders deleted or + -- moved away since enqueue (the old location's + -- surviving ancestors still get their bump). + SELECT lpath FROM drained + UNION + -- Flush-time resolution: a folder MOVED since + -- enqueue had its subtree's lpaths rewritten, so + -- the captured lpath no longer matches it — its id + -- resolves to the CURRENT chain instead. Without + -- this, a bump queued just before a move would be + -- silently lost and sync clients would never + -- discover the change. + SELECT fo.lpath + FROM storage.folders fo + JOIN drained d ON fo.id = d.folder_id + ), + victims AS ( + -- `lpath @> target` = the target folder itself plus + -- every ancestor (GiST-indexed). Folder rows deleted + -- since enqueue simply don't match. Lock in id order + -- so overlapping closures cannot deadlock. + SELECT f.id + FROM storage.folders f + WHERE EXISTS (SELECT 1 FROM targets t + WHERE f.lpath @> t.lpath) + ORDER BY f.id + FOR NO KEY UPDATE + ), + bumped AS ( + UPDATE storage.folders f + SET tree_modified_at = + GREATEST(NOW(), f.tree_modified_at + interval '1 second') + FROM victims v + WHERE f.id = v.id + RETURNING f.id + ) + SELECT (SELECT COUNT(*) FROM drained)::bigint, + (SELECT COUNT(*) FROM bumped)::bigint + "#, + ) + .bind(DRAIN_BATCH) + .fetch_one(pool) + }) + .await?; + + total_drained += drained; + total_bumped += bumped; + if drained < DRAIN_BATCH { + break; + } + } + + Ok((total_drained, total_bumped)) + } +} diff --git a/src/interfaces/nextcloud/webdav_handler.rs b/src/interfaces/nextcloud/webdav_handler.rs index 47e4f4d7..8999fe04 100644 --- a/src/interfaces/nextcloud/webdav_handler.rs +++ b/src/interfaces/nextcloud/webdav_handler.rs @@ -1211,9 +1211,9 @@ pub fn write_folder_response( .unwrap_or_else(Utc::now); write_text_element(xml, "d:getlastmodified", &modified_at.to_rfc2822())?; - // Route through `FolderDto::etag` (= `Folder::etag()`, currently - // the folder UUID — see the entity for the documented v1 formula - // and the follow-up plan to make it descendant-aware). + // Route through `FolderDto::etag` (= `Folder::etag()`: the + // descendant-aware `{id[..16]}-{tree_modified_at}` — see the + // entity for the formula and the async-bump freshness contract). write_text_element(xml, "d:getetag", &format!("\"{}\"", folder.etag))?; write_text_element(xml, "d:getcontenttype", "httpd/unix-directory")?; write_text_element(xml, "d:getcontentlength", "0")?;