perf(file_storage): off-loop S3 spool I/O, one client per multipart upload, browse index

CHANGELOG.md

@@ -83,6 +83,7 @@ All notable changes to this project are documented in this file. The format is b
 
 ### Fixed
 
+- Offload S3 server-upload spool I/O, wait for in-flight I/O during cancellation, and reuse one client per multipart upload. Index the default tenant file browse order (migration `f6c2d4e8b109`; local performance review, 2026-11-05).
 - Users password creation, forgot-password fingerprints, reset verification and
   password updates offload hashing to worker threads, keeping unrelated async
   requests responsive during registration and bulk invites. Password policy,

docs/database/migrations.md

@@ -71,7 +71,7 @@ The history is **not** a single linear chain. It has several roots and several h
 
 | Head | Owning module | Label Alembic reports |
 |---|---|---|
-| `c5d8e2f1a903` | background_tasks (newest-first listing index) | `api_tokens`, `background_tasks`, `file_storage`, `users` |
+| `f6c2d4e8b109` | file_storage (tenant browse index) | `api_tokens`, `background_tasks`, `file_storage`, `users` |
 | `b8d4f2a6c901` | audit_log | `audit_log` |
 | `f1a9c3d7b2e4` | tenants | `tenants` |
 | `d263ea727663` | framework (`sm_lease`) | `leases` |

docs/modules/file_storage.md

@@ -332,7 +332,11 @@ worker loop itself.
 S3 server-side `put` switches to multipart above
 `direct_upload_threshold_bytes`, even for streams whose size is unknown. It
 spools only the threshold prefix and one part at a time (each spills to disk
-above 10 MiB). Part size comes from `multipart_part_size_bytes`, grown for known
+above 10 MiB). Spool I/O runs in worker threads; cancellation waits for
+in-flight I/O before closing its file. Each server-side multipart upload reuses
+one S3 client through completion or abort, allowing connection-pool reuse
+without sharing a client across requests or event loops. Part size comes from
+`multipart_part_size_bytes`, grown for known
 sizes to fit 10,000 parts; unknown-size streams abort if they exceed that bound.
 Declared nonzero sizes must match the bytes read. Multipart is also used above
 S3's single-PUT limit, even if the threshold was configured higher. Stream,
@@ -343,6 +347,11 @@ without masking the failure. Worker uploads still honor `max_file_size_bytes`
 32-bit `INTEGER` on PostgreSQL, which refused files of 2 GiB or more), and the
 S3 backend's `put` goes multipart, so objects beyond 5 GiB work end to end.
 
+Migration `f6c2d4e8b109` adds an index on `(tenant_id, is_deleted,
+created_at DESC, id)` for the default live-file browse order. It extends the
+shared `file_storage@head` trunk; apply with `upgrade heads`. Other sort orders
+and filtered searches may still require sorting.
+
 A process killed mid-upload cannot run compensation. Its incomplete multipart
 upload is an orphan like an abandoned browser one, and the upload reaper aborts
 it once it is older than twice `upload_session_ttl_seconds` (48 hours by

docs/perf/2026-11-05-file-storage.md

@@ -0,0 +1,53 @@
+# File-storage performance fixes, 2026-11-05
+
+Local review against `4920a13a`: synchronous server-upload spool I/O, per-part
+S3 clients, and a missing ordered tenant browse index.
+
+## Local measurements
+
+No S3, HTTP server, browser, or PostgreSQL service was used. These measurements
+are evidence for the local changes, not production latency predictions.
+
+### Upload loop lag
+
+Two concurrent uploads (32 MiB and 64 MiB), 1 MiB source chunks, real temporary
+files with the existing 10 MiB spool limit, and the existing `S3Wire` fake.
+The wire consumed file bodies in executor threads, like aiohttp's file payload,
+without simulating network latency. A 1 ms asyncio timer sampled scheduling
+lag. Known sizes used the actual length; unknown sizes used zero. Each cell is
+the median of three runs. Three implementations were compared in one process:
+`4920a13a` (origin/main), the first offloading change where every spool call
+went through the executor ("all-offload"), and the final change, where spool
+operations that stay in memory (up to 2 MiB) run inline and only operations that
+touch disk are offloaded. The final change also replays the prefix in 1 MiB
+blocks instead of 64 KiB and tracks the spool position itself instead of
+calling `tell()` through the executor.
+
+| Threshold | Size known | Loop lag p99, main / all-offload / final | Elapsed, main / all-offload / final | Client opens main → final |
+| --- | --- | --- | --- | --- |
+| 16 MiB (force multipart) | yes | 28.9 / 7.2 / 13.4 ms | 0.234 / 0.306 / 0.297 s | 10 → 2 |
+| 16 MiB (force multipart) | no | 54.8 / 1.5 / 11.4 ms | 0.251 / 0.794 / 0.340 s | 10 → 2 |
+| 64 MiB (default; single PUT) | yes | 37.1 / 10.4 / 13.8 ms | 0.261 / 0.314 / 0.301 s | 2 → 2 |
+| 64 MiB (default; single PUT) | no | 36.8 / 11.5 / 12.8 ms | 0.257 / 0.340 / 0.290 s | 2 → 2 |
+
+The final change keeps most of the loop-lag gain (p99 2-4x lower than main) and
+removes most of the throughput regression: elapsed is within 10-35% of main
+(unknown-size multipart was 3.2x slower with full offloading, now 1.35x),
+at the cost of a somewhat higher p99 than full offloading (inline memory copies
+and the remaining executor hops). This fake-only run cannot establish
+end-to-end throughput or connection reuse at the network level.
+
+`/health` p99 during real S3 uploads was not measured. SDK-internal signing/checksum work was not profiled by this fake.
+
+### Browse query
+
+A one-off script loaded 100,000 rows (the committed test uses 1,000) for one live tenant into
+SQLite and checks the first 25 IDs, before and after migration `f6c2d4e8b109`.
+Ten executions averaged **38.455 → 0.076 ms**. Before the change SQLite used a
+temporary B-tree for ordering; afterward the ordered tenant index served the
+page without a sort. Both model metadata and the upgrade/downgrade are tested.
+
+The test uses SQLite as a local proxy. A PostgreSQL `EXPLAIN (ANALYZE, BUFFERS)`
+on 100k+ representative tenant files remains necessary to establish that
+platform's plan and timings. The index targets the default live-file order;
+counts, alternate sort orders and arbitrary filters may still scan or sort.

host/migrations/versions/f6c2d4e8b109_file_storage_browse_index.py

@@ -0,0 +1,24 @@
+"""Index tenant file browsing in page order.
+
+Revision ID: f6c2d4e8b109
+Revises: c5d8e2f1a903 (current file_storage@head on the shared trunk)
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+revision = "f6c2d4e8b109"
+down_revision = "c5d8e2f1a903"
+branch_labels = None
+depends_on = None
+
+_TABLE = "file_storage_stored_file"
+_INDEX = "ix_file_storage_stored_file_tenant_browse"
+
+
+def upgrade() -> None:
+    op.create_index(_INDEX, _TABLE, ["tenant_id", "is_deleted", sa.text("created_at DESC"), "id"])
+
+
+def downgrade() -> None:
+    op.drop_index(_INDEX, table_name=_TABLE)

host/tests/test_migration_graph.py

@@ -46,6 +46,7 @@
     "c5d8e2f1a903": "background_tasks",  # newest-first listing index
     "e731a1b2c3d4": "users",  # unique lower(email) (#731)
     "e805a1b2c3d4": "users",  # browser session retirement
+    "f6c2d4e8b109": "file_storage",  # tenant browse index
     "51b2058d8afd": "api_tokens",
     "168a2882f443": "keycloak",
     "f6a3b9d2e8c4": "keycloak",
@@ -72,7 +73,7 @@
 # head revision -> module owning the tables it creates or alters.
 _HEADS = {
     "b8d4f2a6c901": "audit_log",
-    "c5d8e2f1a903": "background_tasks",  # also carries ``api_tokens``, ``file_storage``, ``users``
+    "f6c2d4e8b109": "file_storage",  # also carries ``api_tokens``, ``background_tasks``, ``users``
     "f1a9c3d7b2e4": "tenants",
     "f6a3b9d2e8c4": "keycloak",
     "c9d2e4f6a810": "oidc",
@@ -122,6 +123,7 @@
     "e731a1b2c3d4": "api_tokens",
     "e805a1b2c3d4": "api_tokens",
     "c5d8e2f1a903": "api_tokens",
+    "f6c2d4e8b109": "api_tokens",
     "a626b1c8d023": "api_tokens",  # also inherits ``background_tasks``
     "51b2058d8afd": "api_tokens",  # ancestors of a labelled revision inherit its label too
 }

modules/file_storage/file_storage/backends/s3.py

@@ -30,9 +30,9 @@
 class S3Backend(S3MultipartMixin):
     """S3-compatible object storage.
 
-    Each operation opens a fresh ``aioboto3`` client context (cheap; the
-    underlying ``aiobotocore`` session is reused via ``self._session``). We
-    deliberately do not hold one long-lived client across requests because
+    Each operation opens an ``aioboto3`` client context; a server-side
+    multipart upload shares that client through completion or abort. We
+    do not hold one long-lived client across requests because
     aioboto3 client contexts are per-event-loop and tests run multiple loops.
     """
 

modules/file_storage/file_storage/backends/s3_multipart.py

@@ -44,9 +44,17 @@ def _presign_client(self):
     async def create_multipart(self, key: str, content_type: str) -> str:
         try:
             async with self._client() as client:
-                resp = await client.create_multipart_upload(
-                    Bucket=self.bucket, Key=key, ContentType=content_type
-                )
+                return await self._create_multipart_on(client, key, content_type)
+        except StorageBackendError:
+            raise
+        except Exception as exc:
+            raise StorageBackendError(f"S3 create_multipart_upload failed: {exc}") from exc
+
+    async def _create_multipart_on(self, client, key: str, content_type: str) -> str:
+        try:
+            resp = await client.create_multipart_upload(
+                Bucket=self.bucket, Key=key, ContentType=content_type
+            )
             return resp["UploadId"]
         except Exception as exc:
             raise StorageBackendError(f"S3 create_multipart_upload failed: {exc}") from exc
@@ -108,24 +116,42 @@ async def list_parts(self, key: str, upload_id: str) -> list[Part]:
     async def complete_multipart(self, key: str, upload_id: str, parts: list[Part]) -> None:
         try:
             async with self._client() as client:
-                await client.complete_multipart_upload(
-                    Bucket=self.bucket,
-                    Key=key,
-                    UploadId=upload_id,
-                    MultipartUpload={
-                        "Parts": [
-                            {"PartNumber": p.no, "ETag": p.etag}
-                            for p in sorted(parts, key=lambda p: p.no)
-                        ]
-                    },
-                )
+                await self._complete_multipart_on(client, key, upload_id, parts)
+        except StorageBackendError:
+            raise
+        except Exception as exc:
+            raise StorageBackendError(f"S3 complete_multipart_upload failed: {exc}") from exc
+
+    async def _complete_multipart_on(
+        self, client, key: str, upload_id: str, parts: list[Part]
+    ) -> None:
+        try:
+            await client.complete_multipart_upload(
+                Bucket=self.bucket,
+                Key=key,
+                UploadId=upload_id,
+                MultipartUpload={
+                    "Parts": [
+                        {"PartNumber": p.no, "ETag": p.etag}
+                        for p in sorted(parts, key=lambda p: p.no)
+                    ]
+                },
+            )
         except Exception as exc:
             raise StorageBackendError(f"S3 complete_multipart_upload failed: {exc}") from exc
 
     async def abort_multipart(self, key: str, upload_id: str) -> None:
         try:
             async with self._client() as client:
-                await client.abort_multipart_upload(Bucket=self.bucket, Key=key, UploadId=upload_id)
+                await self._abort_multipart_on(client, key, upload_id)
+        except StorageBackendError:
+            raise
+        except Exception as exc:
+            raise StorageBackendError(f"S3 abort_multipart_upload failed: {exc}") from exc
+
+    async def _abort_multipart_on(self, client, key: str, upload_id: str) -> None:
+        try:
+            await client.abort_multipart_upload(Bucket=self.bucket, Key=key, UploadId=upload_id)
         except Exception as exc:
             if _is_no_such_upload(exc):
                 return