diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f62517c..0d845e39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## [Unreleased] +### Changes + +- SQL local durability and reservation before commit (`docs/sql-local-durability.md`). Postgres stays the master copy. File-backed SQL databases run `PRAGMA synchronous = FULL` (`[global.storage.sql] synchronous = "full"|"normal"`, env `TINYCLOUD_STORAGE__SQL__SYNCHRONOUS`; `normal` selects the earlier fsync behaviour), and the directories and renames that make a database file or a hydrated cache visible are fsynced. A write is checked before COMMIT and rolled back whole if it would pass the owner's remaining storage budget (`402`), and, when configured, the per-database cap (`max_database_bytes`, `507 sql_database_size_limit`) or the disk headroom (`min_free_bytes`, bounded by the WAL and checkpoint growth the transaction can cause and shared by all databases on the node, `507 sql_disk_headroom`). `max_database_bytes` and `min_free_bytes` default to `0` (off) and are set by config. A COMMIT that fails on disk answers `500 sql_commit_failed` and the node re-hydrates the database from Postgres. A write that fails because the disk is full answers `507 sql_disk_headroom`, whatever `min_free_bytes` is, not the owner's quota or the size limit. Every write request runs in one transaction (a single statement, a batch, a nested write (`WITH … INSERT`) and a request that carries a `PRAGMA` included), so such a request counts two more native SQL calls in request telemetry, and all of them get the same owner budget, size cap and disk gate. A request that wrote nothing is not gated and reserves nothing. The growth of the main file that the next checkpoint will cause stays held in the disk reservation from the commit until a checkpoint runs or the database's connection closes. No migration. Details: TC-1491. +- `MemorySink` caps its size hint at 64 MiB and treats a failed reservation as no hint. Details: TC-1491. +- A SQL request that writes is one transaction: an `execute` (with its `schema` statements), a `batch`, a named statement, and a `query` that writes (`WITH … INSERT`) or carries a `PRAGMA` (admin only). If any statement fails, or a deferred constraint fails at the commit, none of them is kept: the local file is unchanged and matches the durable copy in Postgres, and the request costs no Postgres round trip. This holds for a single statement too, including `INSERT OR FAIL` and a column constraint with `ON CONFLICT FAIL`. Telemetry: such a request counts two more native SQL calls (the `SAVEPOINT` and its `RELEASE` or `ROLLBACK`). No migration. Details: TC-1491. + ## [1.21.6] - 2026-10-21 ### Release notes diff --git a/CLAUDE.md b/CLAUDE.md index 7c4f51fe..fb540588 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -175,6 +175,9 @@ All use the `TINYCLOUD_` prefix: | `TINYCLOUD_STORAGE_LIMIT` | Storage quota | `10 MiB` | | `TINYCLOUD_KEYS_SECRET` | Key derivation secret | Base64URL string | | `TINYCLOUD_SPACES_ALLOWLIST` | Allowlist endpoint | `http://localhost:10000` | +| `TINYCLOUD_STORAGE__SQL__SYNCHRONOUS` | SQL `PRAGMA synchronous` for file-backed databases: `full` (default) or `normal` (rollback flag) | `full` | +| `TINYCLOUD_STORAGE__SQL__MAX_DATABASE_BYTES` | Per-database SQL size cap; `0` (default) = off. Sam's decision (card `sql-storage-report-read`) | `1 GiB` | +| `TINYCLOUD_STORAGE__SQL__MIN_FREE_BYTES` | Disk kept free after in-flight SQL writes; `0` (default) = check off. Sam's decision (card `sql-storage-report-read`) | `512 MiB` | | `ROCKET_MAX_BLOCKING` | Rocket's blocking-thread-pool cap (default 512). `ROCKET_`-prefixed, not `TINYCLOUD_`, because Rocket builds its tokio runtime from its own Figment before TinyCloud's config is loaded — `TINYCLOUD_MAX_BLOCKING` is silently ignored for this setting. | `1024` | ### Database Support diff --git a/Cargo.lock b/Cargo.lock index cc5c798e..0018c8ec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9898,6 +9898,7 @@ dependencies = [ "hmac 0.12.1", "iri-string", "k256", + "libc", "libp2p", "multihash-derive 0.9.1", "pin-project", diff --git a/docs/incremental-database-artifacts.md b/docs/incremental-database-artifacts.md index 4224145d..be5e74dc 100644 --- a/docs/incremental-database-artifacts.md +++ b/docs/incremental-database-artifacts.md @@ -61,6 +61,10 @@ Explicit DuckDB exports checkpoint the engine and durably install that checkpoint before returning. SQLite exports use a non-destructive backup, so they do not disturb the WAL baseline. +## Bounded reads (TC-822) + +The node never reads a whole artifact row. Existence and size checks use `load_meta` (metadata columns only); hydration streams `payload` then `delta_payload` in 4 MiB slices into a cache-file sink, verifying the content hashes as it goes and restarting if the row changes mid-read. See `docs/runbooks/tc-822-artifact-payload-storage.md` for the Postgres `STORAGE EXTERNAL` operator step. + ## Observability Every persistence operation emits structured fields: diff --git a/docs/request-telemetry.md b/docs/request-telemetry.md index 6495e540..93dd24d0 100644 --- a/docs/request-telemetry.md +++ b/docs/request-telemetry.md @@ -90,7 +90,7 @@ SQLx query timing follows the execution/logger lifetime, including network I/O a | Quota/admin | In-memory overrides and size mirrors do not invent queries; usage/hosted-space enumeration and any actual pool work are counted | | SQL/DuckDB identity/fence/alias resolution and durable artifact load/save/CAS/delta | Shared pool, including PostgreSQL bulk scopes; cache hits perform only their actual work | | Request-awaited read audit | Separate measured batch context, fanned out before durable receipts; details below | -| User SQL rusqlite actor | Context on every work message; actual statement/schema/batch/growth-control SQL, prepare/constraint errors, cold-open PRAGMAs, size/promotion/checkpoint SQL | +| User SQL rusqlite actor | Context on every work message; actual statement/schema/batch/growth-control SQL (a write request also counts the `SAVEPOINT` and `RELEASE`/`ROLLBACK` of its request transaction, TC-1491; the reservation's own PRAGMAs and checks are not counted), prepare/constraint errors, cold-open PRAGMAs, size/promotion/checkpoint SQL | | SQL publication | The current withdrawn publication handler executes no SQL; it is not an uninstrumented database path | | Optional DuckDB actor | Context on every message and the initiating cold-open worker; JSON/Arrow queries, schema/batch/transaction/growth/catalog SQL, locked security settings and promotion/copy/describe SQL | | DuckDB import/cold export blocking workers | Explicit worker context for guarded admission/validation/checkpoint/copy SQL; artifact repository work is measured separately by SQLx | diff --git a/docs/runbooks/tc-822-artifact-payload-storage.md b/docs/runbooks/tc-822-artifact-payload-storage.md new file mode 100644 index 00000000..262d586b --- /dev/null +++ b/docs/runbooks/tc-822-artifact-payload-storage.md @@ -0,0 +1,67 @@ +# TC-822: bounded artifact reads and the Postgres `payload` storage setting + +Tracking: [TC-822](https://linear.app/tinycloud-labs/issue/TC-822), under TC-1491. This is an **operator step on Postgres**, not a migration: it is not run at boot and rewrites no data. The PR that introduces the bounded reads contains no migration or schema change (schema freeze). + +## What the node does now + +The node never reads a whole `database_artifact` row. `load_meta` selects the metadata columns only. Hydration (`read_chunks`) reads `payload`, then `delta_payload`, in 4 MiB `substr(column, offset, length)` slices, each its own statement under the pool's base statement timeout and guarded by `WHERE revision = AND checkpoint_content_hash = `. The slices are hashed as they stream and compared with `checkpoint_content_hash` / `delta_content_hash` / `content_hash`. A slice that finds no row (the artifact was saved or deleted meanwhile) restarts the hydration from fresh metadata, at most twice; any hash or size mismatch fails the hydration and seeds nothing. + +Round trips per artifact hydration: an artifact of at most 8 MiB stored bytes (`BASE_TIMEOUT_PAYLOAD_BYTES`) is `1 + 1` (+1 with a WAL delta); a larger one is `1 + ceil(checkpoint_bytes / 4 MiB) + ceil(delta_bytes / 4 MiB)`. + +## Why Postgres needs `STORAGE EXTERNAL` on `payload` + +`bytea` columns default to `STORAGE EXTENDED`: large values are pglz-compressed in TOAST. `substr` on a compressed value decompresses the whole **prefix** up to `offset + length` inside the server, so slice cost and server memory grow with the offset. Measured on PostgreSQL 14 (`shared_buffers=64MB`, SQLite-like data compressing 3.4x): + +| artifact | slice offset | EXTENDED (compressed) | EXTERNAL | +|---|---|---|---| +| 256 MiB | 250 MB | 360 ms, 325 MB backend RSS | 1 ms, ~4 MB | +| 1000 MiB | 1,044 MB | 1.4 s, 1.3 GB backend RSS | 1 ms, ~7 MB | +| 1000 MiB, all 250 slices | | 214 s | 0.67 s | + +Without the setting, the node stays correct and its own memory stays bounded, but a large, compressed artifact hydrates slowly and costs the database server as much memory as one slice reaches into the value, and with a tight base statement timeout a deep slice can be cancelled (a 500 ms timeout is exceeded past ~100 MB offsets). Apply it before relying on large artifacts. + +## The step + +```sql +SET lock_timeout = '3s'; -- ACCESS EXCLUSIVE, metadata only; retry if it times out +ALTER TABLE database_artifact ALTER COLUMN payload SET STORAGE EXTERNAL; +``` + +- Idempotent: running it again is a no-op. Check with `SELECT attstorage FROM pg_attribute WHERE attrelid = 'database_artifact'::regclass AND attname = 'payload'` (`e` = external, `x` = extended). +- Not a rewrite. **Existing rows keep their compressed form** until their next full checkpoint save, which writes `payload` anew (WAL-only saves touch `delta_payload`, bounded to 8 MiB, and do not need the setting). Measured: `ALTER` alone leaves an existing compressed row as slow as before. +- A schema created fresh by the migrations stays `EXTENDED`; run the statement once per database. +- Do not run it inside a migration or the boot transaction. Do not mass-rewrite rows without a separate decision (an `UPDATE ... SET payload = payload || ''::bytea` rewrites every TOAST chunk and is WAL-heavy). + +## Production plan + +- Run the `ALTER` on the **new US-West** branch **before** the logical-replication initial copy: every row then arrives fresh and uncompressed, sliceable, and no rewrite is ever needed. +- On us-east the `ALTER` alone converts nothing; each artifact converts at its next full checkpoint. Until then a large, compressed artifact hydrates on the slow path (numbers above: ~360 ms and ~325 MB server memory for a slice at 250 MB of a 256 MiB artifact; 214 s to hydrate 1000 MiB). Node RAM stays bounded either way. +- The `ALTER` is DDL: run it once per side, outside the replication-copy-to-GO schema freeze window. This change's merge/deploy must not land inside that window either. This PR contains no migration or schema change. + +## Cost: `EXTERNAL` disables compression + +Compressible rows get larger on disk by `stored_bytes x (ratio - 1)` once they convert (PlanetScale bills above 10 GB; the database is about 6.4 GB in total). Measure the exact figure first with a metadata-only query (it reads the stored datum size and the raw size from the TOAST header; it does not detoast or decompress, verified on a 1000 MiB compressible row: 1 ms, 1 shared buffer hit, no TOAST pages read): + +```sql +SELECT count(*), pg_size_pretty(sum(pg_column_size(payload))) AS stored, + pg_size_pretty(sum(octet_length(payload))) AS raw, + round(sum(octet_length(payload))::numeric / NULLIF(sum(pg_column_size(payload)), 0), 2) AS ratio +FROM database_artifact; +``` + +The production figure is **unmeasured** until ops runs it (no production role here). Scenarios, growth = X x (r - 1) GB, X = stored `payload` GB, r = raw/stored ratio (3.45 is the synthetic pglz ratio of the test data; real SQLite ratios are likely lower), total 6.4 GB before: + +| stored X | r = 1.5 | r = 2.5 | r = 3.45 | +|---|---|---|---| +| 0.5 GB | +0.25 GB | +0.75 GB | +1.2 GB | +| 1 GB | +0.5 GB | +1.5 GB | +2.5 GB | +| 2 GB | +1.0 GB | +3.0 GB | +4.9 GB (11.3 GB total, over 10) | +| 4 GB | +2.0 GB | +6.0 GB (12.4 GB, over 10) | +9.8 GB (16.2 GB, over 10) | + +The 10 GB line is crossed when growth exceeds 3.6 GB: X = 4 GB at r = 2.5 or 3.45, and X = 2 GB at r = 3.45. All other cells stay under it. + +SQLite deployments need nothing, but note that SQLite's `substr` on a BLOB materialises the whole blob inside the SQLite library for every slice (about one blob of transient memory, ~0.3 s per slice per 256 MiB), so the "one slice" memory bound holds for the node's own heap and for Postgres, not for SQLite's internal buffer. + +## Adapting for at-rest encoding (TC-887) + +`ArtifactMeta` would gain the encoding columns; chunking applies to the **stored** bytes, so decoding belongs in the `ArtifactSink` (or a decoding wrapper) and the verified hashes are the stored-bytes hashes. `load_meta` / `read_chunks` documentation says the same. diff --git a/docs/sql-local-durability.md b/docs/sql-local-durability.md new file mode 100644 index 00000000..9720ac15 --- /dev/null +++ b/docs/sql-local-durability.md @@ -0,0 +1,87 @@ +# SQL local durability and reservation before commit (TC-1491, slice 1a) + +Postgres stays the master copy of every SQL database. This slice makes the node's +**local** commit durable and refuses, before COMMIT, a write that would not fit. +It adds no migration, no schema change and no new storage. + +## What a request does + +A write request runs in one transaction (an outermost `SAVEPOINT`, so every +write request, whether a single statement, a `Batch` or an `Execute` with `schema` statements, is all-or-nothing): + +1. **Limits.** `PRAGMA max_page_count = min(per-database cap, current pages + owner budget / page size)`. SQLite enforces it itself, mid-statement, so the bound is exact. Not applied to the `Forbid` path (budget 0), which keeps its own guard. +2. The statements run. +3. **Pre-commit gate** (`precommit_gate` in `sql/database.rs`). + - Owner budget and cap are re-checked against the page growth of the open transaction. + - Disk: reserve `unspilled WAL bound + page growth * page size` in the node-wide `DiskBudget`. The reservation succeeds only if `free bytes - other in-flight reservations - this reservation >= min_free_bytes`. +4. `COMMIT` (`RELEASE` of the outermost savepoint). With `synchronous = FULL` the WAL is fsynced before it returns. +5. The reservation shrinks to the checkpoint growth (the WAL bytes are on disk now). The in-flight part ends at COMMIT and is released by `SqlService` when the request's outcome is known; the checkpoint growth stays held until a checkpoint runs or the connection closes. + +Any error before COMMIT is a `ROLLBACK` of the whole transaction. Nothing is committed and then reported as failed. + +### Why the disk bound is sound for index, trigger and foreign-key amplification + +At COMMIT SQLite appends one WAL frame (`page size + 24` bytes) per dirty page that has not already spilled to the WAL. Dirty pages are a subset of the connection's page cache, so `SQLITE_DBSTATUS_CACHE_USED` (heap bytes of the cache) bounds them, whatever the SQL text or payload was: index entries, the pages of every b-tree touched, freelist, overflow and trigger-written pages are all just pages. A page dirtied twice in a transaction still costs one frame. WAL bytes that already spilled are on disk and already out of the free space the gate reads. The main file grows at the next checkpoint by the transaction's page growth, which is why that term is reserved and held until a checkpoint runs. + +`the_unspilled_wal_bound_covers_what_commit_appends` checks the bound against real commits (1 to 100 000 rows, six indexes, plus an update of half the rows). + +What is **not** exact: `free bytes` is a `statvfs` snapshot, so another process on the same filesystem can still take the space after the check (the `min_free_bytes` floor is the margin for that). The reservation covers this process's concurrent databases only. The progress handler that stops a statement early only watches free space, not the cache. + +If free space cannot be read the write is refused (`Internal`), never allowed. + +## fsync policy + +- `PRAGMA synchronous` is set on every file-backed connection: `FULL` by default, in `open_connection` and, before the copy, in `promote_to_file`. With WAL, `FULL` fsyncs the WAL on every COMMIT. `NORMAL` (the rollback flag) fsyncs only at checkpoints. +- SQLite itself fsyncs the directory when it first creates a `-wal` file (the unix VFS `UNIXFILE_DIRSYNC` flag on a new WAL), but not for the main database file. So the node fsyncs: the directories it creates for a database (`create_dir_all_durable`), the directory after `promote_to_file` creates the database file, and the directory after the renames in `CacheFileSink::commit` (the parts are already `sync_all`'d before the rename). +- Deleting stale cache files before a hydration is not directory-synced: Postgres is master and the cache is re-creatable. +- `FULL` is necessary, not sufficient. It does not prove the CVM volume honours flushes. +- Databases below `memory_threshold` (10 MiB) are in-memory SQLite: there is no local file to fsync, and their durability is the Postgres save, as before. Store-master (1b) needs them file-backed. + +## Outcomes in this phase + +| | meaning | +|---|---| +| **Rejected** | Rolled back before COMMIT: no effect on the local file, Postgres or any later snapshot. Reservation refusals (`sql_database_size_limit`, `sql_disk_headroom`, owner budget `402`), `SQLITE_FULL`, any statement error (the whole request), and a COMMIT that returns an error. | +| **Committed** | The local COMMIT returned and, for a file-backed database with `FULL`, its WAL fsync did; and the Postgres save succeeded. Only then is success returned. | +| **Unknown** | Unchanged TC-1274 `outcome_unknown`: the Postgres save was sent and its outcome is not known. | + +Unchanged on purpose: a failed Postgres save discards the local state. A local commit that was never released (the process died after COMMIT, before the response) is dropped when the service next re-hydrates from Postgres, which is correct while Postgres is master. + +**COMMIT error.** If COMMIT's fsync fails after the WAL frames were written, SQLite reports the error and the request is reported as rejected, but the commit can still appear when the file is next opened. That is harmless here: the node discards the local state (`CommitFailed`) and re-hydrates from Postgres, which never saw the write. Under store-master (1b and later) it is an outcome-unknown case: keep the local WAL and answer `outcome_unknown`. + +A kill -9 test cannot inject "after COMMIT, before its fsync completed"; `before_commit`, `after_commit` and `before_response` seams cover the cuts that exist. SIGKILL does not drop the page cache, so the tests prove the commit protocol, not power loss. + +## Settings + +`[global.storage.sql]`; env `TINYCLOUD_STORAGE__SQL__` (the legacy single-underscore form works for names without an underscore). + +| key | default | meaning | +|---|---|---| +| `synchronous` | `"full"` | `"full"` or `"normal"`; the rollback flag. This is the durability fix, not a limit. | +| `max_database_bytes` | `0` (no cap) | per-database size cap, enforced by SQLite's `max_page_count` | +| `min_free_bytes` | `0` (check off) | disk kept free after all in-flight reservations; `0` disables the gate, the reservation and the free-space reads | + +**The two limits are off by default.** They are Sam's decision (card `sql-storage-report-read`) and are set by config once he answers; an unmeasured cap could refuse real users' databases. The mechanism is fully tested with explicit values. What stays on by default besides `synchronous`: the transaction per write request and the owner-budget limit below. + +**Owner budget is not switchable and can refuse a write today's code allows.** `max_page_count` is set from the owner's remaining quota (`limit - used`, the same `storage_check` read as before). Today a write that overshoots a non-zero remainder is allowed once and only the next one is refused with 402; now the overshooting write itself is refused with 402 before COMMIT. The comparison is the database's page growth against the remainder, the unit the quota already charges (logical database size); growth that frees pages elsewhere in the file is not counted, so it is never stricter than the charge. + +## Errors + +| error | HTTP | body prefix | +|---|---|---| +| per-database size cap | 507 | `sql_database_size_limit:` | +| disk headroom | 507 | `sql_disk_headroom:` | +| owner budget (existing `StorageWouldGrow`) | 402 | the TC-619 quota body | +| local COMMIT failed | 500 | `sql_commit_failed:` | + +507, not 413: the SQL client maps 413 to `SQL_RESPONSE_TOO_LARGE`, the KV and quota clients read it as an exhausted storage plan, and TC-1012's 413 is the request body cap. + +## Scope notes + +**What counts as a write.** `Execute`, `Batch` and `ExecuteStatement` are always wrapped (a read inside a batch is wrapped too; only telemetry notices). A single `Query` is wrapped, gated and reserved when `parser::classify_sql` says it is not read-only. That classification covers writes nested in a query tree (TC-1585): a `WITH … INSERT` or `WITH … UPDATE` is a write, so it gets the transaction, the owner budget, the size cap and the pre-commit gate like any other write. The answer does not depend on the caller's abilities; a read-tier grant is only a shortcut (its writes are refused anyway). The route uses the same classifier (`sql_request_is_write`) to decide whether to read the owner's budget, and calls the `*_authorized` service entry points (`execute_with_budget_authorized`, `execute_without_growth_authorized`, `execute_authorized`), so the budget path carries the per-statement authority of TC-1581. `needs_transaction` is the one place this is decided for a request. + +**PRAGMA.** A request that carries a `PRAGMA` (admin only) is not wrapped and not gated. This also keeps the actor's connection reset after a request that held a non-report `PRAGMA` (TC-1586) outside any transaction: the reset runs after the request, after the transaction has ended. `PRAGMA synchronous` is not on the admin allow-list, so it cannot be read or set through a request. + +**Retire order.** When that reset fails, the actor is retired (deregistered before the error reply, TC-1594 tracks closing the connection first). The pre-commit gate and the reservation release are on the same path as before: the reservation is released by a drop guard in `SqlService::execute_locked` when the request returns, whatever the actor did, so the retire order does not change it. A request that triggers the reset held a `PRAGMA`, so it was never wrapped, gated or reserved in the first place. + +`Publication` commands are not gated either. DuckDB is untouched. diff --git a/test/m1-realdata-e2e/Cargo.lock b/test/m1-realdata-e2e/Cargo.lock index f914258f..0e9afc2a 100644 --- a/test/m1-realdata-e2e/Cargo.lock +++ b/test/m1-realdata-e2e/Cargo.lock @@ -7986,6 +7986,7 @@ dependencies = [ "hkdf", "hmac 0.12.1", "k256", + "libc", "libp2p", "multihash-derive 0.9.1", "pin-project", diff --git a/test/w5-policy-runtime-node-e2e/Cargo.lock b/test/w5-policy-runtime-node-e2e/Cargo.lock index 1f4098e9..fd3d8d7f 100644 --- a/test/w5-policy-runtime-node-e2e/Cargo.lock +++ b/test/w5-policy-runtime-node-e2e/Cargo.lock @@ -8434,6 +8434,7 @@ dependencies = [ "hkdf", "hmac 0.12.1", "k256", + "libc", "libp2p", "multihash-derive 0.9.1", "pin-project", diff --git a/tinycloud-core/Cargo.toml b/tinycloud-core/Cargo.toml index 04683d8b..0cda3535 100644 --- a/tinycloud-core/Cargo.toml +++ b/tinycloud-core/Cargo.toml @@ -48,6 +48,7 @@ serde_ipld_dagcbor = "0.3" ucan-capabilities-object.workspace = true multihash-derive = "0.9" rusqlite.workspace = true +libc = "0.2" sqlparser.workspace = true tracing.workspace = true tokio = { workspace = true, features = ["sync", "time"] } diff --git a/tinycloud-core/src/database_artifacts.rs b/tinycloud-core/src/database_artifacts.rs index 383914ac..f06936c5 100644 --- a/tinycloud-core/src/database_artifacts.rs +++ b/tinycloud-core/src/database_artifacts.rs @@ -1,11 +1,17 @@ use async_trait::async_trait; use sea_orm::{ - sea_query::Expr, ActiveModelTrait, ActiveValue::Set, ColumnTrait, DbErr, EntityTrait, - QueryFilter, QuerySelect, + sea_query::{Expr, Func}, + ActiveModelTrait, + ActiveValue::Set, + ColumnTrait, DbErr, EntityTrait, QueryFilter, QuerySelect, }; use time::{format_description::well_known::Rfc3339, OffsetDateTime}; -use crate::{db_pool::BulkScope, hash::hash, models::database_artifact}; +use crate::{ + db_pool::BulkScope, + hash::{hash, Blake3Hasher}, + models::database_artifact, +}; #[derive(Debug, Clone)] pub struct DatabaseArtifact { @@ -109,29 +115,518 @@ pub enum DatabaseArtifactError { IncrementalPersistenceUnsupported, #[error("database artifact was written from a superseded checkpoint")] StaleLineage, + #[error("database artifact changed while it was being read")] + ArtifactChanged, + #[error("database artifact {part} failed its content hash")] + HashMismatch { part: &'static str }, + #[error("database artifact {part} is {actual} bytes, expected {expected}")] + SizeMismatch { + part: &'static str, + expected: u64, + actual: u64, + }, + /// The sink handed to [`DatabaseArtifactRepository::read_chunks`] refused + /// the bytes; the message is the sink's own. + #[error("{0}")] + Sink(String), +} + +/// The stored metadata of one artifact row: everything but the payload bytes. +/// +/// Reading it never touches the `payload` / `delta_payload` columns, so its +/// cost is independent of the artifact's size. It is what an existence check +/// or a size comparison needs, and what [`DatabaseArtifactRepository::read_chunks`] +/// pins a streamed read to. +/// +/// Shape note for at-rest encoding (TC-887): sizes and hashes here describe the +/// STORED bytes, which today are the plain database file. An encoding column +/// (`payload_encoding`) would be added to this struct, and chunking would keep +/// applying to the stored (encoded) bytes, so decoding belongs in the sink. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ArtifactMeta { + pub revision: i64, + pub size_bytes: i64, + pub content_hash: String, + pub checkpoint_content_hash: String, + pub checkpoint_size_bytes: i64, + pub delta_content_hash: Option, + pub delta_size_bytes: i64, + pub updated_at: String, + pub storage_mode: String, + pub backend: String, +} + +impl ArtifactMeta { + /// Whether the row carries a WAL delta next to its checkpoint. + pub fn has_delta(&self) -> bool { + self.delta_content_hash.is_some() + } +} + +/// Which stored column a streamed byte range belongs to. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ArtifactPart { + Checkpoint, + Wal, +} + +impl ArtifactPart { + pub fn as_str(self) -> &'static str { + match self { + Self::Checkpoint => "checkpoint", + Self::Wal => "wal", + } + } +} + +/// Where [`DatabaseArtifactRepository::read_chunks`] delivers the artifact. +/// +/// Calls for one attempt arrive as `begin_part(Checkpoint)`, `write`*, +/// `finish_part`, then, when the row has a delta, the same for `Wal`. A part +/// with no bytes still gets `begin_part` and `finish_part`. Nothing the sink +/// received is final until the caller's own commit step: a sink must be able to +/// `reset` and take the whole sequence again (a changed row restarts the read), +/// and must publish nothing before its caller says the read succeeded. +#[async_trait] +pub trait ArtifactSink: Send { + /// Discard everything received so far; a new attempt is about to begin. + async fn reset(&mut self) -> Result<(), DatabaseArtifactError>; + async fn begin_part(&mut self, part: ArtifactPart) -> Result<(), DatabaseArtifactError>; + /// The stored size of the part just begun, for sinks that buffer it. + fn size_hint(&mut self, _part: ArtifactPart, _bytes: u64) {} + async fn write(&mut self, bytes: &[u8]) -> Result<(), DatabaseArtifactError>; + async fn finish_part(&mut self) -> Result<(), DatabaseArtifactError>; +} + +/// The most [`MemorySink`] allocates ahead of the bytes it is given. +const MAX_SIZE_HINT_BYTES: u64 = 64 * 1024 * 1024; + +/// An [`ArtifactSink`] that keeps the bytes in memory, for callers that return +/// the whole artifact anyway (a DuckDB export) and for tests. +#[derive(Debug, Default)] +pub struct MemorySink { + pub checkpoint: Vec, + pub wal: Option>, + current: Option, +} + +#[async_trait] +impl ArtifactSink for MemorySink { + async fn reset(&mut self) -> Result<(), DatabaseArtifactError> { + *self = Self::default(); + Ok(()) + } + + async fn begin_part(&mut self, part: ArtifactPart) -> Result<(), DatabaseArtifactError> { + self.current = Some(part); + if part == ArtifactPart::Wal { + self.wal = Some(Vec::new()); + } + Ok(()) + } + + fn size_hint(&mut self, part: ArtifactPart, bytes: u64) { + let target = match part { + ArtifactPart::Checkpoint => Some(&mut self.checkpoint), + ArtifactPart::Wal => self.wal.as_mut(), + }; + if let Some(target) = target { + // The hint comes from stored metadata. A corrupt value must not + // abort the node (`reserve_exact` panics or aborts on overflow or + // a failed allocation), so cap it and treat a refusal as no hint. + let hint = usize::try_from(bytes.min(MAX_SIZE_HINT_BYTES)).unwrap_or(0); + let _ = target.try_reserve_exact(hint); + } + } + + async fn write(&mut self, bytes: &[u8]) -> Result<(), DatabaseArtifactError> { + match self.current { + Some(ArtifactPart::Checkpoint) => self.checkpoint.extend_from_slice(bytes), + Some(ArtifactPart::Wal) => self + .wal + .get_or_insert_with(Vec::new) + .extend_from_slice(bytes), + None => return Err(DatabaseArtifactError::Sink("write outside a part".into())), + } + Ok(()) + } + + async fn finish_part(&mut self) -> Result<(), DatabaseArtifactError> { + self.current = None; + Ok(()) + } +} + +/// An [`ArtifactSink`] that stages the checkpoint and WAL as temp files next +/// to their final paths: nothing is visible at a final path until +/// [`CacheFileSink::commit`], which the caller runs only after +/// [`read_artifact`] succeeded. Dropping the sink (a failed, abandoned or +/// timed-out hydration) removes whatever was staged. +/// +/// The leading bytes of each part are checked with the given validators before +/// the part is accepted, so a payload the database engine would misread (and +/// silently revert to empty) is refused. +pub struct CacheFileSink { + service: &'static str, + label: String, + checkpoint_path: std::path::PathBuf, + wal_path: std::path::PathBuf, + valid_checkpoint: fn(&[u8]) -> bool, + valid_wal: fn(&[u8]) -> bool, + current: Option, + staged: Vec<(std::path::PathBuf, std::path::PathBuf)>, +} + +struct StagedPart { + part: ArtifactPart, + temp: std::path::PathBuf, + file: tokio::fs::File, + validated: bool, +} + +impl CacheFileSink { + pub fn new( + service: &'static str, + label: String, + checkpoint_path: std::path::PathBuf, + wal_path: std::path::PathBuf, + valid_checkpoint: fn(&[u8]) -> bool, + valid_wal: fn(&[u8]) -> bool, + ) -> Self { + Self { + service, + label, + checkpoint_path, + wal_path, + valid_checkpoint, + valid_wal, + current: None, + staged: Vec::new(), + } + } + + fn io(error: std::io::Error) -> DatabaseArtifactError { + DatabaseArtifactError::Sink(error.to_string()) + } + + fn validator(&self, part: ArtifactPart) -> fn(&[u8]) -> bool { + match part { + ArtifactPart::Checkpoint => self.valid_checkpoint, + ArtifactPart::Wal => self.valid_wal, + } + } + + fn validate(&self, part: ArtifactPart, head: &[u8]) -> Result<(), DatabaseArtifactError> { + if (self.validator(part))(head) { + return Ok(()); + } + tracing::error!( + service = self.service, + role = part.as_str(), + "Durable database artifact does not carry the expected file header; refusing to hydrate" + ); + Err(DatabaseArtifactError::Sink(format!( + "database artifact {} for {} does not carry the expected file header", + part.as_str(), + self.label + ))) + } + + fn discard(&mut self) { + if let Some(current) = self.current.take() { + let _ = std::fs::remove_file(¤t.temp); + release_staging(¤t.temp); + } + for (temp, _) in self.staged.drain(..) { + let _ = std::fs::remove_file(&temp); + release_staging(&temp); + } + } + + /// Move the staged files to their final paths. The caller has already + /// removed the stale cache files they replace. + pub async fn commit(&mut self) -> std::io::Result<()> { + // Renamed entries leave `staged`; on failure every one still in it + // (the failing temp included) is removed by `discard`. + let mut dirs = std::collections::BTreeSet::new(); + while let Some((temp, path)) = self.staged.first().cloned() { + if let Err(error) = tokio::fs::rename(&temp, &path).await { + self.discard(); + return Err(error); + } + release_staging(&temp); + self.staged.remove(0); + if let Some(parent) = path.parent() { + dirs.insert(parent.to_path_buf()); + } + } + // The parts were synced before the rename; the rename itself is only + // durable once its directory is (TC-1491). + for dir in dirs { + tokio::fs::File::open(&dir).await?.sync_all().await?; + } + Ok(()) + } +} + +impl Drop for CacheFileSink { + fn drop(&mut self) { + self.discard(); + } +} + +/// Staging files this process has created and not yet published or removed. +/// The sweep never touches these, so it cannot race a live write whose pid +/// matches a dead process's (pid reuse) or a write of another task. +fn live_staging() -> &'static std::sync::Mutex> { + static LIVE: std::sync::OnceLock< + std::sync::Mutex>, + > = std::sync::OnceLock::new(); + LIVE.get_or_init(Default::default) +} + +/// A temp sibling of `path`, unique per call, registered as live until +/// [`release_staging`]. +pub(crate) fn staging_path(path: &std::path::Path) -> std::path::PathBuf { + static NEXT_ID: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + // Appended to the whole path, never `with_extension`: `main.db` and + // `main.db-wal` must not share a temp name. + let temp = std::path::PathBuf::from(format!( + "{}.tmp.{}.{}", + path.display(), + std::process::id(), + NEXT_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed) + )); + live_staging() + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .insert(temp.clone()); + temp +} + +/// The staging file at `temp` was published or removed: the sweep may take it +/// from here on (a no-op if it is already gone). +pub(crate) fn release_staging(temp: &std::path::Path) { + live_staging() + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .remove(temp); +} + +#[cfg(feature = "duckdb")] +/// Releases a [`staging_path`] when dropped, whichever way its writer ends. +pub(crate) struct StagingRelease(pub(crate) std::path::PathBuf); + +#[cfg(feature = "duckdb")] +impl Drop for StagingRelease { + fn drop(&mut self) { + release_staging(&self.0); + } +} + +/// Remove the `.tmp..` files a crashed hydration left beside +/// each of `finals`: exactly this sink's temp-name pattern, and never a file +/// this process still has in flight. Run before a hydration begins, under the +/// database's hydration exclusion. +pub(crate) async fn sweep_stale_staging(finals: &[&std::path::Path]) { + for path in finals { + let (Some(dir), Some(name)) = (path.parent(), path.file_name()) else { + continue; + }; + let prefix = format!("{}.tmp.", name.to_string_lossy()); + let Ok(mut entries) = tokio::fs::read_dir(dir).await else { + continue; + }; + while let Ok(Some(entry)) = entries.next_entry().await { + let file_name = entry.file_name(); + let Some(rest) = file_name + .to_string_lossy() + .strip_prefix(&prefix) + .map(str::to_owned) + else { + continue; + }; + let is_staging = matches!( + rest.split_once('.'), + Some((pid, id)) + if !pid.is_empty() + && !id.is_empty() + && pid.bytes().all(|b| b.is_ascii_digit()) + && id.bytes().all(|b| b.is_ascii_digit()) + ); + let temp = entry.path(); + let live = live_staging() + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .contains(&temp); + if is_staging && !live && tokio::fs::remove_file(&temp).await.is_ok() { + tracing::info!("Removed a staging file left by an interrupted hydration"); + } + } + } +} + +#[async_trait] +impl ArtifactSink for CacheFileSink { + async fn reset(&mut self) -> Result<(), DatabaseArtifactError> { + self.discard(); + Ok(()) + } + + async fn begin_part(&mut self, part: ArtifactPart) -> Result<(), DatabaseArtifactError> { + let path = match part { + ArtifactPart::Checkpoint => self.checkpoint_path.clone(), + ArtifactPart::Wal => self.wal_path.clone(), + }; + if let Some(parent) = path.parent() { + tokio::fs::create_dir_all(parent).await.map_err(Self::io)?; + } + let temp = staging_path(&path); + let file = match tokio::fs::File::create(&temp).await { + Ok(file) => file, + Err(error) => { + release_staging(&temp); + return Err(Self::io(error)); + } + }; + self.current = Some(StagedPart { + part, + temp, + file, + validated: false, + }); + Ok(()) + } + + async fn write(&mut self, bytes: &[u8]) -> Result<(), DatabaseArtifactError> { + use tokio::io::AsyncWriteExt; + let Some(current) = self.current.as_ref() else { + return Err(DatabaseArtifactError::Sink("write outside a part".into())); + }; + // The first slice holds every header the validators look at. + if !current.validated { + self.validate(current.part, bytes)?; + } + let current = self.current.as_mut().expect("checked above"); + current.validated = true; + current.file.write_all(bytes).await.map_err(Self::io) + } + + async fn finish_part(&mut self) -> Result<(), DatabaseArtifactError> { + let Some(current) = self.current.as_ref() else { + return Err(DatabaseArtifactError::Sink("no part to finish".into())); + }; + // An empty part never saw a write; it is judged as the empty header. + if !current.validated { + self.validate(current.part, &[])?; + } + let StagedPart { + part, temp, file, .. + } = self.current.take().expect("checked above"); + if let Err(error) = file.sync_all().await { + let _ = tokio::fs::remove_file(&temp).await; + release_staging(&temp); + return Err(Self::io(error)); + } + drop(file); + let path = match part { + ArtifactPart::Checkpoint => self.checkpoint_path.clone(), + ArtifactPart::Wal => self.wal_path.clone(), + }; + self.staged.push((temp, path)); + Ok(()) + } +} + +/// Read an artifact into `sink` in bounded slices, restarting when the row +/// changes underneath the read. +/// +/// Returns the metadata the delivered bytes belong to, or `None` when there is +/// no such artifact (the sink is reset). A concurrent save between two +/// slices surfaces from [`DatabaseArtifactRepository::read_chunks`] as +/// [`DatabaseArtifactError::ArtifactChanged`]; the sink is `reset` and the read +/// restarts from fresh metadata, at most [`ARTIFACT_READ_RETRIES`] times, so a +/// sink never ends up holding a checkpoint of one revision and a WAL of +/// another. +pub async fn read_artifact( + repository: &R, + service: &str, + space: &str, + name: &str, + sink: &mut dyn ArtifactSink, +) -> Result, DatabaseArtifactError> { + let mut retries = 0; + loop { + let Some(meta) = repository.load_meta(service, space, name).await? else { + sink.reset().await?; + return Ok(None); + }; + sink.reset().await?; + match repository + .read_chunks(service, space, name, &meta, sink) + .await + { + Ok(()) => return Ok(Some(meta)), + Err(DatabaseArtifactError::ArtifactChanged) if retries < ARTIFACT_READ_RETRIES => { + retries += 1; + tracing::info!( + service, + retries, + "Database artifact changed mid-read; restarting" + ); + } + Err(error) => return Err(error), + } + } } #[async_trait] pub trait DatabaseArtifactRepository: Send + Sync { - async fn load( + /// The artifact's stored metadata, with no payload bytes read. + /// + /// One cheap statement whatever the artifact's size: use it for existence + /// checks and size comparisons, never `read_chunks`. + async fn load_meta( &self, service: &str, space: &str, name: &str, - ) -> Result, DatabaseArtifactError>; + ) -> Result, DatabaseArtifactError>; - /// [`Self::load`], told the artifact's stored size when the caller knows - /// it, so a repository can skip the bulk statement timeout's extra round - /// trips for a small artifact (TC-1054). `None` means unknown. - async fn load_sized( + /// Stream the stored checkpoint, then the WAL delta when `meta` has one, + /// into `sink` in slices of at most [`ARTIFACT_CHUNK_BYTES`]; an artifact + /// of at most [`BASE_TIMEOUT_PAYLOAD_BYTES`] stored bytes is one slice per + /// part. + /// + /// Peak memory is one slice on PostgreSQL. On SQLite it is one stored blob + /// transiently (the engine materialises the value before `substr` cuts it) + /// and CPU is O(n^2 / chunk) in the artifact size, so the bound there is + /// the part's size, not the slice. Every slice is + /// its own statement (`substr(column, offset, length)`, which runs on + /// both PostgreSQL `bytea` and SQLite `BLOB`) under the pool's base + /// timeout, guarded by `WHERE revision = meta.revision AND + /// checkpoint_content_hash = meta.checkpoint_content_hash`: a row that + /// moved on yields [`DatabaseArtifactError::ArtifactChanged`] instead of + /// bytes from two revisions. Round trips are `ceil(size / chunk)` per + /// non-empty part, on top of the one `load_meta` that produced `meta`. + /// + /// The checkpoint is hashed while it streams and compared to + /// `checkpoint_content_hash`, the WAL to `delta_content_hash`, and the row's + /// logical `content_hash` to what those imply; any mismatch is an error and + /// the caller must discard what the sink received. Prefer [`read_artifact`], + /// which also handles the restart. + /// + /// TC-887: chunking applies to the STORED bytes; an encoded payload would + /// be decoded by the sink (or a decoding wrapper around it), and the + /// hashes verified here would be the stored-bytes hashes. + async fn read_chunks( &self, service: &str, space: &str, name: &str, - _known_size: Option, - ) -> Result, DatabaseArtifactError> { - self.load(service, space, name).await - } + meta: &ArtifactMeta, + sink: &mut dyn ArtifactSink, + ) -> Result<(), DatabaseArtifactError>; async fn save( &self, @@ -159,11 +654,19 @@ pub trait DatabaseArtifactRepository: Send + Sync { /// [`RecoveringConnection::begin_bulk`] scope (TC-1054). It matches the SQL /// service's WAL delta bound (8 MiB): the server's work on an 8 MiB value /// (compression, TOAST, WAL) takes well under a second, far inside the 30 s -/// default. Loads use the same bound, judged by the stored size. +/// default. /// /// [`RecoveringConnection::begin_bulk`]: crate::db_pool::RecoveringConnection::begin_bulk pub const BASE_TIMEOUT_PAYLOAD_BYTES: usize = 8 * 1024 * 1024; +/// The slice size of a streamed artifact read. Two slices fit well inside +/// [`BASE_TIMEOUT_PAYLOAD_BYTES`]'s budget, so each runs under the base +/// statement timeout. +pub const ARTIFACT_CHUNK_BYTES: usize = 4 * 1024 * 1024; + +/// How many times [`read_artifact`] restarts a read whose row changed. +pub const ARTIFACT_READ_RETRIES: usize = 2; + /// The stored metadata a full checkpoint save is checked against. struct CurrentArtifact { revision: i64, @@ -223,64 +726,203 @@ impl SeaOrmDatabaseArtifactRepository { } } +struct ArtifactKey<'a> { + service: &'a str, + space: &'a str, + name: &'a str, +} + +impl SeaOrmDatabaseArtifactRepository { + /// Stream one payload column into `sink`, hash it, and return its CID. + async fn read_part( + &self, + key: &ArtifactKey<'_>, + meta: &ArtifactMeta, + part: ArtifactPart, + size_bytes: i64, + expected_hash: &str, + sink: &mut dyn ArtifactSink, + ) -> Result { + let column = match part { + ArtifactPart::Checkpoint => database_artifact::Column::Payload, + ArtifactPart::Wal => database_artifact::Column::DeltaPayload, + }; + let expected_len = u64::try_from(size_bytes).unwrap_or(0); + // An artifact that fits the base-timeout budget is one slice per part: + // no extra round trips for the common small database. + // Gated on the part's own size as well, so inconsistent metadata + // can never ask for a slice past the budget. + let budget = BASE_TIMEOUT_PAYLOAD_BYTES as i64; + let slice_bytes = if meta.size_bytes <= budget && size_bytes <= budget { + expected_len.max(1) + } else { + ARTIFACT_CHUNK_BYTES as u64 + }; + let mut hasher = Blake3Hasher::new(); + sink.begin_part(part).await?; + sink.size_hint(part, expected_len); + let mut offset = 0_u64; + while offset < expected_len { + let len = (expected_len - offset).min(slice_bytes); + // `substr` is 1-based and takes int4 on PostgreSQL. + let start = i32::try_from(offset + 1) + .map_err(|_| DatabaseArtifactError::PayloadTooLarge(expected_len))?; + let slice = database_artifact::Entity::find() + .select_only() + .expr( + Func::cust("substr") + .arg(Expr::col(column)) + .arg(start) + .arg(len as i32), + ) + .filter(database_artifact::Column::Service.eq(key.service)) + .filter(database_artifact::Column::Space.eq(key.space)) + .filter(database_artifact::Column::Name.eq(key.name)) + .filter(database_artifact::Column::Revision.eq(meta.revision)) + .filter( + database_artifact::Column::CheckpointContentHash + .eq(meta.checkpoint_content_hash.as_str()), + ) + .into_tuple::>>() + .one(&self.conn) + .await?; + // No row: the artifact was replaced or deleted since `meta`. + let Some(slice) = slice else { + return Err(DatabaseArtifactError::ArtifactChanged); + }; + let slice = slice.unwrap_or_default(); + if slice.len() as u64 != len { + return Err(DatabaseArtifactError::SizeMismatch { + part: part.as_str(), + expected: expected_len, + actual: offset + slice.len() as u64, + }); + } + hasher.update(&slice); + sink.write(&slice).await?; + offset += len; + } + let actual = hasher.finalize().to_cid(0x55).to_string(); + if actual != expected_hash { + return Err(DatabaseArtifactError::HashMismatch { + part: part.as_str(), + }); + } + sink.finish_part().await?; + Ok(actual) + } +} + #[async_trait] impl DatabaseArtifactRepository for SeaOrmDatabaseArtifactRepository { - async fn load( + async fn load_meta( &self, service: &str, space: &str, name: &str, - ) -> Result, DatabaseArtifactError> { - self.load_sized(service, space, name, None).await + ) -> Result, DatabaseArtifactError> { + use database_artifact::Column; + let row = database_artifact::Entity::find_by_id(( + service.to_string(), + space.to_string(), + name.to_string(), + )) + .select_only() + .column(Column::Revision) + .column(Column::SizeBytes) + .column(Column::ContentHash) + .column(Column::CheckpointContentHash) + .column(Column::CheckpointSizeBytes) + .column(Column::DeltaContentHash) + .column(Column::DeltaSizeBytes) + .column(Column::UpdatedAt) + .column(Column::StorageMode) + .column(Column::Backend) + .into_tuple::<( + i64, + i64, + String, + String, + i64, + Option, + i64, + String, + String, + String, + )>() + .one(&self.conn) + .await?; + Ok(row.map( + |( + revision, + size_bytes, + content_hash, + checkpoint_content_hash, + checkpoint_size_bytes, + delta_content_hash, + delta_size_bytes, + updated_at, + storage_mode, + backend, + )| ArtifactMeta { + revision, + size_bytes, + content_hash, + checkpoint_content_hash, + checkpoint_size_bytes, + delta_content_hash, + delta_size_bytes, + updated_at, + storage_mode, + backend, + }, + )) } - async fn load_sized( + async fn read_chunks( &self, service: &str, space: &str, name: &str, - known_size: Option, - ) -> Result, DatabaseArtifactError> { - // Hydration reads the whole checkpoint and WAL in one statement. One - // that may be larger than a delta runs under the bulk statement - // timeout (TC-1054); a known-small one under the base timeout, with - // no extra round trip. - let bulk = known_size.is_none_or(|size| size > BASE_TIMEOUT_PAYLOAD_BYTES as u64); - let find = || { - database_artifact::Entity::find_by_id(( - service.to_string(), - space.to_string(), - name.to_string(), - )) + meta: &ArtifactMeta, + sink: &mut dyn ArtifactSink, + ) -> Result<(), DatabaseArtifactError> { + let key = ArtifactKey { + service, + space, + name, }; - let row = if bulk { - self.conn - .bulk_read("load_artifact", |scope| async move { - let row = find().one(&scope).await?; - scope.commit().await?; - Ok(row) - }) - .await - } else { - // A read-only statement on the pool, retried like any other. - find().one(&self.conn).await + let checkpoint = self + .read_part( + &key, + meta, + ArtifactPart::Checkpoint, + meta.checkpoint_size_bytes, + &meta.checkpoint_content_hash, + sink, + ) + .await?; + let logical = match &meta.delta_content_hash { + Some(delta_hash) => { + self.read_part( + &key, + meta, + ArtifactPart::Wal, + meta.delta_size_bytes, + delta_hash, + sink, + ) + .await?; + hash(format!("{checkpoint}:{delta_hash}").as_bytes()) + .to_cid(0x55) + .to_string() + } + None => checkpoint, }; - row.map(|row| { - row.map(|model| DatabaseArtifact { - payload: model.payload, - content_hash: model.content_hash, - revision: model.revision, - size_bytes: model.size_bytes, - updated_at: model.updated_at, - backend: model.backend, - storage_mode: model.storage_mode, - checkpoint_content_hash: model.checkpoint_content_hash, - delta_payload: model.delta_payload, - delta_content_hash: model.delta_content_hash, - delta_size_bytes: model.delta_size_bytes, - }) - }) - .map_err(DatabaseArtifactError::Db) + if logical != meta.content_hash { + return Err(DatabaseArtifactError::HashMismatch { part: "content" }); + } + Ok(()) } async fn save( @@ -575,8 +1217,55 @@ impl DatabaseArtifactRepository for SeaOrmDatabaseArtifactRepository { } } +/// Test-only whole-artifact read built on the public streaming API, so tests +/// can assert on payload bytes. There is deliberately no such method on the +/// repository: the node never reads a whole row. +#[cfg(test)] +pub(crate) mod test_support { + use super::*; + + #[async_trait] + pub(crate) trait ReadAll { + async fn read_all( + &self, + service: &str, + space: &str, + name: &str, + ) -> Result, DatabaseArtifactError>; + } + + #[async_trait] + impl ReadAll for T { + async fn read_all( + &self, + service: &str, + space: &str, + name: &str, + ) -> Result, DatabaseArtifactError> { + let mut sink = MemorySink::default(); + let Some(meta) = read_artifact(self, service, space, name, &mut sink).await? else { + return Ok(None); + }; + Ok(Some(DatabaseArtifact { + payload: sink.checkpoint, + content_hash: meta.content_hash, + revision: meta.revision, + size_bytes: meta.size_bytes, + updated_at: meta.updated_at, + backend: meta.backend, + storage_mode: meta.storage_mode, + checkpoint_content_hash: meta.checkpoint_content_hash, + delta_payload: sink.wal, + delta_content_hash: meta.delta_content_hash, + delta_size_bytes: meta.delta_size_bytes, + })) + } + } +} + #[cfg(test)] mod tests { + use super::test_support::ReadAll; use super::*; use crate::migrations::Migrator; use sea_orm::{ConnectOptions, Database}; @@ -613,7 +1302,11 @@ mod tests { assert_eq!(delta.size_bytes, 112); assert_eq!(delta.delta_size_bytes, 12); - let loaded = repo.load("sql", "space", "main").await.unwrap().unwrap(); + let loaded = repo + .read_all("sql", "space", "main") + .await + .unwrap() + .unwrap(); assert_eq!(loaded.payload, vec![1; 100]); assert_eq!(loaded.delta_payload, Some(vec![2; 12])); assert_eq!(loaded.storage_mode, "checkpoint+wal"); @@ -627,7 +1320,11 @@ mod tests { ) .await .unwrap(); - let checkpoint = repo.load("sql", "space", "main").await.unwrap().unwrap(); + let checkpoint = repo + .read_all("sql", "space", "main") + .await + .unwrap() + .unwrap(); assert_eq!(checkpoint.size_bytes, 80); assert_eq!(checkpoint.delta_size_bytes, 0); assert_eq!(checkpoint.delta_payload, None); @@ -685,7 +1382,11 @@ mod tests { "expected a lineage rejection, got {err:?}" ); - let loaded = repo.load("sql", "space", "main").await.unwrap().unwrap(); + let loaded = repo + .read_all("sql", "space", "main") + .await + .unwrap() + .unwrap(); assert_eq!(loaded.revision, 2); assert_eq!(loaded.payload, vec![2; 200]); assert_eq!(loaded.delta_payload, None); @@ -741,7 +1442,11 @@ mod tests { "expected a lineage rejection, got {err:?}" ); - let loaded = repo.load("sql", "space", "main").await.unwrap().unwrap(); + let loaded = repo + .read_all("sql", "space", "main") + .await + .unwrap() + .unwrap(); assert_eq!(loaded.revision, 1); assert_eq!(loaded.payload, vec![1; 4096]); } @@ -815,11 +1520,767 @@ mod tests { // The stale writer must not have clobbered: durable state is the winner's // payload at revision 2, never the loser's bytes. - let loaded = repo.load("sql", "space", "main").await.unwrap().unwrap(); + let loaded = repo + .read_all("sql", "space", "main") + .await + .unwrap() + .unwrap(); assert_eq!(loaded.revision, 2); assert_eq!( loaded.payload, winner.payload, "stale writer must not overwrite the winner's checkpoint" ); } + + // ---- bounded streaming reads (TC-822) ---- + + use sea_orm::{ConnectionTrait, DatabaseConnection}; + use std::sync::{Arc, Mutex}; + + const CHUNK: usize = ARTIFACT_CHUNK_BYTES; + + /// The replacement (checkpoint, optional WAL) a disturbance saves. + type Disturb = fn(usize) -> (Vec, Option>); + + fn bytes(len: usize, seed: u8) -> Vec { + (0..len) + .map(|i| (i as u8).wrapping_mul(31).wrapping_add(seed)) + .collect() + } + + /// A migrated in-memory SQLite repository plus a log of every statement + /// it sends, the "counting connection" of the round-trip assertions. + async fn spied_repo() -> ( + SeaOrmDatabaseArtifactRepository, + DatabaseConnection, + Arc>>, + ) { + let mut options = ConnectOptions::new("sqlite::memory:".to_string()); + options.max_connections(1); + let mut conn = Database::connect(options).await.unwrap(); + Migrator::up(&conn, None).await.unwrap(); + let statements = Arc::new(Mutex::new(Vec::new())); + let log = statements.clone(); + conn.set_metric_callback(move |info| { + log.lock().unwrap().push(info.statement.sql.clone()); + }); + ( + SeaOrmDatabaseArtifactRepository::new(conn.clone()), + conn, + statements, + ) + } + + fn ceil_div(len: usize) -> usize { + len.div_ceil(CHUNK) + } + + #[tokio::test] + async fn chunk_boundaries_round_trip_within_the_round_trip_budget() { + let (repo, _conn, statements) = spied_repo().await; + let sizes = [0, 1, CHUNK - 1, CHUNK, CHUNK + 1]; + let deltas = [None, Some(1), Some(CHUNK), Some(CHUNK + 1)]; + for (index, len) in sizes.into_iter().enumerate() { + for (delta_index, delta) in deltas.into_iter().enumerate() { + let name = format!("db-{index}-{delta_index}"); + let payload = bytes(len, 1); + repo.save( + "sql", + "space", + &name, + payload.clone(), + ArtifactExpectation::Any, + ) + .await + .unwrap(); + let wal = delta.map(|len| bytes(len, 2)); + if let Some(wal) = &wal { + repo.save_delta("sql", "space", &name, wal.clone(), ArtifactExpectation::Any) + .await + .unwrap(); + } + + statements.lock().unwrap().clear(); + let mut sink = MemorySink::default(); + let meta = read_artifact(&repo, "sql", "space", &name, &mut sink) + .await + .unwrap() + .expect("the artifact exists"); + assert_eq!(sink.checkpoint, payload, "{name}: checkpoint"); + assert_eq!(sink.wal, wal, "{name}: wal"); + assert_eq!(meta.has_delta(), wal.is_some(), "{name}"); + + // Every statement is a retryable read: a lost connection costs + // one slice's retry, not the hydration. + for sql in statements.lock().unwrap().iter() { + assert!(crate::db_pool::is_read_only_statement(sql), "{sql}"); + } + // One metadata statement, then one per slice of each part; an + // artifact within the base-timeout budget is one slice a part. + let total = len + delta.unwrap_or(0); + let parts = |part: usize| { + if total <= BASE_TIMEOUT_PAYLOAD_BYTES { + usize::from(part > 0) + } else { + ceil_div(part) + } + }; + let expected = 1 + parts(len) + delta.map_or(0, parts); + assert_eq!( + statements.lock().unwrap().len(), + expected, + "{name}: round trips for {len} + {delta:?} bytes" + ); + } + } + } + + #[tokio::test] + async fn metadata_and_absence_never_select_payload_columns() { + let (repo, _conn, statements) = spied_repo().await; + repo.save( + "sql", + "space", + "main", + bytes(CHUNK + 5, 3), + ArtifactExpectation::Any, + ) + .await + .unwrap(); + repo.save_delta( + "sql", + "space", + "main", + bytes(9, 4), + ArtifactExpectation::Any, + ) + .await + .unwrap(); + + statements.lock().unwrap().clear(); + let meta = repo + .load_meta("sql", "space", "main") + .await + .unwrap() + .unwrap(); + assert_eq!(meta.revision, 2); + assert_eq!(meta.checkpoint_size_bytes, (CHUNK + 5) as i64); + assert_eq!(meta.delta_size_bytes, 9); + assert_eq!(meta.size_bytes, (CHUNK + 14) as i64); + assert_eq!(meta.storage_mode, "checkpoint+wal"); + assert!(repo + .load_meta("sql", "space", "absent") + .await + .unwrap() + .is_none()); + + let seen = statements.lock().unwrap().clone(); + assert_eq!(seen.len(), 2, "{seen:?}"); + assert!( + seen.iter().all(|sql| !sql.contains("payload")), + "a metadata read must not touch payload columns: {seen:?}" + ); + + // Reading an absent artifact through the streaming helper stops at the + // metadata too. + statements.lock().unwrap().clear(); + let mut sink = MemorySink::default(); + assert!(read_artifact(&repo, "sql", "space", "absent", &mut sink) + .await + .unwrap() + .is_none()); + assert_eq!(statements.lock().unwrap().len(), 1); + } + + /// Disturbs the row once per attempt, right after the first slice landed, + /// the way a concurrent save would between two slice statements. + struct RaceSink<'a> { + inner: MemorySink, + repo: &'a SeaOrmDatabaseArtifactRepository, + disturbances_left: usize, + disturbed_this_attempt: bool, + disturb: Disturb, + attempts: usize, + } + + #[async_trait] + impl ArtifactSink for RaceSink<'_> { + async fn reset(&mut self) -> Result<(), DatabaseArtifactError> { + self.attempts += 1; + self.disturbed_this_attempt = false; + self.inner.reset().await + } + async fn begin_part(&mut self, part: ArtifactPart) -> Result<(), DatabaseArtifactError> { + self.inner.begin_part(part).await + } + async fn write(&mut self, bytes: &[u8]) -> Result<(), DatabaseArtifactError> { + self.inner.write(bytes).await?; + if self.disturbances_left > 0 && !self.disturbed_this_attempt { + self.disturbed_this_attempt = true; + self.disturbances_left -= 1; + let (checkpoint, wal) = (self.disturb)(self.disturbances_left); + self.repo + .save("sql", "space", "main", checkpoint, ArtifactExpectation::Any) + .await + .unwrap(); + if let Some(wal) = wal { + self.repo + .save_delta("sql", "space", "main", wal, ArtifactExpectation::Any) + .await + .unwrap(); + } + } + Ok(()) + } + async fn finish_part(&mut self) -> Result<(), DatabaseArtifactError> { + self.inner.finish_part().await + } + } + + fn race_sink( + repo: &SeaOrmDatabaseArtifactRepository, + disturbances: usize, + disturb: Disturb, + ) -> RaceSink<'_> { + RaceSink { + inner: MemorySink::default(), + repo, + disturbances_left: disturbances, + disturbed_this_attempt: false, + disturb, + attempts: 0, + } + } + + #[tokio::test] + async fn a_row_replaced_mid_read_restarts_and_never_mixes_revisions() { + let (repo, _conn, _) = spied_repo().await; + repo.save( + "sql", + "space", + "main", + bytes(2 * CHUNK + 7, 1), + ArtifactExpectation::Any, + ) + .await + .unwrap(); + let replacement = bytes(CHUNK + 3, 9); + let wal = bytes(11, 8); + + let mut sink = race_sink(&repo, 1, |_| (bytes(CHUNK + 3, 9), Some(bytes(11, 8)))); + let meta = read_artifact(&repo, "sql", "space", "main", &mut sink) + .await + .unwrap() + .unwrap(); + assert_eq!(sink.attempts, 2, "one restart"); + assert_eq!(meta.revision, 3, "save, save, then the WAL delta"); + assert_eq!(sink.inner.checkpoint, replacement, "only the new revision"); + assert_eq!(sink.inner.wal, Some(wal)); + } + + #[tokio::test] + async fn a_wal_only_change_mid_read_also_restarts() { + let (repo, _conn, _) = spied_repo().await; + repo.save( + "sql", + "space", + "main", + bytes(2 * CHUNK + 1, 1), + ArtifactExpectation::Any, + ) + .await + .unwrap(); + repo.save_delta( + "sql", + "space", + "main", + bytes(5, 2), + ArtifactExpectation::Any, + ) + .await + .unwrap(); + + // The checkpoint hash is untouched by a delta, so the revision guard + // is what catches this. + struct DeltaRace<'a>(RaceSink<'a>, bool); + #[async_trait] + impl ArtifactSink for DeltaRace<'_> { + async fn reset(&mut self) -> Result<(), DatabaseArtifactError> { + self.0.reset().await + } + async fn begin_part( + &mut self, + part: ArtifactPart, + ) -> Result<(), DatabaseArtifactError> { + self.0.begin_part(part).await + } + async fn write(&mut self, bytes: &[u8]) -> Result<(), DatabaseArtifactError> { + self.0.inner.write(bytes).await?; + if !self.1 { + self.1 = true; + self.0 + .repo + .save_delta("sql", "space", "main", vec![7; 6], ArtifactExpectation::Any) + .await + .unwrap(); + } + Ok(()) + } + async fn finish_part(&mut self) -> Result<(), DatabaseArtifactError> { + self.0.finish_part().await + } + } + let mut sink = DeltaRace(race_sink(&repo, 0, |_| unreachable!()), false); + let meta = read_artifact(&repo, "sql", "space", "main", &mut sink) + .await + .unwrap() + .unwrap(); + assert_eq!(meta.revision, 3); + assert_eq!(sink.0.attempts, 2); + assert_eq!(sink.0.inner.wal, Some(vec![7; 6])); + assert_eq!(sink.0.inner.checkpoint, bytes(2 * CHUNK + 1, 1)); + } + + #[tokio::test] + async fn a_row_that_keeps_changing_gives_up_after_the_retries() { + let (repo, _conn, _) = spied_repo().await; + repo.save( + "sql", + "space", + "main", + bytes(2 * CHUNK + 1, 1), + ArtifactExpectation::Any, + ) + .await + .unwrap(); + let mut sink = race_sink(&repo, 10, |left| (bytes(2 * CHUNK + 1, left as u8), None)); + let error = read_artifact(&repo, "sql", "space", "main", &mut sink) + .await + .expect_err("every attempt is overtaken"); + assert!( + matches!(error, DatabaseArtifactError::ArtifactChanged), + "{error:?}" + ); + assert_eq!(sink.attempts, 1 + ARTIFACT_READ_RETRIES); + } + + fn dir_names(dir: &std::path::Path) -> Vec { + let mut names: Vec = std::fs::read_dir(dir) + .unwrap() + .map(|entry| entry.unwrap().file_name().to_string_lossy().into_owned()) + .collect(); + names.sort(); + names + } + + fn cache_sink(dir: &std::path::Path) -> CacheFileSink { + CacheFileSink::new( + "sql", + "space/main".to_string(), + dir.join("main.db"), + dir.join("main.db-wal"), + |_| true, + |_| true, + ) + } + + #[tokio::test] + async fn a_cache_file_is_never_seeded_from_a_mixed_read() { + let (repo, _conn, _) = spied_repo().await; + repo.save( + "sql", + "space", + "main", + bytes(2 * CHUNK + 7, 1), + ArtifactExpectation::Any, + ) + .await + .unwrap(); + let dir = tempfile::tempdir().unwrap(); + let mut files = cache_sink(dir.path()); + + // Overtaken after the first slice of the old checkpoint reached the + // temp file; the restart must rewrite it from the new revision. + struct Overtaken<'a>( + &'a mut CacheFileSink, + &'a SeaOrmDatabaseArtifactRepository, + bool, + ); + #[async_trait] + impl ArtifactSink for Overtaken<'_> { + async fn reset(&mut self) -> Result<(), DatabaseArtifactError> { + self.0.reset().await + } + async fn begin_part( + &mut self, + part: ArtifactPart, + ) -> Result<(), DatabaseArtifactError> { + self.0.begin_part(part).await + } + async fn write(&mut self, bytes: &[u8]) -> Result<(), DatabaseArtifactError> { + self.0.write(bytes).await?; + if !self.2 { + self.2 = true; + self.1 + .save( + "sql", + "space", + "main", + vec![5; CHUNK + 2], + ArtifactExpectation::Any, + ) + .await + .unwrap(); + } + Ok(()) + } + async fn finish_part(&mut self) -> Result<(), DatabaseArtifactError> { + self.0.finish_part().await + } + } + read_artifact( + &repo, + "sql", + "space", + "main", + &mut Overtaken(&mut files, &repo, false), + ) + .await + .unwrap() + .unwrap(); + assert!( + dir_names(dir.path()) + .iter() + .all(|name| name.contains(".tmp.")), + "nothing is published before commit: {:?}", + dir_names(dir.path()) + ); + files.commit().await.unwrap(); + assert_eq!(dir_names(dir.path()), ["main.db"]); + assert_eq!( + std::fs::read(dir.path().join("main.db")).unwrap(), + vec![5; CHUNK + 2] + ); + } + + #[tokio::test] + async fn corrupt_artifact_bytes_are_rejected_and_nothing_is_seeded() { + let (repo, conn, _) = spied_repo().await; + let len = CHUNK + 10; + repo.save( + "sql", + "space", + "main", + bytes(len, 1), + ArtifactExpectation::Any, + ) + .await + .unwrap(); + repo.save_delta( + "sql", + "space", + "main", + bytes(20, 2), + ArtifactExpectation::Any, + ) + .await + .unwrap(); + + for (what, sql, part) in [ + ( + "checkpoint flipped, same length", + format!("UPDATE database_artifact SET payload = zeroblob({len})"), + "checkpoint", + ), + ( + "wal flipped, same length", + "UPDATE database_artifact SET delta_payload = zeroblob(20)".to_string(), + "wal", + ), + ] { + // Restore a good row before each corruption. + repo.save( + "sql", + "space", + "main", + bytes(len, 1), + ArtifactExpectation::Any, + ) + .await + .unwrap(); + repo.save_delta( + "sql", + "space", + "main", + bytes(20, 2), + ArtifactExpectation::Any, + ) + .await + .unwrap(); + conn.execute_unprepared(&sql).await.unwrap(); + + let dir = tempfile::tempdir().unwrap(); + let mut sink = cache_sink(dir.path()); + let error = read_artifact(&repo, "sql", "space", "main", &mut sink) + .await + .expect_err(what); + assert!( + matches!(&error, DatabaseArtifactError::HashMismatch { part: got } if *got == part), + "{what}: {error:?}" + ); + drop(sink); + assert_eq!( + dir_names(dir.path()), + Vec::::new(), + "{what}: nothing left behind" + ); + } + + // A stored size that disagrees with the bytes is refused too. + repo.save( + "sql", + "space", + "main", + bytes(len, 1), + ArtifactExpectation::Any, + ) + .await + .unwrap(); + conn.execute_unprepared("UPDATE database_artifact SET payload = zeroblob(10)") + .await + .unwrap(); + let error = read_artifact(&repo, "sql", "space", "main", &mut MemorySink::default()) + .await + .expect_err("short payload"); + assert!( + matches!(error, DatabaseArtifactError::SizeMismatch { .. }), + "{error:?}" + ); + } + + #[tokio::test] + async fn the_cache_sink_checks_the_leading_bytes_of_each_part() { + let (repo, _conn, _) = spied_repo().await; + repo.save( + "sql", + "space", + "main", + b"MAGIC-body".to_vec(), + ArtifactExpectation::Any, + ) + .await + .unwrap(); + let dir = tempfile::tempdir().unwrap(); + let mut sink = CacheFileSink::new( + "sql", + "space/main".to_string(), + dir.path().join("main.db"), + dir.path().join("main.db-wal"), + |head| head.starts_with(b"OTHER"), + |_| true, + ); + let error = read_artifact(&repo, "sql", "space", "main", &mut sink) + .await + .expect_err("wrong header"); + assert!( + error.to_string().contains("expected file header"), + "{error}" + ); + drop(sink); + assert!(dir_names(dir.path()).is_empty()); + } + + #[tokio::test] + async fn postgres_slices_round_trip_at_chunk_boundaries_and_detect_changes() { + let Some(fixture) = crate::test_support::PostgresTestSchema::migrated_through( + "postgres_slices_round_trip_at_chunk_boundaries_and_detect_changes", + None, + ) + .await + else { + return; + }; + let repo = SeaOrmDatabaseArtifactRepository::new(fixture.conn.clone()); + for (index, len) in [0, 1, CHUNK - 1, CHUNK, CHUNK + 1].into_iter().enumerate() { + for (delta_index, delta) in [None, Some(1), Some(CHUNK + 1)].into_iter().enumerate() { + let name = format!("db-{index}-{delta_index}"); + let payload = bytes(len, 1); + repo.save( + "sql", + "space", + &name, + payload.clone(), + ArtifactExpectation::Any, + ) + .await + .unwrap(); + let wal = delta.map(|len| bytes(len, 2)); + if let Some(wal) = &wal { + repo.save_delta("sql", "space", &name, wal.clone(), ArtifactExpectation::Any) + .await + .unwrap(); + } + let mut sink = MemorySink::default(); + read_artifact(&repo, "sql", "space", &name, &mut sink) + .await + .unwrap() + .unwrap(); + assert_eq!(sink.checkpoint, payload, "{name}: checkpoint"); + assert_eq!(sink.wal, wal, "{name}: wal"); + } + } + + // Mid-read replacement restarts on Postgres too. + repo.save( + "sql", + "space", + "main", + bytes(2 * CHUNK + 7, 1), + ArtifactExpectation::Any, + ) + .await + .unwrap(); + let mut sink = race_sink(&repo, 1, |_| (bytes(CHUNK + 3, 9), None)); + read_artifact(&repo, "sql", "space", "main", &mut sink) + .await + .unwrap() + .unwrap(); + assert_eq!(sink.attempts, 2); + assert_eq!(sink.inner.checkpoint, bytes(CHUNK + 3, 9)); + + // And so does corruption. + fixture + .conn + .execute_unprepared("UPDATE database_artifact SET payload = decode(repeat('00', 8), 'hex') || substr(payload, 9) WHERE name = 'main'") + .await + .unwrap(); + let error = read_artifact(&repo, "sql", "space", "main", &mut MemorySink::default()) + .await + .expect_err("corrupt"); + assert!( + matches!(error, DatabaseArtifactError::HashMismatch { .. }), + "{error:?}" + ); + } + + fn names(dir: &std::path::Path) -> Vec { + let mut found: Vec = std::fs::read_dir(dir) + .unwrap() + .map(|entry| entry.unwrap().file_name().to_string_lossy().into_owned()) + .collect(); + found.sort(); + found + } + + async fn stage_both(sink: &mut CacheFileSink) { + for (part, body) in [ + (ArtifactPart::Checkpoint, b"cp"), + (ArtifactPart::Wal, b"wl"), + ] { + sink.begin_part(part).await.unwrap(); + sink.write(body).await.unwrap(); + sink.finish_part().await.unwrap(); + } + } + + #[tokio::test] + async fn a_failed_commit_leaves_no_staged_temp_files() { + for blocked in ["main.db", "main.db-wal"] { + let dir = tempfile::tempdir().unwrap(); + let mut sink = cache_sink(dir.path()); + stage_both(&mut sink).await; + // A non-empty directory at the destination makes the rename fail. + let target = dir.path().join(blocked); + std::fs::create_dir(&target).unwrap(); + std::fs::write(target.join("occupied"), b"x").unwrap(); + + sink.commit().await.unwrap_err(); + + assert!( + names(dir.path()).iter().all(|name| !name.contains(".tmp.")), + "{blocked}: {:?}", + names(dir.path()) + ); + drop(sink); + assert!(names(dir.path()).iter().all(|name| !name.contains(".tmp."))); + } + } + + #[tokio::test] + async fn the_sweep_removes_only_orphaned_staging_files() { + let dir = tempfile::tempdir().unwrap(); + let orphans = ["main.db.tmp.4242.0", "main.db-wal.tmp.4242.7"]; + let kept = [ + "main.db", + "main.db-wal", + "other.db.tmp.4242.0", + "main.db.tmp.abc.def", + "main.db.tmp.1", + "main.db.tmp.", + "main.db.tmpx.1.2", + ]; + for name in orphans.iter().chain(kept.iter()) { + std::fs::write(dir.path().join(name), b"x").unwrap(); + } + // A hydration in progress: one part still streaming, one finished. + let mut live = cache_sink(dir.path()); + live.begin_part(ArtifactPart::Checkpoint).await.unwrap(); + live.write(b"cp").await.unwrap(); + live.finish_part().await.unwrap(); + live.begin_part(ArtifactPart::Wal).await.unwrap(); + let live_files: Vec = names(dir.path()) + .into_iter() + .filter(|name| name.contains(&format!(".tmp.{}.", std::process::id()))) + .collect(); + assert_eq!(live_files.len(), 2, "{live_files:?}"); + + let main = dir.path().join("main.db"); + let wal = dir.path().join("main.db-wal"); + sweep_stale_staging(&[&main, &wal]).await; + + let left = names(dir.path()); + for name in orphans { + assert!(!left.iter().any(|n| n == name), "{name} survived: {left:?}"); + } + for name in kept.iter().map(|n| n.to_string()).chain(live_files) { + assert!(left.contains(&name), "{name} was removed: {left:?}"); + } + + // Once the hydration ends, its files are no longer protected. + drop(live); + assert!(names(dir.path()) + .iter() + .all(|n| !n.contains(&format!(".tmp.{}.", std::process::id())))); + } + + /// A corrupt size in the artifact metadata is only a hint: it must not + /// abort or panic the node, and the bytes still arrive intact. + #[tokio::test] + async fn tc1491_memory_sink_survives_an_absurd_size_hint() { + for hint in [ + u64::MAX, + usize::MAX as u64, + (isize::MAX as u64) + 1, + 1 << 62, + 1 << 40, + ] { + let mut sink = MemorySink::default(); + sink.begin_part(ArtifactPart::Checkpoint).await.unwrap(); + sink.size_hint(ArtifactPart::Checkpoint, hint); + sink.write(b"SQLite format 3\0").await.unwrap(); + sink.write(b"checkpoint").await.unwrap(); + sink.finish_part().await.unwrap(); + sink.begin_part(ArtifactPart::Wal).await.unwrap(); + sink.size_hint(ArtifactPart::Wal, hint); + sink.write(b"wal").await.unwrap(); + sink.finish_part().await.unwrap(); + assert_eq!(sink.checkpoint, b"SQLite format 3\0checkpoint", "{hint}"); + assert_eq!(sink.wal.as_deref(), Some(&b"wal"[..]), "{hint}"); + assert!( + sink.checkpoint.capacity() <= 64 * 1024 * 1024 + 64, + "a hint of {hint} reserved {} bytes", + sink.checkpoint.capacity() + ); + } + } } diff --git a/tinycloud-core/src/db_pool.rs b/tinycloud-core/src/db_pool.rs index 700271f3..cf1dab56 100644 --- a/tinycloud-core/src/db_pool.rs +++ b/tinycloud-core/src/db_pool.rs @@ -282,7 +282,7 @@ impl RecoveringConnection { /// transaction opened with `SET LOCAL statement_timeout`: three more /// round trips (`BEGIN`, `SET LOCAL`, `COMMIT`), so use it only on /// infrequent paths. Statements in it are not retried after a lost - /// connection; wrap a read in [`Self::bulk_read`] for that. Everywhere + /// connection; a read-only scope is not retried either. Everywhere /// else the scope is the pool itself, statement by statement, under the /// usual retry policy. pub async fn begin_bulk(&self) -> Result { @@ -306,30 +306,6 @@ impl RecoveringConnection { Ok(BulkScope::Transaction(transaction)) } - /// Run `read` in a [`Self::begin_bulk_read`] scope, and run the whole - /// attempt once more if it lost its connection within the retry window, - /// as a read-only statement on the pool would be. The attempt's `BEGIN` is - /// not retried on its own: nested retries would let one request wait for - /// several connection-loss detections. - pub async fn bulk_read(&self, operation: &'static str, read: F) -> Result - where - F: Fn(BulkScope) -> Fut, - Fut: Future>, - { - let Some(timeout) = self.bulk_statement_timeout() else { - // The scope is the pool, whose statements retry themselves. - return read(BulkScope::Pool(self.clone())).await; - }; - self.run(operation, true, || async { - let transaction = self.conn.begin().await?; - transaction - .execute_unprepared(&set_local_statement_timeout(timeout)) - .await?; - read(BulkScope::Transaction(transaction)).await - }) - .await - } - /// The `statement_timeout` a [`Self::begin_bulk`] transaction runs with, or /// `None` when no statement timeout applies to this pool. fn bulk_statement_timeout(&self) -> Option { @@ -985,7 +961,7 @@ const WRITE_KEYWORDS: [&str; 5] = ["INSERT", "UPDATE", "DELETE", "MERGE", "INTO" /// of anything else (`nextval`, `setval`, `pg_advisory_lock`, /// `pg_terminate_backend`, `set_config`, `lo_*`, a user-defined function) /// may have side effects, so it makes the statement non-retryable. -const CALL_ALLOWLIST: [&str; 36] = [ +const CALL_ALLOWLIST: [&str; 37] = [ // keywords "SELECT", "FROM", @@ -1024,6 +1000,7 @@ const CALL_ALLOWLIST: [&str; 36] = [ "COALESCE", "LOWER", "UPPER", + "SUBSTR", ]; #[derive(Debug, PartialEq, Eq)] diff --git a/tinycloud-core/src/duckdb/service.rs b/tinycloud-core/src/duckdb/service.rs index 49e469b5..ac88d42d 100644 --- a/tinycloud-core/src/duckdb/service.rs +++ b/tinycloud-core/src/duckdb/service.rs @@ -1,17 +1,15 @@ use std::{ collections::HashMap, path::{Path, PathBuf}, - sync::{ - atomic::{AtomicU64, Ordering}, - Arc, Weak, - }, + sync::{Arc, Weak}, }; use dashmap::DashMap; use tinycloud_auth::resource::SpaceId; use crate::database_artifacts::{ - ArtifactExpectation, DatabaseArtifactError, DatabaseArtifactRepository, + read_artifact, staging_path, sweep_stale_staging, ArtifactExpectation, CacheFileSink, + DatabaseArtifactError, DatabaseArtifactRepository, MemorySink, StagingRelease, }; use crate::request_telemetry::{self, QueryTimer}; @@ -207,21 +205,28 @@ impl DuckDbService { let operation_lock = Self::database_lock(&self.operation_locks, &key).await; let _operation = operation_lock.lock().await; - let artifact = self - .artifact_repository - .load("duckdb", &key.0, db_name) - .await - .map_err(artifact_error_to_duckdb)? - .ok_or(DuckDbError::DatabaseNotFound)?; + // The export is the response body, so it is buffered whole; the read + // itself still streams in slices. + let mut artifact = MemorySink::default(); + read_artifact( + self.artifact_repository.as_ref(), + "duckdb", + &key.0, + db_name, + &mut artifact, + ) + .await + .map_err(artifact_error_to_duckdb)? + .ok_or(DuckDbError::DatabaseNotFound)?; validate_payload( space, db_name, "checkpoint", - &artifact.payload, + &artifact.checkpoint, is_duckdb_file, )?; - let Some(delta) = artifact.delta_payload else { - return Ok(artifact.payload); + let Some(delta) = artifact.wal else { + return Ok(artifact.checkpoint); }; validate_payload(space, db_name, "wal", &delta, |payload| { !is_duckdb_file(payload) @@ -236,7 +241,7 @@ impl DuckDbService { let _context = request_telemetry::enter(context); let temp = tempfile::tempdir().map_err(|e| DuckDbError::Internal(e.to_string()))?; let path = temp.path().join("export.duckdb"); - std::fs::write(&path, artifact.payload) + std::fs::write(&path, artifact.checkpoint) .map_err(|e| DuckDbError::Internal(e.to_string()))?; std::fs::write(duckdb_wal_path(&path), delta) .map_err(|e| DuckDbError::Internal(e.to_string()))?; @@ -274,7 +279,8 @@ impl DuckDbService { .map_err(|e| DuckDbError::Internal(e.to_string()))?; let final_path = dir.join(format!("{}.duckdb", db_name)); - let temp_path = temp_write_path(&final_path); + let temp_path = staging_path(&final_path); + let _live = StagingRelease(temp_path.clone()); // Write to temp file first tokio::fs::write(&temp_path, data) @@ -414,51 +420,46 @@ impl DuckDbService { async fn hydrate_cache(&self, space: &SpaceId, db_name: &str) -> Result<(), DuckDbError> { let cache_path = self.cache_path(space, db_name); let key = (space.to_string(), db_name.to_string()); - match self - .artifact_repository - .load("duckdb", &space.to_string(), db_name) - .await - .map_err(artifact_error_to_duckdb)? + sweep_stale_staging(&[&cache_path, &duckdb_wal_path(&cache_path)]).await; + let mut sink = CacheFileSink::new( + "duckdb", + format!("{space}/{db_name}"), + cache_path.clone(), + duckdb_wal_path(&cache_path), + is_duckdb_file, + is_not_duckdb_file, + ); + match read_artifact( + self.artifact_repository.as_ref(), + "duckdb", + &space.to_string(), + db_name, + &mut sink, + ) + .await + .map_err(artifact_error_to_duckdb)? { - Some(artifact) => { + Some(meta) => { tracing::info!( service = "duckdb", - revision = artifact.revision, - storage_mode = %artifact.storage_mode, - bytes = artifact.payload.len(), - delta_bytes = artifact.delta_size_bytes, - logical_bytes = artifact.size_bytes, - content_hash = %artifact.content_hash, - checkpoint_content_hash = %artifact.checkpoint_content_hash, + revision = meta.revision, + storage_mode = %meta.storage_mode, + bytes = meta.checkpoint_size_bytes, + delta_bytes = meta.delta_size_bytes, + logical_bytes = meta.size_bytes, + content_hash = %meta.content_hash, + checkpoint_content_hash = %meta.checkpoint_content_hash, "Loaded database artifact" ); - // Refuse to seed the cache with bytes DuckDB would misread. The - // checkpoint must be a database; the WAL, which carries no - // header of its own, must at least not be one — that is the - // shape a crossed hydration writes. - validate_payload( - space, - db_name, - "checkpoint", - &artifact.payload, - is_duckdb_file, - )?; - if let Some(delta) = artifact.delta_payload.as_deref() { - validate_payload(space, db_name, "wal", delta, |payload| { - !is_duckdb_file(payload) - })?; - } - remove_duckdb_cache_files(&cache_path).await?; - write_cache_file(&cache_path, &artifact.payload).await?; - if let Some(delta) = artifact.delta_payload { - write_cache_file(&duckdb_wal_path(&cache_path), &delta).await?; - } + sink.commit() + .await + .map_err(|e| DuckDbError::Internal(e.to_string()))?; self.lineage.insert( key, ArtifactExpectation::Derived { - revision: artifact.revision, - checkpoint_content_hash: artifact.checkpoint_content_hash, + revision: meta.revision, + checkpoint_content_hash: meta.checkpoint_content_hash, }, ); Ok(()) @@ -571,7 +572,7 @@ impl DuckDbService { if without_growth { let durable_bytes = self .artifact_repository - .load("duckdb", &key.0, db_name) + .load_meta("duckdb", &key.0, db_name) .await .map_err(artifact_error_to_duckdb)? .map_or(0, |artifact| artifact.size_bytes.max(0) as u64); @@ -603,6 +604,12 @@ impl DuckDbService { } } +/// The WAL carries no header of its own; it must at least not be a database, +/// which is the shape a crossed hydration writes. +fn is_not_duckdb_file(payload: &[u8]) -> bool { + !is_duckdb_file(payload) +} + /// Whether `payload` carries DuckDB's main-database header. fn is_duckdb_file(payload: &[u8]) -> bool { payload @@ -633,42 +640,6 @@ fn validate_payload( ))) } -/// A temp sibling of `path`, unique per call. -/// -/// The suffix is APPENDED to the whole path. `Path::with_extension` replaces -/// everything after the last `.`, so `main.duckdb` and `main.duckdb.wal` both -/// mapped to `main.duckdb.tmp` — as did `import_db`'s own staging file. The -/// checkpoint and WAL writes of one hydration raced on a single file and -/// cross-consumed each other's bytes, reverting the database silently. -fn temp_write_path(path: &Path) -> PathBuf { - static NEXT_TEMP_ID: AtomicU64 = AtomicU64::new(0); - PathBuf::from(format!( - "{}.tmp.{}.{}", - path.display(), - std::process::id(), - NEXT_TEMP_ID.fetch_add(1, Ordering::Relaxed) - )) -} - -async fn write_cache_file(path: &Path, payload: &[u8]) -> Result<(), DuckDbError> { - if let Some(parent) = path.parent() { - tokio::fs::create_dir_all(parent) - .await - .map_err(|e| DuckDbError::Internal(e.to_string()))?; - } - - let temp_path = temp_write_path(path); - if let Err(e) = tokio::fs::write(&temp_path, payload).await { - let _ = tokio::fs::remove_file(&temp_path).await; - return Err(DuckDbError::Internal(e.to_string())); - } - if let Err(e) = tokio::fs::rename(&temp_path, path).await { - let _ = tokio::fs::remove_file(&temp_path).await; - return Err(DuckDbError::Internal(e.to_string())); - } - Ok(()) -} - async fn remove_duckdb_cache_files(path: &Path) -> Result<(), DuckDbError> { for candidate in [ path.to_path_buf(), @@ -688,6 +659,8 @@ fn duckdb_wal_path(path: &Path) -> PathBuf { PathBuf::from(format!("{}.wal", path.display())) } +/// `DuckDbError` has no retryable-unavailable variant (adding one would reach +/// every route mapping), so an exhausted `ArtifactChanged` stays `Internal`. fn artifact_error_to_duckdb(err: DatabaseArtifactError) -> DuckDbError { DuckDbError::Internal(err.to_string()) } @@ -696,7 +669,7 @@ fn artifact_error_to_duckdb(err: DatabaseArtifactError) -> DuckDbError { mod tests { use super::*; use crate::{ - database_artifacts::SeaOrmDatabaseArtifactRepository, + database_artifacts::{test_support::ReadAll, SeaOrmDatabaseArtifactRepository}, migrations::Migrator, sea_orm::{ConnectOptions, Database}, sea_orm_migration::MigratorTrait, @@ -788,7 +761,7 @@ mod tests { assert_eq!(sizes.space_total(&space).await, 0); assert!(matches!(exported, Err(DuckDbError::DatabaseNotFound))); assert!(repo - .load("duckdb", &space.to_string(), name) + .read_all("duckdb", &space.to_string(), name) .await .unwrap() .is_none()); @@ -847,7 +820,7 @@ mod tests { assert_eq!(sizes.space_total(&space).await, 0); assert!(matches!(exported, Err(DuckDbError::DatabaseNotFound))); assert!(repo - .load("duckdb", &space.to_string(), "absent") + .read_all("duckdb", &space.to_string(), "absent") .await .unwrap() .is_none()); @@ -888,7 +861,7 @@ mod tests { .expect("dropping an absent table is a successful no-op"); assert_eq!(sizes.space_total(&space).await, 0); assert!(repo - .load("duckdb", &space.to_string(), name) + .read_all("duckdb", &space.to_string(), name) .await .unwrap() .is_none()); @@ -933,7 +906,7 @@ mod tests { .await .unwrap(); let before = repo - .load("duckdb", &space.to_string(), "main") + .read_all("duckdb", &space.to_string(), "main") .await .unwrap() .unwrap(); @@ -973,7 +946,7 @@ mod tests { "{rejected:?}" ); let after = repo - .load("duckdb", &space.to_string(), "main") + .read_all("duckdb", &space.to_string(), "main") .await .unwrap() .unwrap(); @@ -1174,7 +1147,7 @@ mod tests { .await .unwrap(); let artifact = repo - .load("duckdb", &space.to_string(), "analytics") + .read_all("duckdb", &space.to_string(), "analytics") .await .unwrap() .unwrap(); @@ -1205,7 +1178,7 @@ mod tests { .unwrap(); assert_eq!(names, "one"); let after = repo - .load("duckdb", &space.to_string(), "analytics") + .read_all("duckdb", &space.to_string(), "analytics") .await .unwrap() .unwrap(); diff --git a/tinycloud-core/src/sql/database.rs b/tinycloud-core/src/sql/database.rs index 1efdc553..ab3dd99c 100644 --- a/tinycloud-core/src/sql/database.rs +++ b/tinycloud-core/src/sql/database.rs @@ -6,7 +6,7 @@ //! SQL query time. use std::path::{Path, PathBuf}; -use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::atomic::{AtomicU64, AtomicU8, Ordering}; use std::sync::Arc; use dashmap::DashMap; @@ -19,6 +19,10 @@ use super::{ authority::{record_ddl_without_schema, SqlAuthority}, authorizer::{PragmaTracker, StatementScope}, caveats::SqlCaveats, + durability::{ + cache_used_bytes, unspilled_wal_bound, DurabilityPolicy, HeldGuard, ReservationRefused, + WriteProbe, + }, parser, storage::{self, StorageMode}, types::*, @@ -59,6 +63,11 @@ enum DbMessage { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum GrowthPolicy { Allow, + /// The owner's storage budget has this many bytes left: the write may grow + /// the database by at most that. SQLite enforces it (`max_page_count`), so + /// a write that would pass it is rolled back before COMMIT with + /// [`SqlError::StorageWouldGrow`] instead of overshooting by one write. + Budget(u64), /// The space is full: run the request, but undo it and fail with /// [`SqlError::StorageWouldGrow`] if the database grew. No-op DDL, /// `DELETE` and `DROP` free pages instead of adding them, so they pass. @@ -77,7 +86,7 @@ pub struct DatabaseHandle { impl DatabaseHandle { /// Close the connection before acknowledging that its local files can be removed. - pub(crate) async fn shutdown(&self) -> Result<(), SqlError> { + pub async fn shutdown(&self) -> Result<(), SqlError> { let (response_tx, response_rx) = oneshot::channel(); self.tx .send(( @@ -195,6 +204,26 @@ pub fn spawn_actor( base_path: String, memory_threshold: u64, databases: Arc>, +) -> DatabaseHandle { + spawn_actor_with_policy( + space_id, + db_name, + base_path, + memory_threshold, + databases, + DurabilityPolicy::default(), + ) +} + +/// [`spawn_actor`] with the node's durability policy (TC-1491): the +/// `synchronous` level, the size cap and the disk headroom. +pub fn spawn_actor_with_policy( + space_id: String, + db_name: String, + base_path: String, + memory_threshold: u64, + databases: Arc>, + policy: DurabilityPolicy, ) -> DatabaseHandle { let (tx, mut rx) = mpsc::channel::<(Option>, DbMessage)>(32); let id = NEXT_ACTOR_ID.fetch_add(1, Ordering::Relaxed); @@ -214,7 +243,7 @@ pub fn spawn_actor( }; let mut conn = { let _context = request_telemetry::enter(initial_context); - let Ok(connection) = storage::open_connection(&mode) else { + let Ok(connection) = storage::open_connection(&mode, policy.synchronous) else { tracing::error!("Failed to open database"); databases.remove_if(&(space_id, db_name), |_, handle| handle.id == id); return; @@ -222,6 +251,11 @@ pub fn spawn_actor( connection }; + // What this database holds of the disk budget is dropped when its + // connection is gone, however the actor ends. + let key = format!("{space_id}/{db_name}"); + let held_guard = HeldGuard::new(policy.budget.clone(), key.clone()); + let shutdown_response = loop { // Block on receiving with timeout let (context, msg) = @@ -260,35 +294,56 @@ pub fn spawn_actor( response_tx, } => { let pragmas = PragmaTracker::default(); + let ctx = WriteContext { + policy: &policy, + key: &key, + dir: existing_ancestor(&file_path), + file_backed: matches!(mode, StorageMode::File(_)), + remaining: match growth { + GrowthPolicy::Budget(remaining) => Some(remaining), + _ => None, + }, + file: &file_path, + }; let result = match growth { - GrowthPolicy::Allow => handle_message_authorized( - &conn, &request, &caveats, &authority, &pragmas, + GrowthPolicy::Allow | GrowthPolicy::Budget(_) => handle_message_ctx( + &conn, &request, &caveats, &authority, &pragmas, &ctx, ), - GrowthPolicy::Forbid => handle_message_without_growth_authorized( - &conn, &request, &caveats, &authority, &pragmas, + GrowthPolicy::Forbid => handle_message_without_growth_ctx( + &conn, &request, &caveats, &authority, &pragmas, &ctx, ), }; conn.authorizer(None::) -> Authorization>); + inject_open_transaction(&conn, &space_id, &db_name); + // A request ends its transaction, by commit or rollback. + // One still open here (a rollback that itself failed) + // would swallow the next request's writes, so serve no + // further request on this connection. + // // TC-1586: the connection is shared by every request on // this database. After any request in which the // authorizer allowed a PRAGMA outside the plain schema // reports, reset it; if that fails, serve no further - // request on this connection (the next one rebuilds it). - if pragmas.load(Ordering::Relaxed) + // request on this connection either (the next one + // rebuilds it). + let retire = if !conn.is_autocommit() { + Some("SQL connection transaction could not be ended") + } else if pragmas.load(Ordering::Relaxed) && connection_reset_failed(&conn, &mode, &space_id, &db_name) { - tracing::error!( - "SQL connection state could not be reset; retiring the database actor" - ); + Some("SQL connection state could not be reset") + } else { + None + }; + if let Some(reason) = retire { + tracing::error!("{reason}; retiring the database actor"); // Deregister first, so the request that follows the // error builds a new actor instead of finding this one. databases.remove_if(&(space_id.clone(), db_name.clone()), |_, handle| { handle.id == id }); - let _ = response_tx.send(Err(SqlError::Internal( - "SQL connection state could not be reset".into(), - ))); + let _ = response_tx.send(Err(SqlError::Internal(reason.into()))); break None; } @@ -296,20 +351,48 @@ pub fn spawn_actor( if result.is_ok() && matches!(mode, StorageMode::InMemory) { if let Ok(size) = storage::database_size(&conn) { if size > memory_threshold { - match storage::promote_to_file(&conn, &file_path) { - Ok(new_conn) => { - conn = new_conn; - mode = StorageMode::File(file_path.clone()); - tracing::info!("Promoted database to file storage"); - } - Err(_) => { - tracing::error!("Failed to promote database to file"); + // The copy needs the whole database on disk, + // next to what in-flight writes still need. + let room = if policy.min_free_bytes == 0 { + Ok(()) + } else { + policy.budget.reserve( + &key, + size, + 0, + policy.free_space.as_ref(), + existing_ancestor(&file_path), + policy.min_free_bytes, + ) + }; + if room.is_err() { + tracing::warn!( + "Not enough free disk to promote a database; keeping it in memory" + ); + } else { + let promoted = storage::promote_to_file( + &conn, + &file_path, + policy.synchronous, + ); + // The copy is on disk now. + policy.budget.release(&key); + match promoted { + Ok(new_conn) => { + conn = new_conn; + mode = StorageMode::File(file_path.clone()); + tracing::info!("Promoted database to file storage"); + } + Err(_) => { + tracing::error!("Failed to promote database to file"); + } } } } } } + seam(&policy.seams.before_response); let _ = response_tx.send(result); } DbMessage::Export { response_tx } => { @@ -318,6 +401,11 @@ pub fn spawn_actor( } DbMessage::Checkpoint { response_tx } => { let result = handle_checkpoint(&conn, &mode, &file_path); + if result.is_ok() { + // The WAL is in the main file now: its growth is on + // the disk and no longer pending. + policy.budget.forget(&key); + } let _ = response_tx.send(result); } DbMessage::Wal { response_tx } => { @@ -330,6 +418,7 @@ pub fn spawn_actor( // SQLite can checkpoint its WAL on close. Finish that before removing // the registry entry or allowing a service to replace the local files. drop(conn); + drop(held_guard); // Deregister only OUR OWN entry. A replacement actor may already have // been hydrated and registered for this key (the dead-actor respawn in @@ -348,6 +437,14 @@ pub fn spawn_actor( DatabaseHandle { id, tx } } +/// The nearest existing ancestor of `path`: where `statvfs` can be asked how +/// much room the filesystem has before the file or its directory exists. +fn existing_ancestor(path: &Path) -> &Path { + path.ancestors() + .find(|ancestor| !ancestor.as_os_str().is_empty() && ancestor.exists()) + .unwrap_or_else(|| Path::new(".")) +} + fn handle_wal(mode: &StorageMode, file_path: &Path) -> Result>, SqlError> { if !matches!(mode, StorageMode::File(_)) { return Ok(None); @@ -415,34 +512,22 @@ fn handle_checkpoint( const GUARD_BEGIN: &str = "SAVEPOINT tinycloud_storage_guard"; const GUARD_KEEP: &str = "RELEASE tinycloud_storage_guard"; -const GUARD_UNDO: &str = "ROLLBACK TO tinycloud_storage_guard; RELEASE tinycloud_storage_guard"; +/// The guard's savepoint is the outermost transaction, so undoing the request +/// is a ROLLBACK: it writes nothing, where `ROLLBACK TO` + `RELEASE` would +/// commit an empty write transaction (a WAL frame for the header). +const GUARD_UNDO: &str = "ROLLBACK"; /// Run `request` inside a savepoint and keep its effects only if the /// database did not grow (`PRAGMA page_count` reflects uncommitted pages on /// this connection). Every failed guarded request is rolled back, including /// schema entries and batch statements that ran before a later rejection. -#[cfg(test)] -fn handle_message_without_growth( - conn: &rusqlite::Connection, - request: &SqlRequest, - caveats: &Option, - ability: &str, -) -> Result { - handle_message_without_growth_authorized( - conn, - request, - caveats, - &SqlAuthority::from(ability), - &PragmaTracker::default(), - ) -} - -fn handle_message_without_growth_authorized( +fn handle_message_without_growth_ctx( conn: &rusqlite::Connection, request: &SqlRequest, caveats: &Option, authority: &SqlAuthority, pragmas: &PragmaTracker, + ctx: &WriteContext<'_>, ) -> Result { let before = page_count(conn)?; { @@ -450,7 +535,7 @@ fn handle_message_without_growth_authorized( conn.execute_batch(GUARD_BEGIN) .map_err(|e| SqlError::Internal(e.to_string()))?; } - let result = handle_message_authorized(conn, request, caveats, authority, pragmas); + let result = handle_message_ctx(conn, request, caveats, authority, pragmas, ctx); // An early error return can leave the request's authorizer installed. conn.authorizer(None::) -> Authorization>); // ROLLBACK conflict handling can already have undone the transaction. @@ -458,11 +543,31 @@ fn handle_message_without_growth_authorized( return result; } let grew = page_count(conn).map(|after| after > before); + // The disk check before COMMIT, for a request that is about to be kept + // and has written something. `Some` is what stays held afterwards. + let gate = match (&result, &grew) { + (Ok(_), Ok(false)) if WriteProbe::of(conn).writing() => baseline(conn) + .and_then(|base| { + precommit_gate( + conn, + ctx, + &Baseline { + pages: u64::try_from(before).unwrap_or(0), + page_size: base.page_size, + }, + ) + }) + .map(Some), + _ => Ok(None), + }; let end = match grew { - Ok(false) if result.is_ok() => GUARD_KEEP, + Ok(false) if result.is_ok() && gate.is_ok() => GUARD_KEEP, // Undo earlier statements on any error, even without page growth. _ => GUARD_UNDO, }; + if end == GUARD_KEEP { + seam(&ctx.policy.seams.before_commit); + } let end_result = { let _timer = QueryTimer::new(); conn.execute_batch(end) @@ -476,10 +581,25 @@ fn handle_message_without_growth_authorized( conn.execute_batch(GUARD_UNDO) .map_err(|e| SqlError::Internal(e.to_string()))?; } - return Err(SqlError::Sqlite(error.to_string())); + ctx.policy.budget.release(ctx.key); + return Err(commit_error(&error)); } + let held = match gate { + Ok(held) => held, + Err(error) => { + ctx.policy.budget.release(ctx.key); + return Err(error); + } + }; match grew? { - false => result, + false => { + seam(&ctx.policy.seams.after_commit); + // The WAL bytes are on disk; the checkpoint growth is what stays. + if let Some(pending) = held { + ctx.policy.budget.hold(ctx.key, pending); + } + result + } true => Err(SqlError::StorageWouldGrow), } } @@ -490,6 +610,296 @@ fn page_count(conn: &rusqlite::Connection) -> Result { .map_err(|e| SqlError::Internal(e.to_string())) } +/// The unit tests drive a bare connection: no disk, no budget. +#[cfg(test)] +fn handle_message( + conn: &rusqlite::Connection, + request: &SqlRequest, + caveats: &Option, + ability: &str, +) -> Result { + let policy = DurabilityPolicy::default(); + let ctx = WriteContext { + policy: &policy, + key: "test/test", + dir: Path::new("."), + file_backed: false, + remaining: None, + file: Path::new(""), + }; + handle_message_ctx( + conn, + request, + caveats, + &SqlAuthority::from(ability), + &PragmaTracker::default(), + &ctx, + ) +} + +#[cfg(test)] +fn handle_message_without_growth( + conn: &rusqlite::Connection, + request: &SqlRequest, + caveats: &Option, + ability: &str, +) -> Result { + let policy = DurabilityPolicy::default(); + let ctx = WriteContext { + policy: &policy, + key: "test/test", + dir: Path::new("."), + file_backed: false, + remaining: None, + file: Path::new(""), + }; + handle_message_without_growth_ctx( + conn, + request, + caveats, + &SqlAuthority::from(ability), + &PragmaTracker::default(), + &ctx, + ) +} + +const REQUEST_BEGIN: &str = "SAVEPOINT tinycloud_request"; +const REQUEST_KEEP: &str = "RELEASE tinycloud_request"; +/// SQLite's default `max_page_count`. +const UNLIMITED_PAGES: u64 = 4_294_967_294; +/// Opcodes between the disk checks of a running statement. +const PROGRESS_INTERVAL_OPS: std::os::raw::c_int = 10_000; +/// What the progress handler saw: the disk below its floor, or free space it +/// could not read. +const TRIPPED_NONE: u8 = 0; +const TRIPPED_EXHAUSTED: u8 = 1; +const TRIPPED_UNKNOWN: u8 = 2; + +/// What a write request needs to be bounded before it commits (TC-1491). +struct WriteContext<'a> { + policy: &'a DurabilityPolicy, + /// `space/db`, the key of this database's reservation. + key: &'a str, + /// An existing directory on the filesystem holding the database. + dir: &'a Path, + /// True once the database is a file (and so has a WAL and uses the disk). + file_backed: bool, + /// Bytes the owner's storage budget still allows this database to grow + /// by, or `None` for no budget. + remaining: Option, + /// The database's main file (it may not exist yet). + file: &'a Path, +} + +/// The state a request starts from. +struct Baseline { + pages: u64, + page_size: u64, +} + +fn pragma_u64(conn: &rusqlite::Connection, pragma: &str) -> Result { + conn.query_row(&format!("PRAGMA {pragma}"), [], |row| row.get::<_, i64>(0)) + .map(|value| u64::try_from(value).unwrap_or(0)) + .map_err(|e| SqlError::Internal(e.to_string())) +} + +fn baseline(conn: &rusqlite::Connection) -> Result { + Ok(Baseline { + pages: pragma_u64(conn, "page_count")?, + page_size: pragma_u64(conn, "page_size")?, + }) +} + +fn set_max_pages(conn: &rusqlite::Connection, pages: u64) -> Result<(), SqlError> { + conn.query_row(&format!("PRAGMA max_page_count = {pages}"), [], |_| Ok(())) + .map_err(|e| SqlError::Internal(e.to_string())) +} + +/// The page limit SQLite enforces for this request, and whether the owner's +/// budget (rather than the per-database cap) is the tighter of the two. SQLite +/// never sets it below the current size, so a database already over the cap +/// can still run writes that do not grow it. +fn page_limit(ctx: &WriteContext<'_>, baseline: &Baseline) -> (u64, bool) { + let page_size = baseline.page_size.max(1); + let cap = match ctx.policy.max_database_bytes { + 0 => UNLIMITED_PAGES, + bytes => (bytes / page_size).max(1), + }; + match ctx.remaining { + Some(remaining) => { + let budget = baseline.pages.saturating_add(remaining / page_size); + (budget.min(cap), budget < cap) + } + None => (cap, false), + } +} + +fn is_full_error(error: &SqlError) -> bool { + matches!(error, SqlError::Sqlite(m) | SqlError::SchemaError(m) if m.contains("database or disk is full")) +} + +/// A disk with fewer free bytes than this many database pages cannot take +/// another write. Below it, a `SQLITE_FULL` is the disk's (ENOSPC) whatever +/// the headroom setting: a fixed, small floor, so that no configuration is +/// needed for a full disk to be reported as one. +const DISK_FULL_FLOOR_PAGES: u64 = 16; + +/// What a `SQLITE_FULL` means: the size limit SQLite enforces (`max_page_count`) +/// or the disk itself (ENOSPC). The page count cannot tell them apart, since +/// SQLite has already undone the failed statement by the time it is read; the +/// free-space reading can, and it is read here whatever `min_free_bytes` is +/// (one `statvfs`, on this error path only): less than [`DISK_FULL_FLOOR_PAGES`] +/// pages free, or less than the headroom floor next to what other writes +/// reserved, is the disk. A full disk is never reported as the owner's quota +/// or the database's size limit. +fn classify_full(ctx: &WriteContext<'_>, baseline: &Baseline, budget_binding: bool) -> SqlError { + if ctx.policy.max_database_bytes == 0 && !budget_binding { + // No limit of ours can have fired, so the disk really is full. + return SqlError::DiskHeadroom; + } + let min_free = ctx.policy.min_free_bytes; + let disk_exhausted = ctx.file_backed + && match ctx.policy.free_space.free_bytes(ctx.dir) { + Ok(free) => { + free < DISK_FULL_FLOOR_PAGES.saturating_mul(baseline.page_size) + || (min_free > 0 + && free + < ctx + .policy + .budget + .reserved_excluding(ctx.key) + .saturating_add(min_free)) + } + // Unreadable: with the headroom check on, refuse like the gate does. + Err(_) => min_free > 0, + }; + match (disk_exhausted, budget_binding) { + (true, _) => SqlError::DiskHeadroom, + (false, true) => SqlError::StorageWouldGrow, + (false, false) => SqlError::DatabaseSizeLimit, + } +} + +/// Free space could not be read: the headroom check cannot answer, so the +/// request is refused. One error for every place that reads it. +fn free_space_unreadable() -> SqlError { + SqlError::Internal("could not read the free disk space".into()) +} + +/// Bytes the next checkpoint will grow the main file by: the pages the +/// database has beyond the file's own size, all of them committed since the +/// last checkpoint and held in the WAL until then. Read from the file, so it +/// needs no state of its own and counts every earlier commit, not only this +/// request's. +fn pending_checkpoint_bytes(ctx: &WriteContext<'_>, pages: u64, page_size: u64) -> u64 { + let file_pages = std::fs::metadata(ctx.file) + .map(|meta| meta.len() / page_size.max(1)) + .unwrap_or(pages); + pages.saturating_sub(file_pages).saturating_mul(page_size) +} + +/// The last check before COMMIT, for a request that has written. Rolling back +/// on an error is the caller's. On success, returns the bytes of checkpoint +/// growth to keep held once the commit is done (the rest of the reservation +/// ends with the commit). +/// +/// The disk bound: the pages COMMIT still writes to the WAL are the dirty +/// pages that have not spilled yet, and those are all in the connection's +/// page cache, so `cache_used` bounds them whatever the SQL payload was +/// (index, trigger and foreign-key writes, freelist and overflow pages are +/// all just pages). A later checkpoint grows the main file by every page +/// committed since the last one ([`pending_checkpoint_bytes`]), which is held +/// until a checkpoint runs. WAL bytes already spilled are on disk and already +/// out of the free space the check reads. +fn precommit_gate( + conn: &rusqlite::Connection, + ctx: &WriteContext<'_>, + baseline: &Baseline, +) -> Result { + let now = pragma_u64(conn, "page_count")?; + let growth_pages = now.saturating_sub(baseline.pages); + let growth_bytes = growth_pages.saturating_mul(baseline.page_size); + if ctx + .remaining + .is_some_and(|remaining| growth_bytes > remaining) + { + return Err(SqlError::StorageWouldGrow); + } + if growth_pages > 0 + && ctx.policy.max_database_bytes > 0 + && now.saturating_mul(baseline.page_size) > ctx.policy.max_database_bytes + { + return Err(SqlError::DatabaseSizeLimit); + } + if !ctx.file_backed || ctx.policy.min_free_bytes == 0 { + return Ok(0); + } + let wal_bound = unspilled_wal_bound(cache_used_bytes(conn), baseline.page_size); + let pending = pending_checkpoint_bytes(ctx, now, baseline.page_size); + match ctx.policy.budget.reserve( + ctx.key, + wal_bound, + pending, + ctx.policy.free_space.as_ref(), + ctx.dir, + ctx.policy.min_free_bytes, + ) { + Ok(()) => Ok(pending), + Err(ReservationRefused::Headroom) => Err(SqlError::DiskHeadroom), + Err(ReservationRefused::Unknown) => Err(free_space_unreadable()), + } +} + +fn seam(hook: &Option>) { + if let Some(hook) = hook { + hook(); + } +} + +/// Whether `request` runs in a transaction of its own (TC-1491). +/// +/// Every request that can write does, so the checks before COMMIT have a +/// transaction to roll back, and the request is all-or-nothing whatever its +/// conflict clauses say: a statement that stops with `OR FAIL` or +/// `ON CONFLICT FAIL` keeps the rows it changed first when it runs on its +/// own, and a request with several statements keeps its earlier ones. +/// A request that carries a `PRAGMA` (admin only) is wrapped like any other, +/// and so is limited, gated and reserved like any write. Every pragma the +/// authorizer admits is transactional (`user_version` and `application_id` +/// are undone by the rollback; the rest are reports), and the authorizer's +/// allow-list is what keeps connection-level settings out of the +/// transaction. The actor's TC-1586 connection reset runs after the +/// transaction has ended. Not wrapped: a query that only reads. +/// +/// Whether a query writes is `parser::classify_sql` (TC-1585), which sees +/// writes nested in a query tree: a `WITH … INSERT` is wrapped, gated and +/// reserved like any other write. It does not depend on the caller's +/// abilities (a read-tier grant is only a shortcut: its writes are refused +/// anyway), and a statement that does not parse is left to +/// `handle_request_authorized` to report. This is the one place that decides +/// it; `Execute`, `Batch` and `ExecuteStatement` are wrapped whatever they +/// contain, a read in a batch included. +fn needs_transaction( + request: &SqlRequest, + caveats: &Option, + authority: &SqlAuthority, +) -> bool { + match request { + SqlRequest::Query { sql, .. } => { + !authority.is_read_tier_only() + && (parser::is_pragma_sql(sql) + || parser::classify_sql(sql) + .map(|class| !class.is_read_only) + .unwrap_or(false)) + } + SqlRequest::Execute { .. } | SqlRequest::Batch { .. } => true, + SqlRequest::ExecuteStatement { name, .. } => caveats + .as_ref() + .is_some_and(|c| c.find_statement(name).is_some()), + SqlRequest::Export => false, + } +} + /// `DdlPolicy::Log` bookkeeping for one request: count every statement that /// only `write` let through, and warn once. The message is constant and the /// counter is labelled by statement kind alone, so neither carries SQL text, @@ -512,23 +922,182 @@ impl DdlLog { } } -#[cfg(test)] -fn handle_message( +/// Run `request` as one transaction when it needs one: every statement is +/// kept or none is. +/// +/// The transaction is a `SAVEPOINT`, which opens one when none is active; +/// users cannot issue transaction control themselves, the authorizer denies +/// it. A failure at any statement, or at the commit (a deferred constraint), +/// is a `ROLLBACK`: the local file is unchanged and matches the durable copy, +/// which is only written after a fully successful request. See +/// [`needs_transaction`] for what is wrapped. The wrap sits inside the request +/// and outside the actor's TC-1586 connection reset, which runs after the +/// transaction has ended. +/// +/// When a transaction is already open, [`handle_message_without_growth_ctx`]'s +/// guard has it and already undoes the whole request on any error (reporting +/// growth ahead of the SQL error), so this adds nothing there. +/// +/// Order, for a write: limits (SQLite's `max_page_count`: the per-database +/// cap and the owner's remaining budget), the statements, the pre-commit gate +/// ([`precommit_gate`]), COMMIT. Any failure before COMMIT is a ROLLBACK. +/// +/// If the transaction is still open after the final `ROLLBACK`, the actor +/// retires the connection instead of serving another request on it. +fn handle_message_ctx( conn: &rusqlite::Connection, request: &SqlRequest, caveats: &Option, - ability: &str, + authority: &SqlAuthority, + pragmas: &PragmaTracker, + ctx: &WriteContext<'_>, ) -> Result { - handle_message_authorized( - conn, - request, - caveats, - &SqlAuthority::from(ability), - &PragmaTracker::default(), - ) + if !conn.is_autocommit() || !needs_transaction(request, caveats, authority) { + return handle_request_authorized(conn, request, caveats, authority, pragmas); + } + let base = baseline(conn)?; + set_max_pages(conn, page_limit(ctx, &base).0)?; + let tripped = Arc::new(AtomicU8::new(TRIPPED_NONE)); + let disk_checked = ctx.file_backed && ctx.policy.min_free_bytes > 0; + if disk_checked { + let (budget, free_space) = (ctx.policy.budget.clone(), ctx.policy.free_space.clone()); + let (key, dir) = (ctx.key.to_string(), ctx.dir.to_path_buf()); + let (min_free, tripped) = (ctx.policy.min_free_bytes, tripped.clone()); + let probe = WriteProbe::of(conn); + // Stops a statement early once the disk is already too full; the + // authoritative check is `precommit_gate`. A transaction that has + // written nothing yet needs no room, so reads are never stopped. + conn.progress_handler( + PROGRESS_INTERVAL_OPS, + Some(move || { + if !probe.writing() { + return false; + } + let Ok(free) = free_space.free_bytes(&dir) else { + tripped.store(TRIPPED_UNKNOWN, Ordering::Relaxed); + return true; + }; + let others = budget.reserved_excluding(&key); + let exhausted = free < others.saturating_add(min_free); + if exhausted { + tripped.store(TRIPPED_EXHAUSTED, Ordering::Relaxed); + } + exhausted + }), + ); + } + let outcome = run_transaction(conn, request, caveats, authority, pragmas, ctx, &base); + if disk_checked { + conn.progress_handler(0, None:: bool>); + } + let outcome = outcome.map_err(|error| match tripped.load(Ordering::Relaxed) { + TRIPPED_EXHAUSTED => SqlError::DiskHeadroom, + TRIPPED_UNKNOWN => free_space_unreadable(), + _ => error, + }); + // A failed request keeps nothing, so nothing of it stays reserved. + if outcome.is_err() { + ctx.policy.budget.release(ctx.key); + } + let _ = set_max_pages(conn, UNLIMITED_PAGES); + outcome.map(|(result, held)| { + // Committed: its WAL bytes are on disk now. What stays held, until a + // checkpoint runs, is the checkpoint growth. A request that wrote + // nothing changes nothing here. + if let Some(pending) = held { + ctx.policy.budget.hold(ctx.key, pending); + } + result + }) } -fn handle_message_authorized( +/// The transaction itself; on `Err` it has been rolled back. On success, the +/// checkpoint growth to keep held, if the request wrote anything. +fn run_transaction( + conn: &rusqlite::Connection, + request: &SqlRequest, + caveats: &Option, + authority: &SqlAuthority, + pragmas: &PragmaTracker, + ctx: &WriteContext<'_>, + base: &Baseline, +) -> Result<(SqlExecutionResult, Option), SqlError> { + { + let _timer = QueryTimer::new(); + conn.execute_batch(REQUEST_BEGIN) + .map_err(|e| SqlError::Internal(e.to_string()))?; + } + let result = handle_request_authorized(conn, request, caveats, authority, pragmas); + // An early error return can leave the request's authorizer installed. + conn.authorizer(None::) -> Authorization>); + // The error is surfaced before the rollback so the caller can still read + // the uncommitted page count to tell a size limit from a full disk. + // A request that took no write transaction (reads only, in a batch or an + // `execute`) wrote nothing: there is nothing to check, reserve or hold. + let result = result.and_then(|ok| { + if WriteProbe::of(conn).writing() { + precommit_gate(conn, ctx, base).map(|pending| (ok, Some(pending))) + } else { + Ok((ok, None)) + } + }); + let (result, held) = match result { + Ok(ok) => ok, + Err(error) => { + // SQLite can already have undone the transaction (ROLLBACK + // conflict handling); there is then nothing left to undo. + let full = is_full_error(&error); + let classified = full.then(|| classify_full(ctx, base, page_limit(ctx, base).1)); + if !conn.is_autocommit() { + // A failure rolls the whole transaction back. `ROLLBACK TO` + + // `RELEASE` would instead commit an empty write transaction + // and rewrite the header. + let _timer = QueryTimer::new(); + let _ = conn.execute_batch("ROLLBACK"); + } + return Err(classified.unwrap_or(error)); + } + }; + seam(&ctx.policy.seams.before_commit); + // RELEASE of the outermost savepoint is the COMMIT. + let commit = { + let _timer = QueryTimer::new(); + conn.execute_batch(REQUEST_KEEP) + }; + if let Err(error) = commit { + if !conn.is_autocommit() { + let _timer = QueryTimer::new(); + let _ = conn.execute_batch("ROLLBACK"); + } + return Err(commit_error(&error)); + } + seam(&ctx.policy.seams.after_commit); + Ok((result, held)) +} + +/// COMMIT failed. A deferred constraint is the request's own fault; anything +/// else is the disk. +/// +/// If COMMIT's fsync fails after the WAL frames were written, SQLite reports +/// the error and rolls the transaction back in memory, but the frames are +/// already in the file and, if their commit mark reached it, the commit can +/// reappear when the database is next opened — although we reported the +/// request as rejected. That is harmless while Postgres is master: the caller +/// discards the local state and re-hydrates from Postgres, which never saw the +/// write. Under store-master (1b+) it is an outcome-unknown case: the local +/// WAL must be kept and the request answered `outcome_unknown`, not rejected. +fn commit_error(error: &rusqlite::Error) -> SqlError { + match error.sqlite_error_code() { + Some(rusqlite::ErrorCode::ConstraintViolation) => SqlError::Sqlite(error.to_string()), + Some(rusqlite::ErrorCode::DiskFull) => SqlError::DiskHeadroom, + code => { + tracing::error!(?code, "Local SQL commit failed"); + SqlError::CommitFailed + } + } +} + +fn handle_request_authorized( conn: &rusqlite::Connection, request: &SqlRequest, caveats: &Option, @@ -762,6 +1331,27 @@ fn injected_reset_failure(_space_id: &str, _db_name: &str) -> bool { false } +/// Test seam: databases whose connection is left inside a transaction after +/// their next request, as a failed final `ROLLBACK` would leave it. +#[cfg(test)] +pub(crate) static OPEN_TRANSACTIONS: std::sync::Mutex> = + std::sync::Mutex::new(Vec::new()); + +#[cfg(test)] +fn inject_open_transaction(conn: &rusqlite::Connection, space_id: &str, db_name: &str) { + let injected = OPEN_TRANSACTIONS + .lock() + .unwrap() + .iter() + .any(|(space, db)| space == space_id && db == db_name); + if injected { + conn.execute_batch("BEGIN").unwrap(); + } +} + +#[cfg(not(test))] +fn inject_open_transaction(_conn: &rusqlite::Connection, _space_id: &str, _db_name: &str) {} + fn is_insert_statement(parsed: &parser::ParsedQuery) -> bool { matches!( parsed.statements.as_slice(), @@ -901,6 +1491,7 @@ fn execute_statement( #[cfg(test)] mod tests { use super::*; + use crate::sql::durability::Synchronous; #[tokio::test] async fn tc626_review_release_escape_cannot_survive_checkpoint() { @@ -992,7 +1583,8 @@ mod tests { "BEGIN", ] { for prefix in ["", "PRAGMA user_version = 1; /* boundary; */ "] { - let conn = storage::open_connection(&StorageMode::InMemory).unwrap(); + let conn = + storage::open_connection(&StorageMode::InMemory, Synchronous::Full).unwrap(); conn.execute_batch( "CREATE TABLE data (value TEXT); INSERT INTO data VALUES ('before')", ) @@ -1030,7 +1622,7 @@ mod tests { #[test] fn tc626_review_single_pragma_preserves_quoted_semicolons_and_comments() { - let conn = storage::open_connection(&StorageMode::InMemory).unwrap(); + let conn = storage::open_connection(&StorageMode::InMemory, Synchronous::Full).unwrap(); conn.execute_batch("CREATE TABLE \"quoted;name\" (value TEXT)") .unwrap(); let result = handle_message_without_growth( @@ -1070,7 +1662,7 @@ mod tests { #[test] fn growth_guard_rolls_back_failed_deferred_commit() { - let conn = storage::open_connection(&StorageMode::InMemory).unwrap(); + let conn = storage::open_connection(&StorageMode::InMemory, Synchronous::Full).unwrap(); conn.execute_batch( "CREATE TABLE parents (id INTEGER PRIMARY KEY); CREATE TABLE children ( @@ -1380,6 +1972,419 @@ mod publication_rollback_tests { "Original" ); } + + // TC-1491: a request with several statements is all-or-nothing. + + fn exec(sql: &str) -> SqlRequest { + SqlRequest::Execute { + schema: None, + sql: sql.into(), + params: vec![], + } + } + + fn batch(statements: &[&str]) -> SqlRequest { + SqlRequest::Batch { + statements: statements + .iter() + .map(|sql| SqlStatement { + sql: (*sql).into(), + params: vec![], + }) + .collect(), + } + } + + async fn run( + actor: &DatabaseHandle, + request: SqlRequest, + growth: GrowthPolicy, + ) -> Result { + actor + .execute(request, None, "tinycloud.sql/admin".into(), growth) + .await + } + + async fn scalar(actor: &DatabaseHandle, sql: &str) -> String { + let result = run( + actor, + SqlRequest::Query { + sql: sql.into(), + params: vec![], + max_rows: None, + max_bytes: None, + }, + GrowthPolicy::Allow, + ) + .await + .unwrap(); + match result.response { + SqlResponse::Query(query) => match &query.rows[0][0] { + SqlValue::Integer(value) => value.to_string(), + SqlValue::Text(value) => value.clone(), + other => format!("{other:?}"), + }, + other => panic!("expected a query response, got {other:?}"), + } + } + + fn spawn_test_actor(dir: &tempfile::TempDir, memory_threshold: u64) -> DatabaseHandle { + spawn_actor( + "space".into(), + "atomic".into(), + dir.path().to_str().unwrap().into(), + memory_threshold, + Arc::new(DashMap::new()), + ) + } + + #[tokio::test] + async fn tc1491_failed_batch_leaves_the_local_file_unchanged() { + for memory_threshold in [u64::MAX, 0] { + for growth in [GrowthPolicy::Allow, GrowthPolicy::Forbid] { + let dir = tempfile::tempdir().unwrap(); + let actor = spawn_test_actor(&dir, memory_threshold); + run( + &actor, + exec("CREATE TABLE t (id INTEGER PRIMARY KEY, v TEXT NOT NULL)"), + GrowthPolicy::Allow, + ) + .await + .unwrap(); + run( + &actor, + exec("INSERT INTO t (v) VALUES ('keep')"), + GrowthPolicy::Allow, + ) + .await + .unwrap(); + let before = actor.checkpoint().await.unwrap(); + + // Two statements apply, the third violates NOT NULL. + let failed = run( + &actor, + batch(&[ + "UPDATE t SET v = 'changed'", + "INSERT INTO t (v) VALUES ('prefix')", + "INSERT INTO t (v) VALUES (NULL)", + ]), + growth, + ) + .await; + assert!(matches!(failed, Err(SqlError::Sqlite(_))), "{failed:?}"); + + assert_eq!(scalar(&actor, "SELECT count(*) FROM t").await, "1"); + assert_eq!(scalar(&actor, "SELECT v FROM t").await, "keep"); + assert!( + actor.checkpoint().await.unwrap() == before, + "a rejected batch must not change a single byte of the checkpoint" + ); + } + } + } + + #[tokio::test] + async fn tc1491_failed_schema_request_leaves_no_schema_behind() { + for memory_threshold in [u64::MAX, 0] { + let dir = tempfile::tempdir().unwrap(); + let actor = spawn_test_actor(&dir, memory_threshold); + run( + &actor, + exec("CREATE TABLE base (id INTEGER)"), + GrowthPolicy::Allow, + ) + .await + .unwrap(); + let failed = run( + &actor, + SqlRequest::Execute { + schema: Some(vec![ + "CREATE TABLE extra (id INTEGER)".into(), + "CREATE INDEX extra_id ON extra (id)".into(), + ]), + sql: "INSERT INTO missing_table VALUES (1)".into(), + params: vec![], + }, + GrowthPolicy::Allow, + ) + .await; + assert!(failed.is_err()); + assert_eq!( + scalar( + &actor, + "SELECT count(*) FROM sqlite_master WHERE name IN ('extra', 'extra_id')" + ) + .await, + "0" + ); + } + } + + #[tokio::test] + async fn tc1491_successful_batch_and_ddl_batch_apply_in_full() { + let dir = tempfile::tempdir().unwrap(); + let actor = spawn_test_actor(&dir, 0); + let result = run( + &actor, + batch(&[ + "CREATE TABLE a (id INTEGER PRIMARY KEY, v TEXT)", + "CREATE INDEX a_v ON a (v)", + "INSERT INTO a (v) VALUES ('x')", + "INSERT INTO a (v) VALUES ('y')", + ]), + GrowthPolicy::Allow, + ) + .await + .unwrap(); + let SqlResponse::Batch(response) = result.response else { + panic!("expected a batch response"); + }; + assert_eq!(response.results.len(), 4); + assert_eq!(response.results[3].last_insert_row_id, Some(2)); + assert_eq!(scalar(&actor, "SELECT count(*) FROM a").await, "2"); + + // A failing DDL batch undoes its DDL too. + let failed = run( + &actor, + batch(&["CREATE TABLE b (id INTEGER)", "CREATE TABLE a (id INTEGER)"]), + GrowthPolicy::Allow, + ) + .await; + assert!(failed.is_err()); + assert_eq!( + scalar( + &actor, + "SELECT count(*) FROM sqlite_master WHERE name = 'b'" + ) + .await, + "0" + ); + } + + #[tokio::test] + async fn tc1491_user_transaction_control_is_still_denied() { + let dir = tempfile::tempdir().unwrap(); + let actor = spawn_test_actor(&dir, 0); + run(&actor, exec("CREATE TABLE t (v TEXT)"), GrowthPolicy::Allow) + .await + .unwrap(); + for control in [ + "BEGIN", + "COMMIT", + "ROLLBACK", + "END", + "SAVEPOINT mine", + "RELEASE tinycloud_request", + "ROLLBACK TO tinycloud_request", + ] { + for request in [ + batch(&["INSERT INTO t VALUES ('a')", control]), + batch(&[control, "INSERT INTO t VALUES ('a')"]), + SqlRequest::Execute { + schema: Some(vec![control.into()]), + sql: "INSERT INTO t VALUES ('a')".into(), + params: vec![], + }, + ] { + let result = run(&actor, request, GrowthPolicy::Allow).await; + assert!(result.is_err(), "{control} must be refused: {result:?}"); + assert_eq!( + scalar(&actor, "SELECT count(*) FROM t").await, + "0", + "{control} must not let a prefix commit" + ); + } + } + } + + /// A request that carries a `PRAGMA` is one transaction like any other: + /// a failure leaves neither its statements nor the header value it set, + /// and the connection is still served afterwards. + #[tokio::test] + async fn tc1491_pragma_requests_are_one_transaction() { + for memory_threshold in [u64::MAX, 0] { + let dir = tempfile::tempdir().unwrap(); + let actor = spawn_test_actor(&dir, memory_threshold); + run( + &actor, + exec("CREATE TABLE t (v TEXT NOT NULL)"), + GrowthPolicy::Allow, + ) + .await + .unwrap(); + let before = actor.checkpoint().await.unwrap(); + + // `user_version` is on the admin PRAGMA allow-list (TC-1586); + // `foreign_keys = OFF` is not. + for failing in [ + batch(&[ + "PRAGMA user_version = 7", + "INSERT INTO t VALUES ('prefix')", + "INSERT INTO t VALUES (NULL)", + ]), + batch(&[ + "INSERT INTO t VALUES ('prefix')", + "PRAGMA application_id = 9", + "INSERT INTO t VALUES (NULL)", + ]), + SqlRequest::Execute { + schema: Some(vec![ + "PRAGMA user_version = 7".into(), + "INSERT INTO t VALUES ('prefix')".into(), + ]), + sql: "INSERT INTO t VALUES (NULL)".into(), + params: vec![], + }, + ] { + let failed = run(&actor, failing, GrowthPolicy::Allow).await; + assert!(failed.is_err()); + assert_eq!(scalar(&actor, "SELECT count(*) FROM t").await, "0"); + assert_eq!(scalar(&actor, "PRAGMA user_version").await, "0"); + assert_eq!(scalar(&actor, "PRAGMA application_id").await, "0"); + assert!( + actor.checkpoint().await.unwrap() == before, + "a rejected request with a PRAGMA must not change a single byte" + ); + } + + // The reports an admin may run, in a request that writes: they + // return normally inside the transaction and the write is kept. + run( + &actor, + SqlRequest::Execute { + schema: Some( + [ + "PRAGMA journal_mode", + "PRAGMA foreign_keys", + "PRAGMA page_count", + "PRAGMA schema_version", + "PRAGMA freelist_count", + "PRAGMA data_version", + "PRAGMA table_info(t)", + "PRAGMA integrity_check", + "PRAGMA quick_check", + "PRAGMA foreign_key_check", + "PRAGMA user_version = 3", + ] + .map(String::from) + .to_vec(), + ), + sql: "INSERT INTO t VALUES ('kept')".into(), + params: vec![], + }, + GrowthPolicy::Allow, + ) + .await + .unwrap(); + assert_eq!(scalar(&actor, "SELECT count(*) FROM t").await, "1"); + assert_eq!(scalar(&actor, "PRAGMA user_version").await, "3"); + // A single PRAGMA request runs, and the connection keeps the + // state it opened with (the TC-1586 reset ran after the request). + assert_eq!(scalar(&actor, "PRAGMA foreign_keys").await, "1"); + } + } + + /// A write that stops with `OR FAIL` or `ON CONFLICT FAIL` keeps the rows + /// it changed first when it runs on its own; the request is one + /// transaction, so it keeps none of them, in memory and on disk. + #[test] + fn tc1491_a_single_write_with_fail_conflict_resolution_leaves_nothing() { + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("fail.db"); + for mode in [StorageMode::InMemory, StorageMode::File(file.clone())] { + let conn = + storage::open_connection(&mode, crate::sql::durability::Synchronous::Full).unwrap(); + // `ON CONFLICT FAIL` on a column is outside what the SQL parser + // accepts, so the schema is created on the connection directly. + conn.execute_batch( + "CREATE TABLE u (id INTEGER PRIMARY KEY, v TEXT UNIQUE ON CONFLICT FAIL); + CREATE TABLE n (id INTEGER PRIMARY KEY, v TEXT NOT NULL ON CONFLICT FAIL); + INSERT INTO u VALUES (10, 'x'), (11, 'y'), (12, 'z');", + ) + .unwrap(); + let bytes = || -> Vec { + match &mode { + StorageMode::File(path) => { + conn.query_row("PRAGMA wal_checkpoint(TRUNCATE)", [], |_| Ok(())) + .unwrap(); + std::fs::read(path).unwrap() + } + // In memory there is no file; the rows are the check. + StorageMode::InMemory => Vec::new(), + } + }; + let before = bytes(); + for sql in [ + "INSERT OR FAIL INTO u VALUES (1, 'a'), (2, 'b'), (1, 'dup')", + "UPDATE u SET v = 'same'", + "UPDATE OR FAIL u SET v = 'same'", + "INSERT INTO n VALUES (1, 'a'), (2, 'b'), (3, NULL)", + ] { + for request in [ + exec(sql), + SqlRequest::Query { + sql: sql.into(), + params: vec![], + max_rows: None, + max_bytes: None, + }, + batch(&[sql]), + ] { + let failed = handle_message(&conn, &request, &None, "tinycloud.sql/write"); + // `UPDATE OR FAIL` is outside what the SQL parser accepts: + // refused, and nothing changes either. + assert!(failed.is_err(), "{sql}: {failed:?}"); + let rows = |sql: &str| -> String { + conn.query_row(sql, [], |row| row.get(0)).unwrap() + }; + assert_eq!( + rows("SELECT count(*) || ':' || group_concat(v) FROM u"), + "3:x,y,z" + ); + assert_eq!(rows("SELECT count(*) || '' FROM n"), "0"); + assert!(conn.is_autocommit()); + assert!(bytes() == before, "{sql}: the file must not change"); + } + } + } + } + + /// A connection still inside a transaction after its request is retired: + /// the next request would otherwise run inside it. + #[tokio::test] + async fn tc1491_a_connection_left_in_a_transaction_retires_the_actor() { + let dir = tempfile::tempdir().unwrap(); + let key = ("space".to_string(), "leaks".to_string()); + for growth in [GrowthPolicy::Allow, GrowthPolicy::Forbid] { + let databases = Arc::new(DashMap::new()); + let actor = spawn_actor( + key.0.clone(), + key.1.clone(), + dir.path().to_str().unwrap().into(), + u64::MAX, + databases.clone(), + ); + databases.insert(key.clone(), actor.clone()); + run(&actor, exec("CREATE TABLE t (v TEXT)"), GrowthPolicy::Allow) + .await + .unwrap(); + + OPEN_TRANSACTIONS.lock().unwrap().push(key.clone()); + let result = run(&actor, exec("INSERT INTO t VALUES ('x')"), growth).await; + OPEN_TRANSACTIONS.lock().unwrap().retain(|k| *k != key); + assert!(matches!(result, Err(SqlError::Internal(_))), "{result:?}"); + assert!( + !databases.contains_key(&key), + "the retired actor must leave the registry" + ); + assert!( + run(&actor, exec("INSERT INTO t VALUES ('y')"), growth) + .await + .is_err(), + "a retired actor serves no further request" + ); + } + } } /// TC-1581: schema-changing statements need `schema`; `write` is data only. @@ -1451,7 +2456,23 @@ mod ddl_authority_tests { caveats: &Option, authority: &SqlAuthority, ) -> Result { - handle_message_authorized(conn, request, caveats, authority, &PragmaTracker::default()) + let policy = DurabilityPolicy::default(); + let ctx = WriteContext { + policy: &policy, + key: "test/test", + dir: Path::new("."), + file_backed: false, + remaining: None, + file: Path::new(""), + }; + handle_message_ctx( + conn, + request, + caveats, + authority, + &PragmaTracker::default(), + &ctx, + ) } fn table_exists(conn: &rusqlite::Connection, name: &str) -> bool { @@ -1931,7 +2952,8 @@ mod ddl_authority_tests { StorageMode::InMemory, StorageMode::File(dir.path().join("reset.db")), ] { - let conn = storage::open_connection(&mode).unwrap(); + let conn = + storage::open_connection(&mode, crate::sql::durability::Synchronous::Full).unwrap(); conn.execute_batch("CREATE TABLE t (a)").unwrap(); let read = |pragma: &str| -> String { conn.query_row(&format!("PRAGMA {pragma}"), [], |row| { diff --git a/tinycloud-core/src/sql/durability.rs b/tinycloud-core/src/sql/durability.rs new file mode 100644 index 00000000..86ddce1c --- /dev/null +++ b/tinycloud-core/src/sql/durability.rs @@ -0,0 +1,406 @@ +//! Local commit durability and the reservation made before a write commits +//! (TC-1491, slice 1a). The contract, the invariants and what is and is not +//! guaranteed are in `docs/sql-local-durability.md`. + +use std::collections::HashMap; +use std::path::Path; +use std::sync::{Arc, Mutex}; + +use rusqlite::Connection; + +use super::types::SqlError; + +/// `PRAGMA synchronous` for file-backed databases. +#[derive( + Debug, Clone, Copy, PartialEq, Eq, Hash, Default, serde::Serialize, serde::Deserialize, +)] +#[serde(rename_all = "lowercase")] +pub enum Synchronous { + /// WAL fsync on every COMMIT: a commit that returned survives power loss + /// (given a volume that honours flushes). + #[default] + Full, + /// WAL fsync only at checkpoints: a power loss can drop the most recent + /// commits. The rollback flag for `Full`. + Normal, +} + +impl Synchronous { + pub fn pragma_value(self) -> &'static str { + match self { + Synchronous::Full => "FULL", + Synchronous::Normal => "NORMAL", + } + } +} + +/// Free space on the filesystem holding the SQL cache. A seam so tests can +/// exhaust it without filling a disk. +pub trait FreeSpace: Send + Sync { + fn free_bytes(&self, path: &Path) -> std::io::Result; +} + +/// `statvfs`: the bytes an unprivileged process can still allocate. +#[derive(Debug, Default)] +pub struct StatvfsFreeSpace; + +impl FreeSpace for StatvfsFreeSpace { + fn free_bytes(&self, path: &Path) -> std::io::Result { + use std::os::unix::ffi::OsStrExt; + let c_path = std::ffi::CString::new(path.as_os_str().as_bytes()) + .map_err(|_| std::io::Error::from(std::io::ErrorKind::InvalidInput))?; + let mut stat = std::mem::MaybeUninit::::uninit(); + // SAFETY: `c_path` is a valid NUL-terminated string and `stat` is a + // valid out-pointer; it is only read after statvfs reports success. + let rc = unsafe { libc::statvfs(c_path.as_ptr(), stat.as_mut_ptr()) }; + if rc != 0 { + return Err(std::io::Error::last_os_error()); + } + // SAFETY: statvfs succeeded, so it initialised the struct. + let stat = unsafe { stat.assume_init() }; + // The field widths differ by platform (u32 on macOS). + #[allow(clippy::unnecessary_cast)] + let free = (stat.f_bavail as u64).saturating_mul(stat.f_frsize as u64); + Ok(free) + } +} + +/// Why a reservation was refused. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ReservationRefused { + /// Free space minus every in-flight reservation and the reservation asked + /// for would fall below the floor. + Headroom, + /// Free space could not be read; refusing is the conservative answer. + Unknown, +} + +/// What one database holds of the [`DiskBudget`]. +#[derive(Debug, Default, Clone, Copy)] +struct Held { + /// The WAL bytes of the write in flight and the checkpoint growth it + /// causes; released when the request's outcome is known. + inflight: u64, + /// The growth of the main file the next checkpoint will cause: pages + /// committed since the last one, which are in the WAL until then. Held + /// until a checkpoint runs or the database's connection closes, because + /// the disk does not have those bytes until then and nothing else pays for + /// them (the save to Postgres ships the WAL, not a checkpoint, while the + /// WAL is small). + pending: u64, +} + +impl Held { + fn total(self) -> u64 { + self.inflight.saturating_add(self.pending) + } +} + +/// The disk bytes in-flight writes of every database on this node still need. +/// One per node, shared by all actors, so two databases cannot each take the +/// same free bytes. +#[derive(Debug, Default)] +pub struct DiskBudget { + held: Mutex>, +} + +impl DiskBudget { + /// Set `key`'s reservation to `inflight` bytes for the write in flight and + /// `pending` bytes of checkpoint growth, if the disk can still hold both + /// next to everyone else's. Leaves the old reservation when refusing. + pub fn reserve( + &self, + key: &str, + inflight: u64, + pending: u64, + free_space: &dyn FreeSpace, + dir: &Path, + min_free_bytes: u64, + ) -> Result<(), ReservationRefused> { + let mut held = self.held.lock().unwrap_or_else(|e| e.into_inner()); + let others: u64 = held + .iter() + .filter(|(other, _)| other.as_str() != key) + .map(|(_, held)| held.total()) + .fold(0, u64::saturating_add); + let free = free_space + .free_bytes(dir) + .map_err(|_| ReservationRefused::Unknown)?; + let needed = others + .saturating_add(inflight) + .saturating_add(pending) + .saturating_add(min_free_bytes); + if free < needed { + return Err(ReservationRefused::Headroom); + } + held.insert(key.to_string(), Held { inflight, pending }); + Ok(()) + } + + /// The write is committed: its WAL bytes are on disk. Keep `pending` bytes + /// of checkpoint growth held, without a free-space check. + pub fn hold(&self, key: &str, pending: u64) { + let mut held = self.held.lock().unwrap_or_else(|e| e.into_inner()); + if pending == 0 { + held.remove(key); + } else { + held.insert( + key.to_string(), + Held { + inflight: 0, + pending, + }, + ); + } + } + + /// The write in flight is over (refused, failed or saved): drop what it + /// reserved. The checkpoint growth of commits that stay is still held. + pub fn release(&self, key: &str) { + let mut held = self.held.lock().unwrap_or_else(|e| e.into_inner()); + if let Some(entry) = held.get_mut(key) { + entry.inflight = 0; + if entry.pending == 0 { + held.remove(key); + } + } + } + + /// Drop everything held for `key`: a checkpoint has run, or the + /// database's connection is gone, and what is on disk is on disk. + pub fn forget(&self, key: &str) { + self.held + .lock() + .unwrap_or_else(|e| e.into_inner()) + .remove(key); + } + + /// Bytes reserved by every database but `key`. + pub fn reserved_excluding(&self, key: &str) -> u64 { + self.held + .lock() + .unwrap_or_else(|e| e.into_inner()) + .iter() + .filter(|(other, _)| other.as_str() != key) + .map(|(_, held)| held.total()) + .fold(0, u64::saturating_add) + } + + /// Bytes currently reserved by all databases. + pub fn reserved(&self) -> u64 { + self.held + .lock() + .unwrap_or_else(|e| e.into_inner()) + .values() + .fold(0, |sum, held| sum.saturating_add(held.total())) + } +} + +/// Releases a database's in-flight reservation when the request's outcome is +/// known. +pub struct ReservationGuard { + budget: Arc, + key: String, +} + +impl ReservationGuard { + pub fn new(budget: Arc, key: String) -> Self { + Self { budget, key } + } +} + +impl Drop for ReservationGuard { + fn drop(&mut self) { + self.budget.release(&self.key); + } +} + +/// Forgets everything a database holds of the [`DiskBudget`] when its actor +/// ends, however it ends. +pub struct HeldGuard { + budget: Arc, + key: String, +} + +impl HeldGuard { + pub fn new(budget: Arc, key: String) -> Self { + Self { budget, key } + } +} + +impl Drop for HeldGuard { + fn drop(&mut self) { + self.budget.forget(&self.key); + } +} + +/// Whether the main database has a write transaction open on a connection, +/// asked of SQLite itself (`sqlite3_txn_state`). A request that never took one +/// wrote nothing: it needs no room on the disk and nothing to reserve. It is +/// set by the first statement that writes (an `UPDATE` that matches no row +/// included) and stays until the transaction ends. +pub struct WriteProbe(*mut rusqlite::ffi::sqlite3); + +// SAFETY: a probe is only used while the connection it was made from is alive +// and on the thread that runs the request (SQLite calls the progress handler on +// the thread running the statement), and `sqlite3_txn_state` only reads the +// connection's own state. +unsafe impl Send for WriteProbe {} + +impl WriteProbe { + pub fn of(conn: &Connection) -> Self { + // SAFETY: the pointer is only dereferenced by SQLite, through + // `writing`, while `conn` is alive (see the `Send` note above). + Self(unsafe { conn.handle() }) + } + + pub fn writing(&self) -> bool { + // SAFETY: see `WriteProbe::of`; "main" is a NUL-terminated literal. + let state = unsafe { rusqlite::ffi::sqlite3_txn_state(self.0, c"main".as_ptr()) }; + state == rusqlite::ffi::SQLITE_TXN_WRITE + } +} + +/// Test seams at the commit boundary. A seam only observes: the kill -9 +/// tests abort the process from one. +#[derive(Clone, Default)] +pub struct CommitSeams { + /// After every check passed, immediately before COMMIT. + pub before_commit: Option>, + /// After COMMIT returned (and, for `Full`, its fsync), before the actor + /// answers. + pub after_commit: Option>, + /// Just before the actor sends its response. + pub before_response: Option>, +} + +impl std::fmt::Debug for CommitSeams { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("CommitSeams").finish_non_exhaustive() + } +} + +/// 0 turns the per-database cap off. Off by default: the limit is a product +/// decision (card `sql-storage-report-read`), set by config once decided. +pub const DEFAULT_MAX_DATABASE_BYTES: u64 = 0; +/// 0 turns the disk headroom check off (no reservation, no free-space reads). +/// Off by default for the same reason. +pub const DEFAULT_MIN_FREE_BYTES: u64 = 0; + +/// Everything the actors need to make a commit durable and to refuse a write +/// that would not fit. +#[derive(Clone)] +pub struct DurabilityPolicy { + pub synchronous: Synchronous, + /// Per-database size cap, enforced by SQLite itself through + /// `max_page_count`. 0 = no cap. + pub max_database_bytes: u64, + /// Free bytes that must remain after every in-flight write's worst case. + /// 0 = the disk headroom check is off. + pub min_free_bytes: u64, + pub budget: Arc, + pub free_space: Arc, + pub seams: CommitSeams, +} + +impl Default for DurabilityPolicy { + fn default() -> Self { + Self { + synchronous: Synchronous::Full, + max_database_bytes: DEFAULT_MAX_DATABASE_BYTES, + min_free_bytes: DEFAULT_MIN_FREE_BYTES, + budget: Arc::new(DiskBudget::default()), + free_space: Arc::new(StatvfsFreeSpace), + seams: CommitSeams::default(), + } + } +} + +impl std::fmt::Debug for DurabilityPolicy { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("DurabilityPolicy") + .field("synchronous", &self.synchronous) + .field("max_database_bytes", &self.max_database_bytes) + .field("min_free_bytes", &self.min_free_bytes) + .finish_non_exhaustive() + } +} + +/// `fsync` a directory so the entries created or renamed in it survive a +/// crash. SQLite syncs the directory itself when it first creates a `-wal` +/// file (`UNIXFILE_DIRSYNC` in its unix VFS), but not for the main file. +pub fn sync_dir(dir: &Path) -> std::io::Result<()> { + std::fs::File::open(dir)?.sync_all() +} + +/// `create_dir_all`, then make every directory it created durable: each new +/// directory and the existing directory that now holds the first of them. +pub fn create_dir_all_durable(path: &Path) -> std::io::Result<()> { + let mut missing = Vec::new(); + let mut cursor = Some(path); + while let Some(dir) = cursor { + if dir.as_os_str().is_empty() || dir.exists() { + break; + } + missing.push(dir); + cursor = dir.parent(); + } + if missing.is_empty() { + return Ok(()); + } + std::fs::create_dir_all(path)?; + for dir in missing.iter().rev() { + sync_dir(dir)?; + if let Some(parent) = dir.parent().filter(|p| !p.as_os_str().is_empty()) { + sync_dir(parent)?; + } + } + Ok(()) +} + +/// Apply `synchronous` to a file-backed connection. +pub fn set_synchronous(conn: &Connection, synchronous: Synchronous) -> Result<(), SqlError> { + conn.pragma_update(None, "synchronous", synchronous.pragma_value()) + .map_err(|e| SqlError::Internal(e.to_string())) +} + +/// Current heap bytes of this connection's page cache. Every page SQLite has +/// yet to write to the WAL at COMMIT is in it. +pub fn cache_used_bytes(conn: &Connection) -> u64 { + let mut current: std::os::raw::c_int = 0; + let mut high: std::os::raw::c_int = 0; + // SAFETY: the handle is valid for the borrow of `conn`, and the two + // out-pointers are valid for the call. `sqlite3_db_status` only reads the + // connection's own counters. + let rc = unsafe { + rusqlite::ffi::sqlite3_db_status( + conn.handle(), + rusqlite::ffi::SQLITE_DBSTATUS_CACHE_USED, + &mut current, + &mut high, + 0, + ) + }; + if rc != rusqlite::ffi::SQLITE_OK { + // Unknown: the high-water mark is the largest it has been. + return u64::try_from(high.max(0)).unwrap_or(0); + } + u64::try_from(current.max(0)).unwrap_or(0) +} + +/// SQLite's per-frame WAL overhead. +pub const WAL_FRAME_HEADER_BYTES: u64 = 24; + +/// Upper bound on the bytes COMMIT still appends to the WAL: one frame per +/// page in the cache. Dirty pages are a subset of cached pages, so this +/// bounds the writes of index, trigger, foreign-key, freelist and overflow +/// amplification alike, whatever the SQL payload was. A page dirtied twice +/// in one transaction still costs one frame. +pub fn unspilled_wal_bound(cache_used: u64, page_size: u64) -> u64 { + if page_size == 0 { + return cache_used; + } + cache_used + .div_ceil(page_size) + .saturating_mul(page_size + WAL_FRAME_HEADER_BYTES) +} diff --git a/tinycloud-core/src/sql/mod.rs b/tinycloud-core/src/sql/mod.rs index 94b8c016..4e898ef6 100644 --- a/tinycloud-core/src/sql/mod.rs +++ b/tinycloud-core/src/sql/mod.rs @@ -2,6 +2,7 @@ pub mod authority; pub mod authorizer; pub mod caveats; pub mod database; +pub mod durability; pub mod parser; pub mod service; pub mod storage; diff --git a/tinycloud-core/src/sql/service.rs b/tinycloud-core/src/sql/service.rs index f569c0ae..83fac75f 100644 --- a/tinycloud-core/src/sql/service.rs +++ b/tinycloud-core/src/sql/service.rs @@ -3,7 +3,7 @@ use std::{ future::Future, path::{Path, PathBuf}, sync::{ - atomic::{AtomicU64, AtomicU8, Ordering}, + atomic::{AtomicU8, Ordering}, Arc, Weak, }, time::Duration, @@ -15,7 +15,8 @@ use tokio::{sync::OwnedMutexGuard, time::Instant}; use tracing::Instrument; use crate::database_artifacts::{ - ArtifactExpectation, DatabaseArtifact, DatabaseArtifactError, DatabaseArtifactRepository, + read_artifact, sweep_stale_staging, ArtifactExpectation, ArtifactMeta, CacheFileSink, + DatabaseArtifactError, DatabaseArtifactRepository, }; use crate::db_pool::{is_transient_error, track_writes, transient_error_class}; use crate::{request_cancel, request_telemetry}; @@ -23,7 +24,8 @@ use crate::{request_cancel, request_telemetry}; use super::{ authority::{DdlPolicy, SqlAuthority}, caveats::SqlCaveats, - database::{spawn_actor, DatabaseHandle, GrowthPolicy}, + database::{spawn_actor_with_policy, DatabaseHandle, GrowthPolicy}, + durability::{DurabilityPolicy, ReservationGuard}, types::*, }; @@ -157,6 +159,7 @@ pub struct SqlService { memory_threshold: u64, artifact_repository: Arc, bounds: SqlWaitBounds, + durability: DurabilityPolicy, ddl_policy: DdlPolicy, } @@ -175,10 +178,18 @@ impl SqlService { memory_threshold, artifact_repository, bounds: SqlWaitBounds::default(), + durability: DurabilityPolicy::default(), ddl_policy: DdlPolicy::default(), } } + /// The node's local durability policy (TC-1491): `synchronous`, the + /// per-database size cap and the disk headroom. + pub fn with_durability(mut self, durability: DurabilityPolicy) -> Self { + self.durability = durability; + self + } + /// Whether a schema-changing statement needs `tinycloud.sql/schema` /// (TC-1581). Defaults to [`DdlPolicy::Enforce`]. pub fn with_ddl_policy(mut self, policy: DdlPolicy) -> Self { @@ -275,6 +286,52 @@ impl SqlService { .await } + /// Execute with the owner's remaining storage budget: a write that would + /// grow the database by more than `remaining_bytes` is rolled back before + /// COMMIT with [`SqlError::StorageWouldGrow`]. + pub async fn execute_with_budget( + &self, + space: &SpaceId, + db_name: &str, + request: SqlRequest, + caveats: Option, + ability: String, + remaining_bytes: u64, + ) -> Result { + self.execute_with_budget_authorized( + space, + db_name, + request, + caveats, + SqlAuthority::from(ability), + remaining_bytes, + ) + .await + } + + /// As [`Self::execute_with_budget`], judging each statement against every + /// SQL ability the invocation holds (TC-1581). This is the entry point + /// `/invoke` uses for a write inside the owner's remaining budget. + pub async fn execute_with_budget_authorized( + &self, + space: &SpaceId, + db_name: &str, + request: SqlRequest, + caveats: Option, + authority: SqlAuthority, + remaining_bytes: u64, + ) -> Result { + self.execute_with_growth( + space, + db_name, + request, + caveats, + authority.with_ddl_policy(self.ddl_policy), + GrowthPolicy::Budget(remaining_bytes), + ) + .await + } + async fn execute_with_growth( &self, space: &SpaceId, @@ -306,6 +363,12 @@ impl SqlService { growth: GrowthPolicy, ) -> Result { let key = (space.to_string(), db_name.to_string()); + // Whatever the request still holds of the disk budget is released once + // its outcome is known: after the durable save, or its failure. + let _reservation = ReservationGuard::new( + self.durability.budget.clone(), + format!("{}/{}", key.0, key.1), + ); let mut handle = self.handle(space, db_name).await?; begin_write()?; @@ -327,8 +390,11 @@ impl SqlService { let result = match result { Ok(result) => result, Err(error) => { - if growth == GrowthPolicy::Forbid - && self.expectation(&key) == ArtifactExpectation::Absent + if (growth == GrowthPolicy::Forbid + && self.expectation(&key) == ArtifactExpectation::Absent) + // The disk failed the commit: its WAL is not to be trusted + // (Postgres never saw the write), so start over from it. + || matches!(error, SqlError::CommitFailed) { self.discard_local_state(&key).await?; } @@ -521,7 +587,7 @@ impl SqlService { || SqlBusyStage::ArtifactTransfer, self.bounds.artifact_transfer, self.artifact_repository - .load("sql", &space.to_string(), db_name), + .load_meta("sql", &space.to_string(), db_name), ) .await? .map_err(artifact_error_to_sql)? @@ -680,18 +746,34 @@ impl SqlService { (HydrationFlight { phase, outcome }, start) } - /// Load the durable artifact, seed the cache from it and spawn the actor. - /// Only the load is bounded (`hydration_max`): the cache writes after it - /// are local and must finish once begun. + /// Stream the durable artifact into the cache, seed it and spawn the actor. + /// Only the streaming read is bounded (`hydration_max`): it stages temp + /// files that a timeout simply drops, while the commit after it is local + /// and must finish once begun. async fn hydrate(&self, space: &SpaceId, db_name: &str, phase: &AtomicU8) -> HydrationOutcome { + let cache_path = self.cache_path(space, db_name); + sweep_stale_staging(&[&cache_path, &sql_wal_path(&cache_path)]).await; + let mut sink = CacheFileSink::new( + "sql", + format!("{space}/{db_name}"), + cache_path.clone(), + sql_wal_path(&cache_path), + is_sqlite_database, + is_sqlite_wal, + ); let loaded = tokio::time::timeout( self.bounds.hydration_max, - self.artifact_repository - .load("sql", &space.to_string(), db_name), + read_artifact( + self.artifact_repository.as_ref(), + "sql", + &space.to_string(), + db_name, + &mut sink, + ), ) .await; - let artifact = match loaded { - Ok(Ok(artifact)) => artifact, + let meta = match loaded { + Ok(Ok(meta)) => meta, Ok(Err(error)) => return hydration_failure(artifact_error_to_sql(error)), Err(_) => { tracing::warn!( @@ -704,70 +786,57 @@ impl SqlService { } }; phase.store(PHASE_WRITING, Ordering::Release); - if let Err(error) = self.seed_cache(space, db_name, artifact).await { + if let Err(error) = self.seed_cache(space, db_name, meta, sink).await { return hydration_failure(error); } self.databases .entry((space.to_string(), db_name.to_string())) .or_insert_with(|| { - spawn_actor( + spawn_actor_with_policy( space.to_string(), db_name.to_string(), self.base_path.clone(), self.memory_threshold, self.databases.clone(), + self.durability.clone(), ) }); HydrationOutcome::Ready } + /// Replace the cache files with the verified artifact staged in `sink` + /// (or with nothing, when there is no artifact). async fn seed_cache( &self, space: &SpaceId, db_name: &str, - artifact: Option, + meta: Option, + mut sink: CacheFileSink, ) -> Result<(), SqlError> { let cache_path = self.cache_path(space, db_name); let key = (space.to_string(), db_name.to_string()); - match artifact { - Some(artifact) => { + match meta { + Some(meta) => { tracing::info!( service = "sql", - revision = artifact.revision, - storage_mode = %artifact.storage_mode, - bytes = artifact.payload.len(), - delta_bytes = artifact.delta_size_bytes, - logical_bytes = artifact.size_bytes, - content_hash = %artifact.content_hash, - checkpoint_content_hash = %artifact.checkpoint_content_hash, + revision = meta.revision, + storage_mode = %meta.storage_mode, + bytes = meta.checkpoint_size_bytes, + delta_bytes = meta.delta_size_bytes, + logical_bytes = meta.size_bytes, + content_hash = %meta.content_hash, + checkpoint_content_hash = %meta.checkpoint_content_hash, "Loaded database artifact" ); - // Refuse to seed the cache with bytes SQLite would misread. A - // checkpoint that is not a database opens as a fresh empty one, - // and a WAL that is not a WAL is ignored outright — both revert - // the database silently, and the next write makes the reverted - // state durable. - validate_payload( - space, - db_name, - "checkpoint", - &artifact.payload, - is_sqlite_database, - )?; - if let Some(delta) = artifact.delta_payload.as_deref() { - validate_payload(space, db_name, "wal", delta, is_sqlite_wal)?; - } - remove_sql_cache_files(&cache_path).await?; - write_cache_file(&cache_path, &artifact.payload).await?; - if let Some(delta) = artifact.delta_payload { - write_cache_file(&sql_wal_path(&cache_path), &delta).await?; - } + sink.commit() + .await + .map_err(|e| SqlError::Internal(e.to_string()))?; self.lineage.insert( key, ArtifactExpectation::Derived { - revision: artifact.revision, - checkpoint_content_hash: artifact.checkpoint_content_hash, + revision: meta.revision, + checkpoint_content_hash: meta.checkpoint_content_hash, }, ); Ok(()) @@ -835,7 +904,7 @@ impl SqlService { // adding a database page. Full spaces replace checkpoint + WAL, but // must also bound the resulting image against the durable charge: // live state can include partial writes from an earlier failed request. - let wal = if growth == GrowthPolicy::Allow { + let wal = if growth != GrowthPolicy::Forbid { handle.wal().await? } else { None @@ -885,7 +954,7 @@ impl SqlService { .bounded( || SqlBusyStage::ArtifactTransfer, self.bounds.artifact_transfer, - self.artifact_repository.load("sql", &key.0, db_name), + self.artifact_repository.load_meta("sql", &key.0, db_name), ) .await? .map_err(artifact_error_to_sql)? @@ -939,66 +1008,6 @@ fn is_sqlite_wal(payload: &[u8]) -> bool { .any(|magic| payload.starts_with(magic)) } -/// Reject a hydration payload whose leading bytes are not the format the file -/// it is about to become is read as. -fn validate_payload( - space: &SpaceId, - db_name: &str, - role: &'static str, - payload: &[u8], - is_valid: impl Fn(&[u8]) -> bool, -) -> Result<(), SqlError> { - if is_valid(payload) { - return Ok(()); - } - tracing::error!( - service = "sql", - role, - bytes = payload.len(), - "Durable database artifact does not carry the expected file header; refusing to hydrate" - ); - Err(SqlError::Internal(format!( - "database artifact {role} for {space}/{db_name} does not carry the expected file header" - ))) -} - -/// A temp sibling of `path`, unique per call. -/// -/// The suffix is APPENDED to the whole path. `Path::with_extension` replaces -/// everything after the last `.`, so `main.db` and `main.db-wal` both mapped to -/// `main.db.tmp`: the checkpoint and WAL writes of one hydration raced on a -/// single file and cross-consumed each other's bytes, leaving either a WAL that -/// SQLite ignores (silent revert to the checkpoint) or a "database" with no -/// SQLite header (silent revert to empty). -fn temp_write_path(path: &Path) -> PathBuf { - static NEXT_TEMP_ID: AtomicU64 = AtomicU64::new(0); - PathBuf::from(format!( - "{}.tmp.{}.{}", - path.display(), - std::process::id(), - NEXT_TEMP_ID.fetch_add(1, Ordering::Relaxed) - )) -} - -async fn write_cache_file(path: &Path, payload: &[u8]) -> Result<(), SqlError> { - if let Some(parent) = path.parent() { - tokio::fs::create_dir_all(parent) - .await - .map_err(|e| SqlError::Internal(e.to_string()))?; - } - - let temp_path = temp_write_path(path); - if let Err(e) = tokio::fs::write(&temp_path, payload).await { - let _ = tokio::fs::remove_file(&temp_path).await; - return Err(SqlError::Internal(e.to_string())); - } - if let Err(e) = tokio::fs::rename(&temp_path, path).await { - let _ = tokio::fs::remove_file(&temp_path).await; - return Err(SqlError::Internal(e.to_string())); - } - Ok(()) -} - async fn remove_sql_cache_files(path: &Path) -> Result<(), SqlError> { for candidate in [ path.to_path_buf(), @@ -1028,6 +1037,16 @@ fn artifact_error_to_sql(err: DatabaseArtifactError) -> SqlError { ); SqlError::Unavailable } + // The row kept changing across every restart. Nothing was written, so + // a fresh request is safe: the same answer as any contended store. + DatabaseArtifactError::ArtifactChanged => { + tracing::warn!( + service = "sql", + error_class = "artifact_changed", + "Database artifact kept changing while it was read" + ); + SqlError::Unavailable + } err => SqlError::Internal(err.to_string()), } } @@ -1105,14 +1124,19 @@ fn log_busy(stage: SqlBusyStage, bound: Duration, waited: Duration) { #[cfg(test)] mod tests { use super::*; + use crate::database_artifacts::staging_path as temp_write_path; use crate::{ - database_artifacts::{DatabaseArtifact, DeltaSave, SeaOrmDatabaseArtifactRepository}, + database_artifacts::{ + test_support::ReadAll, ArtifactMeta, ArtifactSink, DatabaseArtifact, DeltaSave, + SeaOrmDatabaseArtifactRepository, + }, migrations::Migrator, sea_orm::{ConnectOptions, Database, DbErr}, sea_orm_migration::MigratorTrait, sql_sizes::{SizeTrackingArtifactRepository, SqlSizes}, }; use async_trait::async_trait; + use std::sync::atomic::AtomicU64; use tempfile::TempDir; use tinycloud_auth::{ resolver::DID_METHODS, @@ -1197,7 +1221,7 @@ mod tests { )); assert_eq!(sizes.space_total(&space).await, 0); assert!(repo - .load("sql", &space.to_string(), name) + .read_all("sql", &space.to_string(), name) .await .unwrap() .is_none()); @@ -1245,7 +1269,7 @@ mod tests { )); assert_eq!(sizes.space_total(&space).await, 0); assert!(repo - .load("sql", &space.to_string(), "main") + .read_all("sql", &space.to_string(), "main") .await .unwrap() .is_none()); @@ -1284,7 +1308,7 @@ mod tests { .expect("dropping an absent table is a successful no-op"); assert_eq!(sizes.space_total(&space).await, 0); assert!(repo - .load("sql", &space.to_string(), name) + .read_all("sql", &space.to_string(), name) .await .unwrap() .is_none()); @@ -1326,29 +1350,29 @@ mod tests { .await .unwrap(); let before = repo - .load("sql", &space.to_string(), "main") + .read_all("sql", &space.to_string(), "main") .await .unwrap() .unwrap(); - // Unguarded requests preserve partial-batch semantics. The actor can - // therefore contain more pages than its last durable checkpoint. - let failed = service + // A failing request no longer leaves a committed prefix (TC-1491), + // so put the actor ahead of its last durable checkpoint directly: + // the durable-size comparison must still hold against live state + // that was never persisted. + let actor = service.handle(&space, "main").await.unwrap(); + actor .execute( - &space, - "main", SqlRequest::Execute { - schema: Some(vec![ - "INSERT INTO data VALUES (2, hex(zeroblob(65536)))".into() - ]), - sql: "INSERT INTO missing_table VALUES (1)".into(), + schema: None, + sql: "INSERT INTO data VALUES (2, hex(zeroblob(65536)))".into(), params: vec![], }, None, "tinycloud.sql/admin".into(), + GrowthPolicy::Allow, ) - .await; - assert!(matches!(failed, Err(SqlError::Sqlite(_)))); + .await + .unwrap(); let rejected = service .execute_without_growth( &space, @@ -1367,7 +1391,7 @@ mod tests { "{rejected:?}" ); let after = repo - .load("sql", &space.to_string(), "main") + .read_all("sql", &space.to_string(), "main") .await .unwrap() .unwrap(); @@ -2087,7 +2111,7 @@ mod tests { .unwrap(); } let mut charged_bytes = repo - .load("sql", &space.to_string(), "main") + .read_all("sql", &space.to_string(), "main") .await .unwrap() .unwrap() @@ -2103,7 +2127,7 @@ mod tests { .await .unwrap_or_else(|e| panic!("{name}: non-growing write must pass: {e}")); let artifact = repo - .load("sql", &space.to_string(), "main") + .read_all("sql", &space.to_string(), "main") .await .unwrap() .unwrap(); @@ -2188,7 +2212,7 @@ mod tests { .await .unwrap_or_else(|e| panic!("{name}: DROP must pass: {e}")); let artifact = repo - .load("sql", &space.to_string(), "main") + .read_all("sql", &space.to_string(), "main") .await .unwrap() .unwrap(); @@ -2252,7 +2276,7 @@ mod tests { .await .unwrap(); let charged_bytes = repo - .load("sql", &space.to_string(), "main") + .read_all("sql", &space.to_string(), "main") .await .unwrap() .unwrap() @@ -2303,7 +2327,7 @@ mod tests { .await .unwrap(); let artifact = repo - .load("sql", &space.to_string(), "main") + .read_all("sql", &space.to_string(), "main") .await .unwrap() .unwrap(); @@ -2528,7 +2552,7 @@ mod tests { .unwrap(); let artifact = repo - .load("sql", &space.to_string(), "main") + .read_all("sql", &space.to_string(), "main") .await .unwrap() .unwrap(); @@ -2548,7 +2572,7 @@ mod tests { // still has to contain both acknowledged writes. service.export(&space, "main").await.unwrap(); let after_export = repo - .load("sql", &space.to_string(), "main") + .read_all("sql", &space.to_string(), "main") .await .unwrap() .unwrap(); @@ -2710,14 +2734,27 @@ mod tests { #[async_trait] impl DatabaseArtifactRepository for CountingArtifactRepository { - async fn load( + async fn load_meta( &self, service: &str, space: &str, name: &str, - ) -> Result, DatabaseArtifactError> { + ) -> Result, DatabaseArtifactError> { self.loads.fetch_add(1, Ordering::Relaxed); - self.inner.load(service, space, name).await + self.inner.load_meta(service, space, name).await + } + + async fn read_chunks( + &self, + service: &str, + space: &str, + name: &str, + meta: &ArtifactMeta, + sink: &mut dyn ArtifactSink, + ) -> Result<(), DatabaseArtifactError> { + self.inner + .read_chunks(service, space, name, meta, sink) + .await } async fn save( @@ -2860,13 +2897,26 @@ mod tests { #[async_trait] impl DatabaseArtifactRepository for LostReplyArtifactRepository { - async fn load( + async fn load_meta( + &self, + service: &str, + space: &str, + name: &str, + ) -> Result, DatabaseArtifactError> { + self.inner.load_meta(service, space, name).await + } + + async fn read_chunks( &self, service: &str, space: &str, name: &str, - ) -> Result, DatabaseArtifactError> { - self.inner.load(service, space, name).await + meta: &ArtifactMeta, + sink: &mut dyn ArtifactSink, + ) -> Result<(), DatabaseArtifactError> { + self.inner + .read_chunks(service, space, name, meta, sink) + .await } async fn save( @@ -2962,14 +3012,27 @@ mod tests { #[async_trait] impl DatabaseArtifactRepository for FlakyArtifactRepository { - async fn load( + async fn load_meta( &self, service: &str, space: &str, name: &str, - ) -> Result, DatabaseArtifactError> { + ) -> Result, DatabaseArtifactError> { self.fail()?; - self.inner.load(service, space, name).await + self.inner.load_meta(service, space, name).await + } + + async fn read_chunks( + &self, + service: &str, + space: &str, + name: &str, + meta: &ArtifactMeta, + sink: &mut dyn ArtifactSink, + ) -> Result<(), DatabaseArtifactError> { + self.inner + .read_chunks(service, space, name, meta, sink) + .await } async fn save( @@ -3117,15 +3180,26 @@ mod tests { #[async_trait] impl DatabaseArtifactRepository for FailingArtifactRepository { - async fn load( + async fn load_meta( &self, _service: &str, _space: &str, _name: &str, - ) -> Result, DatabaseArtifactError> { + ) -> Result, DatabaseArtifactError> { Ok(None) } + async fn read_chunks( + &self, + _service: &str, + _space: &str, + _name: &str, + _meta: &ArtifactMeta, + _sink: &mut dyn ArtifactSink, + ) -> Result<(), DatabaseArtifactError> { + Ok(()) + } + async fn save( &self, _service: &str, @@ -3298,12 +3372,12 @@ mod tests { #[async_trait] impl DatabaseArtifactRepository for GatedArtifactRepository { - async fn load( + async fn load_meta( &self, service: &str, space: &str, name: &str, - ) -> Result, DatabaseArtifactError> { + ) -> Result, DatabaseArtifactError> { self.loads.fetch_add(1, Ordering::SeqCst); Self::pass(&self.load_open).await; if self.fail_loads.load(Ordering::SeqCst) { @@ -3311,7 +3385,20 @@ mod tests { sea_orm::error::ConnAcquireErr::Timeout, ))); } - self.inner.load(service, space, name).await + self.inner.load_meta(service, space, name).await + } + + async fn read_chunks( + &self, + service: &str, + space: &str, + name: &str, + meta: &ArtifactMeta, + sink: &mut dyn ArtifactSink, + ) -> Result<(), DatabaseArtifactError> { + self.inner + .read_chunks(service, space, name, meta, sink) + .await } async fn save( @@ -3402,8 +3489,16 @@ mod tests { async fn tc1274_a_held_operation_lock_answers_sql_busy_lock_wait() { let (repo, space) = seeded("sql-busy-lock").await; let cache = TempDir::new().unwrap(); - let service = SqlService::new(cache.path().to_string_lossy().into_owned(), u64::MAX, repo) - .with_wait_bounds(wait_bounds(200)); + // Warm both databases under the default bounds first: the hydration + // (disk, fsync) is not what this test asserts, only the lock wait, + // which must not depend on how fast the disk is. + let service = SqlService::new(cache.path().to_string_lossy().into_owned(), u64::MAX, repo); + read(&service, &space).await.unwrap(); + service + .execute(&space, "b", insert("w"), None, "tinycloud.sql/admin".into()) + .await + .unwrap(); + let service = service.with_wait_bounds(wait_bounds(200)); let held = service.operation_lock(&space, "a").await.lock_owned().await; let err = tokio::time::timeout(PATIENCE, read(&service, &space)) @@ -3425,6 +3520,25 @@ mod tests { assert_eq!(rows(read(&service, &space).await.unwrap()), 1); } + #[tokio::test] + async fn a_hydration_sweeps_temp_files_a_crashed_one_left_behind() { + let (repo, space) = seeded("sql-sweep-orphans").await; + let cache = TempDir::new().unwrap(); + let dir = cache.path().join(space.to_string()); + std::fs::create_dir_all(&dir).unwrap(); + // pid 1 never matches this process's live files: a crashed hydration's. + let orphan = dir.join("a.db.tmp.1.0"); + let orphan_wal = dir.join("a.db-wal.tmp.1.1"); + let unrelated = dir.join("b.db.tmp.1.0"); + for file in [&orphan, &orphan_wal, &unrelated] { + std::fs::write(file, b"x").unwrap(); + } + let service = SqlService::new(cache.path().to_string_lossy().into_owned(), u64::MAX, repo); + assert_eq!(rows(read(&service, &space).await.unwrap()), 1); + assert!(!orphan.exists() && !orphan_wal.exists()); + assert!(unrelated.exists(), "another database's file is not ours"); + } + #[tokio::test] async fn tc1274_a_stalled_hydration_answers_busy_and_still_warms_once() { let (inner, space) = seeded("sql-busy-hydration").await; @@ -3748,6 +3862,9 @@ mod tests { ); assert!(service.hydrations.lock().unwrap().is_empty()); + // The fresh hydration does real disk work (fsync); give it the + // default bound so only the gated first attempt is bound-limited. + let service = service.with_wait_bounds(SqlWaitBounds::default()); repo.load_open.send_replace(true); assert_eq!(rows(read(&service, &space).await.unwrap()), 1); assert_eq!(repo.loads(), 2, "the next request hydrated afresh"); @@ -3820,16 +3937,29 @@ mod tests { #[async_trait] impl DatabaseArtifactRepository for PanickingArtifactRepository { - async fn load( + async fn load_meta( &self, service: &str, space: &str, name: &str, - ) -> Result, DatabaseArtifactError> { + ) -> Result, DatabaseArtifactError> { if !self.panicked.swap(true, Ordering::SeqCst) { panic!("injected hydration panic"); } - self.inner.load(service, space, name).await + self.inner.load_meta(service, space, name).await + } + + async fn read_chunks( + &self, + service: &str, + space: &str, + name: &str, + meta: &ArtifactMeta, + sink: &mut dyn ArtifactSink, + ) -> Result<(), DatabaseArtifactError> { + self.inner + .read_chunks(service, space, name, meta, sink) + .await } async fn save( @@ -3910,4 +4040,912 @@ mod tests { assert!(!context.latch(), "the request was cut, not latched"); assert_eq!(rows(read(&service, &space).await.unwrap()), 1); } + + /// Counts metadata reads and the payload bytes delivered to sinks. + struct ByteSpyRepository { + inner: Arc, + metas: AtomicU64, + chunk_reads: AtomicU64, + payload_bytes: Arc, + } + + struct CountingSink<'a>(&'a mut dyn ArtifactSink, Arc); + + #[async_trait] + impl ArtifactSink for CountingSink<'_> { + async fn reset(&mut self) -> Result<(), DatabaseArtifactError> { + self.0.reset().await + } + async fn begin_part( + &mut self, + part: crate::database_artifacts::ArtifactPart, + ) -> Result<(), DatabaseArtifactError> { + self.0.begin_part(part).await + } + async fn write(&mut self, bytes: &[u8]) -> Result<(), DatabaseArtifactError> { + self.1.fetch_add(bytes.len() as u64, Ordering::SeqCst); + self.0.write(bytes).await + } + async fn finish_part(&mut self) -> Result<(), DatabaseArtifactError> { + self.0.finish_part().await + } + } + + #[async_trait] + impl DatabaseArtifactRepository for ByteSpyRepository { + async fn load_meta( + &self, + service: &str, + space: &str, + name: &str, + ) -> Result, DatabaseArtifactError> { + self.metas.fetch_add(1, Ordering::SeqCst); + self.inner.load_meta(service, space, name).await + } + + async fn read_chunks( + &self, + service: &str, + space: &str, + name: &str, + meta: &ArtifactMeta, + sink: &mut dyn ArtifactSink, + ) -> Result<(), DatabaseArtifactError> { + self.chunk_reads.fetch_add(1, Ordering::SeqCst); + let mut counting = CountingSink(sink, Arc::clone(&self.payload_bytes)); + self.inner + .read_chunks(service, space, name, meta, &mut counting) + .await + } + + async fn save( + &self, + service: &str, + space: &str, + name: &str, + payload: Vec, + expected: ArtifactExpectation, + ) -> Result { + self.inner + .save(service, space, name, payload, expected) + .await + } + + async fn save_delta( + &self, + service: &str, + space: &str, + name: &str, + payload: Vec, + expected: ArtifactExpectation, + ) -> Result { + self.inner + .save_delta(service, space, name, payload, expected) + .await + } + } + + #[tokio::test] + async fn forbid_growth_checks_and_export_existence_read_no_payload_bytes() { + let spy = Arc::new(ByteSpyRepository { + inner: artifact_repository().await, + metas: AtomicU64::new(0), + chunk_reads: AtomicU64::new(0), + payload_bytes: Arc::new(AtomicU64::new(0)), + }); + let cache = TempDir::new().unwrap(); + let space = test_space_id("sql-zero-payload"); + let service = SqlService::new( + cache.path().to_string_lossy().into_owned(), + u64::MAX, + spy.clone(), + ); + let write = |sql: &str| SqlRequest::Execute { + schema: Some(vec![ + "CREATE TABLE IF NOT EXISTS notes (id INTEGER PRIMARY KEY, body TEXT)".into(), + ]), + sql: sql.into(), + params: vec![], + }; + service + .execute( + &space, + "main", + write("INSERT INTO notes VALUES (1, 'kept')"), + None, + "tinycloud.sql/admin".into(), + ) + .await + .unwrap(); + let reset = || { + spy.metas.store(0, Ordering::SeqCst); + spy.chunk_reads.store(0, Ordering::SeqCst); + spy.payload_bytes.store(0, Ordering::SeqCst); + }; + + // Export of a live, present database: existence from metadata alone. + reset(); + assert!(!service.export(&space, "main").await.unwrap().is_empty()); + assert_eq!(spy.metas.load(Ordering::SeqCst), 1); + assert_eq!(spy.chunk_reads.load(Ordering::SeqCst), 0); + assert_eq!(spy.payload_bytes.load(Ordering::SeqCst), 0); + + // Export of a missing database. + reset(); + assert!(matches!( + service.export(&space, "absent").await, + Err(SqlError::DatabaseNotFound) + )); + assert_eq!(spy.payload_bytes.load(Ordering::SeqCst), 0); + assert_eq!(spy.chunk_reads.load(Ordering::SeqCst), 0); + + // A write that would grow a full space: the durable size comes from + // metadata, not from reading the checkpoint back. + reset(); + let rejected = service + .execute_without_growth( + &space, + "main", + SqlRequest::Execute { + schema: None, + sql: "INSERT INTO notes VALUES (2, ?)".into(), + params: vec![SqlValue::Text("x".repeat(256 * 1024))], + }, + None, + "tinycloud.sql/admin".into(), + ) + .await; + assert!(matches!(rejected, Err(SqlError::StorageWouldGrow))); + assert_eq!(spy.chunk_reads.load(Ordering::SeqCst), 0); + assert_eq!(spy.payload_bytes.load(Ordering::SeqCst), 0); + + // A write that fits: it reaches the Forbid size comparison, which is + // answered from metadata. (Warm the actor first; hydration does read.) + service + .execute( + &space, + "main", + SqlRequest::Query { + sql: "SELECT COUNT(*) FROM notes".into(), + params: vec![], + max_rows: None, + max_bytes: None, + }, + None, + "tinycloud.sql/read".into(), + ) + .await + .unwrap(); + reset(); + service + .execute_without_growth( + &space, + "main", + SqlRequest::Execute { + schema: None, + sql: "UPDATE notes SET body = 'kept'".into(), + params: vec![], + }, + None, + "tinycloud.sql/admin".into(), + ) + .await + .unwrap(); + assert!( + spy.metas.load(Ordering::SeqCst) >= 1, + "the Forbid path consulted the durable size" + ); + assert_eq!(spy.chunk_reads.load(Ordering::SeqCst), 0); + assert_eq!(spy.payload_bytes.load(Ordering::SeqCst), 0); + } + + #[tokio::test] + async fn hydration_rejects_a_corrupt_artifact_and_seeds_nothing() { + use sea_orm::ConnectionTrait; + let db = Database::connect(ConnectOptions::new("sqlite::memory:".to_string())) + .await + .unwrap(); + Migrator::up(&db, None).await.unwrap(); + let repo = Arc::new(SeaOrmDatabaseArtifactRepository::new(db.clone())); + let space = test_space_id("sql-corrupt-hydration"); + let seed_cache = TempDir::new().unwrap(); + let seed = SqlService::new( + seed_cache.path().to_string_lossy().into_owned(), + u64::MAX, + repo.clone(), + ); + seed.execute( + &space, + "main", + SqlRequest::Execute { + schema: Some(vec!["CREATE TABLE t (id INTEGER)".into()]), + sql: "INSERT INTO t VALUES (1)".into(), + params: vec![], + }, + None, + "tinycloud.sql/admin".into(), + ) + .await + .unwrap(); + // Keep the SQLite header so only the content hash can catch it. + let stored = repo + .read_all("sql", &space.to_string(), "main") + .await + .unwrap() + .unwrap(); + let mut flipped = stored.payload.clone(); + *flipped.last_mut().unwrap() ^= 0xff; + db.execute(sea_orm::Statement::from_sql_and_values( + sea_orm::DbBackend::Sqlite, + "UPDATE database_artifact SET payload = ?", + [flipped.into()], + )) + .await + .unwrap(); + + let cold_cache = TempDir::new().unwrap(); + let cold = SqlService::new( + cold_cache.path().to_string_lossy().into_owned(), + u64::MAX, + repo, + ); + let result = cold + .execute( + &space, + "main", + SqlRequest::Query { + sql: "SELECT COUNT(*) FROM t".into(), + params: vec![], + max_rows: None, + max_bytes: None, + }, + None, + "tinycloud.sql/read".into(), + ) + .await; + assert!(result.is_err(), "a corrupt artifact must not hydrate"); + let leftovers: Vec<_> = walkdir_files(cold_cache.path()); + assert!(leftovers.is_empty(), "nothing seeded: {leftovers:?}"); + } + + fn walkdir_files(dir: &Path) -> Vec { + let mut found = Vec::new(); + for entry in std::fs::read_dir(dir).into_iter().flatten().flatten() { + let path = entry.path(); + if path.is_dir() { + found.extend(walkdir_files(&path)); + } else { + found.push(path); + } + } + found + } + + /// Counts every repository call, by kind. + struct CallCountingRepository { + inner: Arc, + calls: Arc>>, + } + + impl CallCountingRepository { + fn note(&self, call: &'static str) { + self.calls.lock().unwrap().push(call); + } + } + + #[async_trait] + impl DatabaseArtifactRepository for CallCountingRepository { + async fn load_meta( + &self, + service: &str, + space: &str, + name: &str, + ) -> Result, DatabaseArtifactError> { + self.note("load_meta"); + self.inner.load_meta(service, space, name).await + } + + async fn read_chunks( + &self, + service: &str, + space: &str, + name: &str, + meta: &ArtifactMeta, + sink: &mut dyn ArtifactSink, + ) -> Result<(), DatabaseArtifactError> { + self.note("read_chunks"); + self.inner + .read_chunks(service, space, name, meta, sink) + .await + } + + async fn save( + &self, + service: &str, + space: &str, + name: &str, + payload: Vec, + expected: ArtifactExpectation, + ) -> Result { + self.note("save"); + self.inner + .save(service, space, name, payload, expected) + .await + } + + async fn save_delta( + &self, + service: &str, + space: &str, + name: &str, + payload: Vec, + expected: ArtifactExpectation, + ) -> Result { + self.note("save_delta"); + self.inner + .save_delta(service, space, name, payload, expected) + .await + } + } + + fn atomic_batch(statements: &[&str]) -> SqlRequest { + SqlRequest::Batch { + statements: statements + .iter() + .map(|sql| SqlStatement { + sql: (*sql).into(), + params: Vec::new(), + }) + .collect(), + } + } + + async fn rows_of(service: &SqlService, space: &SpaceId, sql: &str) -> Vec> { + let result = service + .execute( + space, + "main", + SqlRequest::Query { + sql: sql.into(), + params: Vec::new(), + max_rows: None, + max_bytes: None, + }, + None, + "tinycloud.sql/read".into(), + ) + .await + .unwrap(); + match result.response { + SqlResponse::Query(query) => query.rows, + other => panic!("expected a query response, got {other:?}"), + } + } + + /// TC-1491: a failing batch leaves the local file unchanged, writes + /// nothing to Postgres, and the local file still equals the durable + /// artifact after the next successful write. + #[tokio::test] + async fn tc1491_failed_batch_matches_what_the_durable_artifact_holds() { + for memory_threshold in [u64::MAX, 0] { + let repo = artifact_repository().await; + let calls = Arc::new(std::sync::Mutex::new(Vec::new())); + let cache = TempDir::new().unwrap(); + let space = test_space_id("sql-atomic-batch"); + let service = SqlService::new( + cache.path().to_string_lossy().into_owned(), + memory_threshold, + Arc::new(CallCountingRepository { + inner: repo.clone(), + calls: calls.clone(), + }), + ); + let write = |sql: &'static str| { + service.execute( + &space, + "main", + SqlRequest::Execute { + schema: None, + sql: sql.into(), + params: Vec::new(), + }, + None, + "tinycloud.sql/admin".into(), + ) + }; + write("CREATE TABLE t (id INTEGER PRIMARY KEY, v TEXT NOT NULL)") + .await + .unwrap(); + write("INSERT INTO t (v) VALUES ('durable')").await.unwrap(); + let handle = service.handle(&space, "main").await.unwrap(); + let before = handle.checkpoint().await.unwrap(); + + // The failing request: no repository call at all, no local change. + calls.lock().unwrap().clear(); + let failed = service + .execute( + &space, + "main", + atomic_batch(&[ + "INSERT INTO t (v) VALUES ('prefix-1')", + "UPDATE t SET v = 'prefix-2'", + "INSERT INTO t (v) VALUES (NULL)", + ]), + None, + "tinycloud.sql/write".into(), + ) + .await; + assert!(matches!(failed, Err(SqlError::Sqlite(_))), "{failed:?}"); + assert!( + calls.lock().unwrap().is_empty(), + "a rejected batch costs no repository (Postgres) round trip: {:?}", + calls.lock().unwrap() + ); + assert_eq!(handle.checkpoint().await.unwrap(), before); + + // A successful 3-statement batch costs exactly the one persist + // call a single statement costs. + calls.lock().unwrap().clear(); + service + .execute( + &space, + "main", + atomic_batch(&[ + "INSERT INTO t (v) VALUES ('next-1')", + "INSERT INTO t (v) VALUES ('next-2')", + "INSERT INTO t (v) VALUES ('next-3')", + ]), + None, + "tinycloud.sql/write".into(), + ) + .await + .unwrap(); + let persisted = calls.lock().unwrap().clone(); + assert_eq!(persisted.len(), 1, "{persisted:?}"); + assert!( + matches!(persisted[0], "save" | "save_delta"), + "{persisted:?}" + ); + + // What a cold node rebuilds from the repository is the local state. + let local = rows_of(&service, &space, "SELECT id, v FROM t ORDER BY id").await; + assert_eq!(local.len(), 4); + assert!(!format!("{local:?}").contains("prefix")); + let cold_cache = TempDir::new().unwrap(); + let cold = SqlService::new( + cold_cache.path().to_string_lossy().into_owned(), + memory_threshold, + repo, + ); + assert_eq!( + rows_of(&cold, &space, "SELECT id, v FROM t ORDER BY id").await, + local + ); + } + } + + /// TC-1491: the budget and the disk reservation cost no Postgres round + /// trip, and a refused write costs none at all. + #[tokio::test] + async fn tc1491_reservation_adds_no_repository_round_trips() { + for memory_threshold in [u64::MAX, 0] { + let mut sequences = Vec::new(); + for budget in [None, Some(1 << 30)] { + let calls = Arc::new(std::sync::Mutex::new(Vec::new())); + let cache = TempDir::new().unwrap(); + let service = SqlService::new( + cache.path().to_string_lossy().into_owned(), + memory_threshold, + Arc::new(CallCountingRepository { + inner: artifact_repository().await, + calls: calls.clone(), + }), + ); + let space = test_space_id("sql-reservation-trips"); + let write = |sql: &str| { + let request = SqlRequest::Execute { + schema: None, + sql: sql.to_string(), + params: Vec::new(), + }; + let ability = "tinycloud.sql/admin".to_string(); + let space = space.clone(); + let service = service.clone(); + async move { + match budget { + None => { + service + .execute(&space, "main", request, None, ability) + .await + } + Some(left) => { + service + .execute_with_budget( + &space, "main", request, None, ability, left, + ) + .await + } + } + } + }; + write("CREATE TABLE t (id INTEGER PRIMARY KEY, v BLOB)") + .await + .unwrap(); + write("INSERT INTO t (v) VALUES (zeroblob(1000))") + .await + .unwrap(); + sequences.push(calls.lock().unwrap().clone()); + } + assert_eq!( + sequences[0], sequences[1], + "a budget changes which repository calls a write makes" + ); + // One persist per write, after the initial hydration lookup. + let persists = sequences[0] + .iter() + .filter(|call| matches!(**call, "save" | "save_delta")) + .count(); + assert_eq!(persists, 2, "{:?}", sequences[0]); + } + + // A write the budget refuses reaches Postgres not at all, and leaves the + // local database as it was. + let calls = Arc::new(std::sync::Mutex::new(Vec::new())); + let cache = TempDir::new().unwrap(); + let service = SqlService::new( + cache.path().to_string_lossy().into_owned(), + 0, + Arc::new(CallCountingRepository { + inner: artifact_repository().await, + calls: calls.clone(), + }), + ); + let space = test_space_id("sql-reservation-refused"); + let request = |sql: &str| SqlRequest::Execute { + schema: None, + sql: sql.to_string(), + params: Vec::new(), + }; + service + .execute( + &space, + "main", + request("CREATE TABLE t (id INTEGER PRIMARY KEY, v BLOB)"), + None, + "tinycloud.sql/admin".into(), + ) + .await + .unwrap(); + calls.lock().unwrap().clear(); + let refused = service + .execute_with_budget( + &space, + "main", + request("INSERT INTO t (v) VALUES (zeroblob(500000))"), + None, + "tinycloud.sql/write".into(), + 4096, + ) + .await; + assert!( + matches!(refused, Err(SqlError::StorageWouldGrow)), + "{refused:?}" + ); + assert!( + calls.lock().unwrap().is_empty(), + "{:?}", + calls.lock().unwrap() + ); + assert!(rows_of(&service, &space, "SELECT id FROM t") + .await + .is_empty()); + assert_eq!(service.durability.budget.reserved(), 0); + } + + /// TC-1491: with the disk headroom check on, a successful write holds + /// nothing once its save is done, except the growth the next checkpoint + /// will cause. A save of a small WAL ships the WAL and does not + /// checkpoint, so that growth stays held until a checkpoint runs. + #[tokio::test] + async fn tc1491_a_saved_write_holds_only_its_pending_checkpoint_growth() { + let cache = TempDir::new().unwrap(); + let durability = crate::sql::durability::DurabilityPolicy { + min_free_bytes: 1, + ..Default::default() + }; + let service = SqlService::new( + cache.path().to_string_lossy().into_owned(), + 0, + artifact_repository().await, + ) + .with_durability(durability); + let space = test_space_id("sql-reservation-pending"); + let write = |sql: &str| { + service.execute_with_budget( + &space, + "main", + SqlRequest::Execute { + schema: None, + sql: sql.to_string(), + params: Vec::new(), + }, + None, + "tinycloud.sql/admin".into(), + 1 << 30, + ) + }; + let budget = &service.durability.budget; + + write("CREATE TABLE t (id INTEGER PRIMARY KEY, v BLOB)") + .await + .unwrap(); + assert_eq!(budget.reserved(), 0); + + // Saved as a WAL delta: no checkpoint, so the growth is still held. + write("INSERT INTO t (v) VALUES (zeroblob(400000))") + .await + .unwrap(); + let held = budget.reserved(); + assert!(held >= 390_000, "held {held}"); + write("INSERT INTO t (v) VALUES (zeroblob(10))") + .await + .unwrap(); + assert!(budget.reserved() >= 390_000); + + // A checkpoint puts it on the disk; a write that adds no page then + // leaves nothing held after its save. + let handle = service.handle(&space, "main").await.unwrap(); + handle.checkpoint().await.unwrap(); + assert_eq!(budget.reserved(), 0); + write("INSERT INTO t (v) VALUES (zeroblob(10))") + .await + .unwrap(); + assert_eq!(budget.reserved(), 0); + } + + /// TC-1491 + TC-1585: a `WITH … INSERT` is a write, so in a batch it is + /// one of the statements that must be kept or undone with the rest. A + /// later failure leaves nothing of it locally, writes nothing to Postgres, + /// and a cold node rebuilt from the durable artifact agrees. + #[tokio::test] + async fn tc1491_nested_write_cte_in_a_failed_batch_leaves_nothing() { + for memory_threshold in [u64::MAX, 0] { + let repo = artifact_repository().await; + let calls = Arc::new(std::sync::Mutex::new(Vec::new())); + let cache = TempDir::new().unwrap(); + let space = test_space_id("sql-atomic-cte"); + let service = SqlService::new( + cache.path().to_string_lossy().into_owned(), + memory_threshold, + Arc::new(CallCountingRepository { + inner: repo.clone(), + calls: calls.clone(), + }), + ); + let run = |request: SqlRequest| { + service.execute(&space, "main", request, None, "tinycloud.sql/admin".into()) + }; + let batch = |statements: &[&str]| SqlRequest::Batch { + statements: statements + .iter() + .map(|sql| SqlStatement { + sql: (*sql).into(), + params: Vec::new(), + }) + .collect(), + }; + run(SqlRequest::Execute { + schema: None, + sql: "CREATE TABLE t (id INTEGER PRIMARY KEY, v TEXT NOT NULL)".into(), + params: Vec::new(), + }) + .await + .unwrap(); + let handle = service.handle(&space, "main").await.unwrap(); + let before = handle.checkpoint().await.unwrap(); + + calls.lock().unwrap().clear(); + let failed = run(batch(&[ + "WITH c AS (SELECT 'cte-insert' AS v) INSERT INTO t (v) SELECT v FROM c", + "INSERT INTO t (v) VALUES (NULL)", + ])) + .await; + assert!(matches!(failed, Err(SqlError::Sqlite(_))), "{failed:?}"); + assert!( + calls.lock().unwrap().is_empty(), + "a rejected batch costs no repository (Postgres) round trip: {:?}", + calls.lock().unwrap() + ); + assert_eq!(handle.checkpoint().await.unwrap(), before); + assert!(rows_of(&service, &space, "SELECT v FROM t") + .await + .is_empty()); + + // The same CTE write kept when the batch succeeds, and durable. + run(batch(&[ + "WITH c AS (SELECT 'cte-insert' AS v) INSERT INTO t (v) SELECT v FROM c", + "INSERT INTO t (v) VALUES ('plain')", + ])) + .await + .unwrap(); + let local = rows_of(&service, &space, "SELECT id, v FROM t ORDER BY id").await; + assert_eq!(local.len(), 2); + let cold_cache = TempDir::new().unwrap(); + let cold = SqlService::new( + cold_cache.path().to_string_lossy().into_owned(), + memory_threshold, + repo, + ); + assert_eq!( + rows_of(&cold, &space, "SELECT id, v FROM t ORDER BY id").await, + local + ); + } + } + + /// TC-1491: a single write is all-or-nothing whatever its conflict clause + /// says. A statement that stops with `OR FAIL` or `ON CONFLICT FAIL` + /// leaves the rows it changed first when it runs on its own; the request + /// is one transaction, so it leaves none of them: the local file is + /// unchanged and no repository (Postgres) call is made. + #[tokio::test] + async fn tc1491_a_single_write_with_fail_conflict_resolution_leaves_nothing() { + for memory_threshold in [u64::MAX, 0] { + let repo = artifact_repository().await; + let calls = Arc::new(std::sync::Mutex::new(Vec::new())); + let cache = TempDir::new().unwrap(); + let space = test_space_id("sql-atomic-or-fail"); + let service = SqlService::new( + cache.path().to_string_lossy().into_owned(), + memory_threshold, + Arc::new(CallCountingRepository { + inner: repo, + calls: calls.clone(), + }), + ); + let run = |sql: &str, ability: &str| { + service.execute( + &space, + "main", + SqlRequest::Execute { + schema: None, + sql: sql.into(), + params: Vec::new(), + }, + None, + ability.into(), + ) + }; + run( + "CREATE TABLE u (id INTEGER PRIMARY KEY, v TEXT UNIQUE)", + "tinycloud.sql/admin", + ) + .await + .unwrap(); + run( + "INSERT INTO u VALUES (10,'x'),(11,'y'),(12,'z')", + "tinycloud.sql/write", + ) + .await + .unwrap(); + let handle = service.handle(&space, "main").await.unwrap(); + let before = handle.checkpoint().await.unwrap(); + + for sql in ["INSERT OR FAIL INTO u VALUES (1,'a'),(2,'b'),(1,'dup')"] { + calls.lock().unwrap().clear(); + let failed = run(sql, "tinycloud.sql/write").await; + assert!( + matches!(failed, Err(SqlError::Sqlite(_))), + "{sql}: {failed:?}" + ); + assert!( + calls.lock().unwrap().is_empty(), + "{sql}: a rejected write costs no repository call" + ); + assert_eq!( + rows_of(&service, &space, "SELECT count(*) FROM u").await, + vec![vec![SqlValue::Integer(3)]], + "{sql}" + ); + assert_eq!( + rows_of(&service, &space, "SELECT group_concat(v) FROM u").await, + vec![vec![SqlValue::Text("x,y,z".into())]], + "{sql}" + ); + assert!( + handle.checkpoint().await.unwrap() == before, + "{sql}: the checkpoint must not change" + ); + } + + // The same write as a `query` request, which the classifier + // treats as a write too. + let failed = service + .execute( + &space, + "main", + SqlRequest::Query { + sql: "INSERT OR FAIL INTO u VALUES (1,'a'),(2,'b'),(1,'dup')".into(), + params: Vec::new(), + max_rows: None, + max_bytes: None, + }, + None, + "tinycloud.sql/write".into(), + ) + .await; + assert!(failed.is_err(), "{failed:?}"); + assert!(handle.checkpoint().await.unwrap() == before); + } + } + + /// TC-1491: a deferred foreign key is checked when the request's + /// transaction commits. A violation there rolls the request back: nothing + /// is kept, the error is reported, and no repository call is made. + #[tokio::test] + async fn tc1491_a_deferred_constraint_failing_at_commit_leaves_nothing() { + for memory_threshold in [u64::MAX, 0] { + let repo = artifact_repository().await; + let calls = Arc::new(std::sync::Mutex::new(Vec::new())); + let cache = TempDir::new().unwrap(); + let space = test_space_id("sql-atomic-deferred-fk"); + let service = SqlService::new( + cache.path().to_string_lossy().into_owned(), + memory_threshold, + Arc::new(CallCountingRepository { + inner: repo, + calls: calls.clone(), + }), + ); + let run = |request: SqlRequest| { + service.execute(&space, "main", request, None, "tinycloud.sql/admin".into()) + }; + let execute = |sql: &str| SqlRequest::Execute { + schema: None, + sql: sql.into(), + params: Vec::new(), + }; + for ddl in [ + "CREATE TABLE parents (id INTEGER PRIMARY KEY)", + "CREATE TABLE children (parent_id INTEGER REFERENCES parents(id) DEFERRABLE INITIALLY DEFERRED)", + "INSERT INTO parents VALUES (1)", + "INSERT INTO children VALUES (1)", + ] { + run(execute(ddl)).await.unwrap(); + } + let handle = service.handle(&space, "main").await.unwrap(); + let before = handle.checkpoint().await.unwrap(); + + for failing in [ + execute("DELETE FROM parents"), + atomic_batch(&[ + "INSERT INTO parents VALUES (2)", + "DELETE FROM parents WHERE id = 1", + ]), + ] { + calls.lock().unwrap().clear(); + let failed = run(failing).await; + assert!(matches!(failed, Err(SqlError::Sqlite(_))), "{failed:?}"); + assert!( + calls.lock().unwrap().is_empty(), + "a request rejected at commit costs no repository call: {:?}", + calls.lock().unwrap() + ); + assert_eq!( + rows_of(&service, &space, "SELECT id FROM parents ORDER BY id").await, + vec![vec![SqlValue::Integer(1)]] + ); + assert!(handle.checkpoint().await.unwrap() == before); + } + + // The connection is not left inside a transaction: the next + // request commits and is persisted. + calls.lock().unwrap().clear(); + run(execute("DELETE FROM children")).await.unwrap(); + assert_eq!(calls.lock().unwrap().len(), 1); + assert!(rows_of(&service, &space, "SELECT * FROM children") + .await + .is_empty()); + } + } } diff --git a/tinycloud-core/src/sql/storage.rs b/tinycloud-core/src/sql/storage.rs index ae0de8f4..d0e9661a 100644 --- a/tinycloud-core/src/sql/storage.rs +++ b/tinycloud-core/src/sql/storage.rs @@ -4,6 +4,7 @@ use rusqlite::Connection; use crate::request_telemetry::QueryTimer; +use super::durability::{create_dir_all_durable, set_synchronous, sync_dir, Synchronous}; use super::types::SqlError; #[derive(Debug, Clone)] @@ -12,14 +13,17 @@ pub enum StorageMode { File(PathBuf), } -pub fn open_connection(mode: &StorageMode) -> Result { +pub fn open_connection( + mode: &StorageMode, + synchronous: Synchronous, +) -> Result { let conn = match mode { StorageMode::InMemory => { Connection::open_in_memory().map_err(|e| SqlError::Internal(e.to_string()))? } StorageMode::File(path) => { if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent).map_err(|e| SqlError::Internal(e.to_string()))?; + create_dir_all_durable(parent).map_err(|e| SqlError::Internal(e.to_string()))?; } Connection::open(path).map_err(|e| SqlError::Internal(e.to_string()))? } @@ -27,6 +31,8 @@ pub fn open_connection(mode: &StorageMode) -> Result { // Enable WAL mode for file-backed databases if matches!(mode, StorageMode::File(_)) { + // Before the journal mode: the WAL switch is itself a write. + set_synchronous(&conn, synchronous)?; { let _timer = QueryTimer::new(); conn.pragma_update(None, "journal_mode", "wal") @@ -63,9 +69,13 @@ pub fn database_size(conn: &Connection) -> Result { Ok(page_count * page_size) } -pub fn promote_to_file(conn: &Connection, path: &PathBuf) -> Result { +pub fn promote_to_file( + conn: &Connection, + path: &PathBuf, + synchronous: Synchronous, +) -> Result { if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent).map_err(|e| SqlError::Internal(e.to_string()))?; + create_dir_all_durable(parent).map_err(|e| SqlError::Internal(e.to_string()))?; } // rusqlite appends the file path, which holds the space id, to open @@ -77,6 +87,9 @@ pub fn promote_to_file(conn: &Connection, path: &PathBuf) -> Result Result>>>, - /// Set once [`SqlSizes::seed_from`] has loaded every artifact's size. - Arc, -); +pub struct SqlSizes(Arc>>>); impl SqlSizes { pub fn new() -> Self { @@ -56,30 +52,9 @@ impl SqlSizes { .insert((service, name), size.max(0) as u64); } *self.0.write().await = fresh; // overwrite the map wholesale with DB truth - self.1.store(true, std::sync::atomic::Ordering::Release); Ok(()) } - /// The stored size of one artifact as far as this node knows: its - /// recorded size, `Some(0)` for one absent from a seeded mirror (this - /// node is the artifacts' only writer), `None` before the mirror is - /// seeded. Stale only after a save whose outcome was unknown, by at most - /// that save's delta. - pub async fn known_size(&self, service: &str, space: &str, name: &str) -> Option { - if !self.1.load(std::sync::atomic::Ordering::Acquire) { - return None; - } - Some( - self.0 - .read() - .await - .get(space) - .and_then(|inner| inner.get(&(service.to_string(), name.to_string()))) - .copied() - .unwrap_or(0), - ) - } - /// Overwrite (not accumulate) the size for one (service, space, db-name). pub async fn update(&self, service: &str, space: &str, name: &str, size_bytes: u64) { self.0 @@ -122,27 +97,25 @@ impl SizeTrackingArtifactRepository { #[async_trait] impl DatabaseArtifactRepository for SizeTrackingArtifactRepository { - async fn load( + async fn load_meta( &self, service: &str, space: &str, name: &str, - ) -> Result, DatabaseArtifactError> { - let known_size = self.sizes.known_size(service, space, name).await; - self.inner - .load_sized(service, space, name, known_size) - .await + ) -> Result, DatabaseArtifactError> { + self.inner.load_meta(service, space, name).await } - async fn load_sized( + async fn read_chunks( &self, service: &str, space: &str, name: &str, - known_size: Option, - ) -> Result, DatabaseArtifactError> { + meta: &ArtifactMeta, + sink: &mut dyn ArtifactSink, + ) -> Result<(), DatabaseArtifactError> { self.inner - .load_sized(service, space, name, known_size) + .read_chunks(service, space, name, meta, sink) .await } @@ -197,24 +170,6 @@ mod tests { ssi::{dids::DIDBuf, jwk::JWK}, }; - #[tokio::test] - async fn known_sizes_are_unknown_until_seeded() { - let db = Database::connect(ConnectOptions::new("sqlite::memory:".to_string())) - .await - .unwrap(); - Migrator::up(&db, None).await.unwrap(); - let sizes = SqlSizes::new(); - assert_eq!(sizes.known_size("sql", "space", "db").await, None); - sizes.seed_from(&db).await.unwrap(); - assert_eq!( - sizes.known_size("sql", "space", "db").await, - Some(0), - "absent" - ); - sizes.update("sql", "space", "db", 9 << 20).await; - assert_eq!(sizes.known_size("sql", "space", "db").await, Some(9 << 20)); - } - fn test_space_id(name: &str) -> SpaceId { let jwk = JWK::generate_ed25519().unwrap(); let did: DIDBuf = DID_METHODS.generate(&jwk, "key").unwrap(); diff --git a/tinycloud-core/tests/artifact_bounded_read.rs b/tinycloud-core/tests/artifact_bounded_read.rs new file mode 100644 index 00000000..6d5ae21d --- /dev/null +++ b/tinycloud-core/tests/artifact_bounded_read.rs @@ -0,0 +1,130 @@ +//! TC-822: hydrating a large artifact streams it in slices, so the Rust heap +//! the node holds for it does not grow with the artifact. +//! +//! The row is inserted with `zeroblob` (no 256 MiB buffer in the test) and its +//! hash is computed incrementally. A tracking allocator records the peak heap +//! growth during the read. SQLite's own `substr(blob, ..)` materialises the +//! blob inside the SQLite library (C allocator, not counted here); the +//! Postgres equivalent is measured separately (see the PR). + +use std::alloc::{GlobalAlloc, Layout, System}; +use std::sync::atomic::{AtomicUsize, Ordering}; + +use sea_orm::{ConnectOptions, ConnectionTrait, Database, DbBackend, Statement}; +use tinycloud_core::{ + database_artifacts::{ + read_artifact, CacheFileSink, SeaOrmDatabaseArtifactRepository, ARTIFACT_CHUNK_BYTES, + }, + hash::Blake3Hasher, + migrations::Migrator, + sea_orm_migration::MigratorTrait, +}; + +struct Tracking; +static CURRENT: AtomicUsize = AtomicUsize::new(0); +static PEAK: AtomicUsize = AtomicUsize::new(0); + +unsafe impl GlobalAlloc for Tracking { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + let ptr = System.alloc(layout); + if !ptr.is_null() { + let now = CURRENT.fetch_add(layout.size(), Ordering::Relaxed) + layout.size(); + PEAK.fetch_max(now, Ordering::Relaxed); + } + ptr + } + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { + System.dealloc(ptr, layout); + CURRENT.fetch_sub(layout.size(), Ordering::Relaxed); + } + unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { + let new = System.realloc(ptr, layout, new_size); + if !new.is_null() { + if new_size >= layout.size() { + let now = CURRENT.fetch_add(new_size - layout.size(), Ordering::Relaxed) + new_size + - layout.size(); + PEAK.fetch_max(now, Ordering::Relaxed); + } else { + CURRENT.fetch_sub(layout.size() - new_size, Ordering::Relaxed); + } + } + new + } +} + +#[global_allocator] +static ALLOC: Tracking = Tracking; + +const ARTIFACT_BYTES: usize = 256 * 1024 * 1024; + +#[tokio::test] +async fn a_256_mib_artifact_hydrates_with_a_bounded_heap() { + let dir = tempfile::tempdir().unwrap(); + let mut options = ConnectOptions::new(format!( + "sqlite:{}?mode=rwc", + dir.path().join("caps.db").display() + )); + options.max_connections(1); + let conn = Database::connect(options).await.unwrap(); + Migrator::up(&conn, None).await.unwrap(); + + let mut hasher = Blake3Hasher::new(); + let zeros = vec![0_u8; ARTIFACT_CHUNK_BYTES]; + for _ in 0..ARTIFACT_BYTES / ARTIFACT_CHUNK_BYTES { + hasher.update(&zeros); + } + drop(zeros); + let hash = hasher.finalize().to_cid(0x55).to_string(); + conn.execute(Statement::from_sql_and_values( + DbBackend::Sqlite, + "INSERT INTO database_artifact (service, space, name, revision, content_hash, payload, \ + size_bytes, backend, storage_mode, created_at, updated_at, checkpoint_size_bytes, \ + checkpoint_content_hash, delta_payload, delta_content_hash, delta_size_bytes) \ + VALUES ('sql', 'space', 'big', 1, ?, zeroblob(?), ?, 'storage.database', \ + 'database-blob', 'now', 'now', ?, ?, NULL, NULL, 0)", + [ + hash.clone().into(), + (ARTIFACT_BYTES as i64).into(), + (ARTIFACT_BYTES as i64).into(), + (ARTIFACT_BYTES as i64).into(), + hash.into(), + ], + )) + .await + .unwrap(); + + let repo = SeaOrmDatabaseArtifactRepository::new(conn); + let cache = tempfile::tempdir().unwrap(); + let mut sink = CacheFileSink::new( + "sql", + "space/big".to_string(), + cache.path().join("big.db"), + cache.path().join("big.db-wal"), + |_| true, + |_| true, + ); + + let baseline = CURRENT.load(Ordering::Relaxed); + PEAK.store(baseline, Ordering::Relaxed); + let meta = read_artifact(&repo, "sql", "space", "big", &mut sink) + .await + .unwrap() + .expect("the artifact exists"); + let growth = PEAK.load(Ordering::Relaxed).saturating_sub(baseline); + sink.commit().await.unwrap(); + + assert_eq!(meta.checkpoint_size_bytes as usize, ARTIFACT_BYTES); + assert_eq!( + std::fs::metadata(cache.path().join("big.db")) + .unwrap() + .len() as usize, + ARTIFACT_BYTES + ); + // A few slice-sized buffers (row decode, copy, write), never the artifact. + assert!( + growth < 6 * ARTIFACT_CHUNK_BYTES, + "heap grew {} MiB hydrating a {} MiB artifact", + growth >> 20, + ARTIFACT_BYTES >> 20 + ); +} diff --git a/tinycloud-core/tests/duckdb_diagnostics_policy.rs b/tinycloud-core/tests/duckdb_diagnostics_policy.rs index 730ecc78..eed3fabd 100644 --- a/tinycloud-core/tests/duckdb_diagnostics_policy.rs +++ b/tinycloud-core/tests/duckdb_diagnostics_policy.rs @@ -131,6 +131,53 @@ fn tenant_diagnostics_cannot_escape_process_streams_and_normal_sql_still_works() assert!(String::from_utf8_lossy(&output.stderr).contains("tc1304_stderr_capture_control")); } +/// Whole-artifact read for assertions, built on the streaming API: the +/// repository itself has no whole-row read. +#[async_trait::async_trait] +trait ReadAll { + async fn read_all( + &self, + service: &str, + space: &str, + name: &str, + ) -> Result< + Option, + tinycloud_core::database_artifacts::DatabaseArtifactError, + >; +} + +#[async_trait::async_trait] +impl ReadAll for T { + async fn read_all( + &self, + service: &str, + space: &str, + name: &str, + ) -> Result< + Option, + tinycloud_core::database_artifacts::DatabaseArtifactError, + > { + use tinycloud_core::database_artifacts::{read_artifact, DatabaseArtifact, MemorySink}; + let mut sink = MemorySink::default(); + let Some(meta) = read_artifact(self, service, space, name, &mut sink).await? else { + return Ok(None); + }; + Ok(Some(DatabaseArtifact { + payload: sink.checkpoint, + content_hash: meta.content_hash, + revision: meta.revision, + size_bytes: meta.size_bytes, + updated_at: meta.updated_at, + backend: meta.backend, + storage_mode: meta.storage_mode, + checkpoint_content_hash: meta.checkpoint_content_hash, + delta_payload: sink.wal, + delta_content_hash: meta.delta_content_hash, + delta_size_bytes: meta.delta_size_bytes, + })) + } +} + #[tokio::test] async fn invocation_template_is_not_authority_to_execute_ddl_with_read_ability() { let directory = tempfile::tempdir().unwrap(); @@ -188,7 +235,7 @@ async fn invocation_template_is_not_authority_to_execute_ddl_with_read_ability() async fn with_prefixed_writes_require_authority_and_persist_query_write_targets() { use tinycloud_auth::resource::SpaceId; use tinycloud_core::{ - database_artifacts::{DatabaseArtifactRepository, SeaOrmDatabaseArtifactRepository}, + database_artifacts::SeaOrmDatabaseArtifactRepository, duckdb::{service::DuckDbService, types::DuckDbError}, migrations::Migrator, sea_orm::{ConnectOptions, Database}, @@ -285,7 +332,7 @@ async fn with_prefixed_writes_require_authority_and_persist_query_write_targets( let before_rows = rows(&service, &space, data_sql).await; let before_catalog = rows(&service, &space, catalog_sql).await; let before_artifact = repository - .load("duckdb", &space.to_string(), "nested") + .read_all("duckdb", &space.to_string(), "nested") .await .unwrap() .unwrap(); @@ -341,7 +388,7 @@ async fn with_prefixed_writes_require_authority_and_persist_query_write_targets( assert_eq!(rows(&service, &space, data_sql).await, before_rows); assert_eq!(rows(&service, &space, catalog_sql).await, before_catalog); let denied_artifact = repository - .load("duckdb", &space.to_string(), "nested") + .read_all("duckdb", &space.to_string(), "nested") .await .unwrap() .unwrap(); @@ -382,7 +429,7 @@ async fn with_prefixed_writes_require_authority_and_persist_query_write_targets( } }; let prior = repository - .load("duckdb", &space.to_string(), "nested") + .read_all("duckdb", &space.to_string(), "nested") .await .unwrap() .unwrap(); @@ -415,7 +462,7 @@ async fn with_prefixed_writes_require_authority_and_persist_query_write_targets( next_value += 1; assert_eq!(rows(&service, &space, data_sql).await, expected); let persisted = repository - .load("duckdb", &space.to_string(), "nested") + .read_all("duckdb", &space.to_string(), "nested") .await .unwrap() .unwrap(); @@ -622,7 +669,7 @@ async fn executing_explain_template_still_requires_the_wrapped_write_ability() { async fn restricted_invocations_fail_closed_on_unsupported_sql_shapes_without_changing_data() { use tinycloud_auth::resource::SpaceId; use tinycloud_core::{ - database_artifacts::{DatabaseArtifactRepository, SeaOrmDatabaseArtifactRepository}, + database_artifacts::SeaOrmDatabaseArtifactRepository, duckdb::{ service::DuckDbService, types::{DuckDbError, DuckDbExecutionResult}, @@ -754,7 +801,7 @@ async fn restricted_invocations_fail_closed_on_unsupported_sql_shapes_without_ch let before_rows = query_rows(run(&service, &space, snapshot_sql, None, READ).await); let before_catalog = query_rows(run(&service, &space, catalog_sql, None, READ).await); let before_artifact = repository - .load("duckdb", &space.to_string(), "restrictions") + .read_all("duckdb", &space.to_string(), "restrictions") .await .unwrap() .unwrap(); @@ -859,7 +906,7 @@ async fn restricted_invocations_fail_closed_on_unsupported_sql_shapes_without_ch before_catalog ); let after_artifact = repository - .load("duckdb", &space.to_string(), "restrictions") + .read_all("duckdb", &space.to_string(), "restrictions") .await .unwrap() .unwrap(); diff --git a/tinycloud-core/tests/embedded_request_telemetry.rs b/tinycloud-core/tests/embedded_request_telemetry.rs index 244404eb..9a052104 100644 --- a/tinycloud-core/tests/embedded_request_telemetry.rs +++ b/tinycloud-core/tests/embedded_request_telemetry.rs @@ -181,7 +181,9 @@ async fn sqlite_counts_schema_execution_errors_and_guard_controls() { .unwrap(); }) .await; - assert_eq!(context.snapshot().query_count, 4); + // Schema statement, statement, and the request transaction's SAVEPOINT and + // RELEASE (TC-1491), plus the two size PRAGMAs. + assert_eq!(context.snapshot().query_count, 6); let failed = request("aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa"); scope(Arc::clone(&failed), async { actor @@ -199,7 +201,8 @@ async fn sqlite_counts_schema_execution_errors_and_guard_controls() { .expect_err("a constraint failure is an execution attempt"); }) .await; - assert_eq!(failed.snapshot().query_count, 1); + // The statement, plus the SAVEPOINT and ROLLBACK of the request transaction. + assert_eq!(failed.snapshot().query_count, 3); assert!(failed.snapshot().query_time_ns > 0); let guarded = request("bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb"); scope( diff --git a/tinycloud-core/tests/sql_local_durability.rs b/tinycloud-core/tests/sql_local_durability.rs new file mode 100644 index 00000000..a03e9897 --- /dev/null +++ b/tinycloud-core/tests/sql_local_durability.rs @@ -0,0 +1,1186 @@ +//! TC-1491 slice 1a: local commit durability and the reservation made before +//! COMMIT. See `docs/sql-local-durability.md` for the contract these pin. + +use std::io::{BufRead, BufReader}; +use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::Arc; + +use dashmap::DashMap; +use tinycloud_core::sql::database::{spawn_actor_with_policy, DatabaseHandle, GrowthPolicy}; +use tinycloud_core::sql::durability::{ + create_dir_all_durable, CommitSeams, DurabilityPolicy, FreeSpace, Synchronous, +}; +use tinycloud_core::sql::storage::{open_connection, promote_to_file, StorageMode}; +use tinycloud_core::sql::{SqlError, SqlExecutionResult, SqlRequest, SqlResponse, SqlValue}; + +const SPACE: &str = "space"; +const DB: &str = "durable"; + +/// Free space that tests set by hand. +#[derive(Debug)] +struct FakeFree(AtomicU64); + +impl FakeFree { + fn new(bytes: u64) -> Arc { + Arc::new(Self(AtomicU64::new(bytes))) + } + fn set(&self, bytes: u64) { + self.0.store(bytes, Ordering::SeqCst); + } +} + +impl FreeSpace for FakeFree { + fn free_bytes(&self, _: &Path) -> std::io::Result { + Ok(self.0.load(Ordering::SeqCst)) + } +} + +/// A policy with the headroom check ON (`min_free_bytes` is 0 = off by default, +/// so the tests that exercise the mechanism pass an explicit value). +fn policy_with(free: &Arc, min_free_bytes: u64) -> DurabilityPolicy { + assert!(min_free_bytes > 0); + DurabilityPolicy { + min_free_bytes, + free_space: free.clone(), + ..Default::default() + } +} + +/// A file-backed actor: with a zero threshold the first write promotes it. +fn actor(dir: &Path, db: &str, policy: DurabilityPolicy) -> DatabaseHandle { + spawn_actor_with_policy( + SPACE.into(), + db.into(), + dir.to_str().unwrap().into(), + 0, + Arc::new(DashMap::new()), + policy, + ) +} + +fn exec(sql: &str) -> SqlRequest { + SqlRequest::Execute { + schema: None, + sql: sql.into(), + params: vec![], + } +} + +fn batch(statements: &[&str]) -> SqlRequest { + SqlRequest::Batch { + statements: statements + .iter() + .map(|sql| tinycloud_core::sql::types::SqlStatement { + sql: (*sql).into(), + params: vec![], + }) + .collect(), + } +} + +async fn run( + actor: &DatabaseHandle, + request: SqlRequest, + growth: GrowthPolicy, +) -> Result { + actor + .execute(request, None, "tinycloud.sql/admin".into(), growth) + .await +} + +async fn ok(actor: &DatabaseHandle, sql: &str) { + run(actor, exec(sql), GrowthPolicy::Allow) + .await + .unwrap_or_else(|error| panic!("{sql}: {error}")); +} + +async fn rows(actor: &DatabaseHandle, sql: &str) -> Vec> { + match run( + actor, + SqlRequest::Query { + sql: sql.into(), + params: vec![], + max_rows: None, + max_bytes: None, + }, + GrowthPolicy::Allow, + ) + .await + .unwrap() + .response + { + SqlResponse::Query(query) => query.rows, + other => panic!("expected rows, got {other:?}"), + } +} + +async fn count(actor: &DatabaseHandle, table: &str) -> i64 { + match rows(actor, &format!("SELECT count(*) FROM {table}")).await[0][0] { + SqlValue::Integer(n) => n, + ref other => panic!("{other:?}"), + } +} + +/// Six indexes: a short statement that dirties many pages. (The SQL parser +/// admits no triggers; indexes are the amplification a caller can cause.) +const AMPLIFIED_SCHEMA: &[&str] = &[ + "CREATE TABLE t (id INTEGER PRIMARY KEY, a TEXT, b TEXT, c TEXT, d TEXT, e TEXT, f TEXT)", + "CREATE INDEX t_a ON t (a)", + "CREATE INDEX t_b ON t (b)", + "CREATE INDEX t_c ON t (c)", + "CREATE INDEX t_d ON t (d)", + "CREATE INDEX t_e ON t (e)", + "CREATE INDEX t_f ON t (f)", +]; +/// ~300 bytes of SQL text; adds 20 000 rows to seven b-trees. +const AMPLIFIED_INSERT: &str = "INSERT INTO t (a, b, c, d, e, f) \ + SELECT hex(randomblob(12)), hex(randomblob(12)), hex(randomblob(12)), \ + hex(randomblob(12)), hex(randomblob(12)), hex(randomblob(12)) \ + FROM seed s1, seed s2 LIMIT 20000"; + +/// 256 rows to cross-join, built by doubling. +async fn seed(actor: &DatabaseHandle) { + ok(actor, "CREATE TABLE seed (x INTEGER)").await; + ok(actor, "INSERT INTO seed VALUES (1)").await; + for _ in 0..8 { + ok(actor, "INSERT INTO seed SELECT x FROM seed").await; + } +} + +fn wal_len(dir: &Path, db: &str) -> u64 { + std::fs::metadata(dir.join(SPACE).join(format!("{db}.db-wal"))) + .map(|m| m.len()) + .unwrap_or(0) +} + +#[test] +fn synchronous_is_full_on_every_file_backed_connection_and_normal_on_request() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("a").join("b").join("x.db"); + let read = |conn: &rusqlite::Connection| -> i64 { + conn.query_row("PRAGMA synchronous", [], |row| row.get(0)) + .unwrap() + }; + // PRAGMA synchronous: 1 = NORMAL, 2 = FULL. + let opened = open_connection(&StorageMode::File(path.clone()), Synchronous::Full).unwrap(); + assert_eq!(read(&opened), 2); + let normal = open_connection(&StorageMode::File(path.clone()), Synchronous::Normal).unwrap(); + assert_eq!(read(&normal), 1); + drop((opened, normal)); + + let memory = open_connection(&StorageMode::InMemory, Synchronous::Full).unwrap(); + memory.execute_batch("CREATE TABLE t (v)").unwrap(); + for (setting, expected) in [(Synchronous::Full, 2), (Synchronous::Normal, 1)] { + let target = dir.path().join(format!("promoted-{expected}.db")); + let promoted = promote_to_file(&memory, &target, setting).unwrap(); + assert_eq!(read(&promoted), expected); + let mode: String = promoted + .query_row("PRAGMA journal_mode", [], |row| row.get(0)) + .unwrap(); + assert_eq!(mode, "wal"); + } +} + +/// An actor runs under either `synchronous` setting through promotion to a +/// file and a reopen. The setting itself is asserted on the connections in +/// `synchronous_is_full_on_every_file_backed_connection_and_normal_on_request`: +/// since TC-1586 an admin can no longer read `PRAGMA synchronous` through the +/// actor (it is not on the PRAGMA allow-list). +#[tokio::test] +async fn an_actor_runs_under_either_synchronous_setting_after_promotion_and_reopen() { + for setting in [Synchronous::Full, Synchronous::Normal] { + let dir = tempfile::tempdir().unwrap(); + let policy = DurabilityPolicy { + synchronous: setting, + ..Default::default() + }; + let first = actor(dir.path(), DB, policy.clone()); + ok(&first, "CREATE TABLE t (v INTEGER)").await; // promotes to a file + ok(&first, "INSERT INTO t VALUES (1)").await; + assert_eq!( + rows(&first, "PRAGMA journal_mode").await[0][0], + SqlValue::Text("wal".into()) + ); + first.shutdown().await.unwrap(); + let reopened = actor(dir.path(), DB, policy); + assert_eq!(count(&reopened, "t").await, 1); + } +} + +#[tokio::test] +async fn an_amplified_transaction_is_rejected_before_commit_and_leaves_no_effects() { + let dir = tempfile::tempdir().unwrap(); + // 1 MiB free, no floor: the SQL text is a few hundred bytes, but the pages + // it dirties are several MiB. + let free = FakeFree::new(1 << 20); + let a = actor(dir.path(), DB, policy_with(&free, 1)); + free.set(64 << 20); + for statement in AMPLIFIED_SCHEMA { + ok(&a, statement).await; + } + seed(&a).await; + ok(&a, "INSERT INTO t (a, b) VALUES ('keep', 'keep')").await; + assert!(AMPLIFIED_INSERT.len() < 600); + + let checkpoint_before = a.checkpoint().await.unwrap(); + let export_before = a.export().await.unwrap(); + + free.set(1 << 20); + let refused = run(&a, exec(AMPLIFIED_INSERT), GrowthPolicy::Allow).await; + assert!( + matches!(refused, Err(SqlError::DiskHeadroom)), + "{refused:?}" + ); + + // No effect anywhere: rows, the snapshot and export. + free.set(64 << 20); + assert_eq!(count(&a, "t").await, 1); + assert_eq!(a.checkpoint().await.unwrap(), checkpoint_before); + assert_eq!(a.export().await.unwrap(), export_before); + + // The same statement fits when the disk has room. + free.set(1 << 30); + ok(&a, AMPLIFIED_INSERT).await; + assert_eq!(count(&a, "t").await, 20_001); +} + +#[tokio::test] +async fn a_batch_is_all_or_nothing_under_each_kind_of_rejection() { + let dir = tempfile::tempdir().unwrap(); + let free = FakeFree::new(64 << 20); + let policy = DurabilityPolicy { + // 32 pages of 4 KiB. + max_database_bytes: 128 * 1024, + min_free_bytes: 1, + free_space: free.clone(), + ..Default::default() + }; + let a = actor(dir.path(), DB, policy); + for statement in AMPLIFIED_SCHEMA { + ok(&a, statement).await; + } + let baseline_checkpoint = a.checkpoint().await.unwrap(); + + // 1. Size cap: the first statements apply, the last one passes the cap. + let over_cap = run( + &a, + batch(&[ + "INSERT INTO t (a) VALUES ('first')", + "INSERT INTO t (a) VALUES ('second')", + "INSERT INTO t (a) VALUES (hex(zeroblob(200000)))", + ]), + GrowthPolicy::Allow, + ) + .await; + assert!( + matches!(over_cap, Err(SqlError::DatabaseSizeLimit)), + "{over_cap:?}" + ); + assert_eq!(count(&a, "t").await, 0); + + // 2. Disk headroom at the gate. + free.set(1 << 10); + let no_room = run( + &a, + batch(&[ + "INSERT INTO t (a) VALUES ('first')", + "INSERT INTO t (a) VALUES ('second')", + ]), + GrowthPolicy::Allow, + ) + .await; + assert!( + matches!(no_room, Err(SqlError::DiskHeadroom)), + "{no_room:?}" + ); + free.set(64 << 20); + assert_eq!(count(&a, "t").await, 0); + + // 3. The owner's budget. + let over_budget = run( + &a, + batch(&[ + "INSERT INTO t (a) VALUES ('first')", + "INSERT INTO t (a) VALUES (hex(zeroblob(40000)))", + ]), + GrowthPolicy::Budget(8192), + ) + .await; + assert!( + matches!(over_budget, Err(SqlError::StorageWouldGrow)), + "{over_budget:?}" + ); + assert_eq!(count(&a, "t").await, 0); + assert_eq!(a.checkpoint().await.unwrap(), baseline_checkpoint); + + // 4. A statement error after earlier statements. + let broken = run( + &a, + batch(&[ + "INSERT INTO t (a) VALUES ('first')", + "INSERT INTO nope VALUES (1)", + ]), + GrowthPolicy::Allow, + ) + .await; + assert!(broken.is_err()); + assert_eq!(count(&a, "t").await, 0); + assert_eq!(a.checkpoint().await.unwrap(), baseline_checkpoint); + + // And the same batch passes when nothing is wrong. + ok(&a, "INSERT INTO t (a) VALUES ('kept')").await; + assert_eq!(count(&a, "t").await, 1); +} + +/// TC-1585: a `WITH … INSERT` is a query to the parser but a write to the +/// node. It gets the transaction, the owner's budget, the size cap and the +/// pre-commit disk gate, and a refusal leaves nothing: no rows, no checkpoint +/// change, no reservation. +#[tokio::test] +async fn a_nested_write_is_bounded_before_commit_like_any_other_write() { + const CTE_WRITE: &str = + "WITH c AS (SELECT hex(zeroblob(40000)) AS v) INSERT INTO t (a) SELECT v FROM c"; + let query = |sql: &str| SqlRequest::Query { + sql: sql.into(), + params: vec![], + max_rows: None, + max_bytes: None, + }; + let dir = tempfile::tempdir().unwrap(); + let free = FakeFree::new(64 << 20); + let policy = DurabilityPolicy { + // 32 pages of 4 KiB. + max_database_bytes: 128 * 1024, + min_free_bytes: 1, + free_space: free.clone(), + ..Default::default() + }; + let a = actor(dir.path(), DB, policy.clone()); + for statement in AMPLIFIED_SCHEMA { + ok(&a, statement).await; + } + let baseline_checkpoint = a.checkpoint().await.unwrap(); + let wal_before = wal_len(dir.path(), DB); + + // The owner's budget. + let over_budget = run(&a, query(CTE_WRITE), GrowthPolicy::Budget(8192)).await; + assert!( + matches!(over_budget, Err(SqlError::StorageWouldGrow)), + "{over_budget:?}" + ); + // The per-database cap. + let over_cap = run( + &a, + query("WITH c AS (SELECT hex(zeroblob(200000)) AS v) INSERT INTO t (a) SELECT v FROM c"), + GrowthPolicy::Allow, + ) + .await; + assert!( + matches!(over_cap, Err(SqlError::DatabaseSizeLimit)), + "{over_cap:?}" + ); + // The disk gate, at the pre-commit check. + free.set(1 << 10); + let no_room = run( + &a, + query("WITH c AS (SELECT hex(zeroblob(2000)) AS v) INSERT INTO t (a) SELECT v FROM c"), + GrowthPolicy::Allow, + ) + .await; + assert!( + matches!(no_room, Err(SqlError::DiskHeadroom)), + "{no_room:?}" + ); + free.set(64 << 20); + + assert_eq!(count(&a, "t").await, 0); + assert_eq!(a.checkpoint().await.unwrap(), baseline_checkpoint); + assert_eq!(wal_len(dir.path(), DB), wal_before); + assert_eq!(policy.budget.reserved(), 0); + + // The same write is kept when it fits. + let fits = run( + &a, + query("WITH c AS (SELECT 'small' AS v) INSERT INTO t (a) SELECT v FROM c"), + GrowthPolicy::Budget(1 << 20), + ) + .await; + assert!(fits.is_ok(), "{fits:?}"); + assert_eq!(count(&a, "t").await, 1); +} + +#[tokio::test] +async fn the_size_cap_and_owner_budget_are_enforced_by_sqlite_and_shrinking_writes_pass() { + let dir = tempfile::tempdir().unwrap(); + let free = FakeFree::new(64 << 20); + let policy = DurabilityPolicy { + max_database_bytes: 64 * 4096, + min_free_bytes: 1, + free_space: free.clone(), + ..Default::default() + }; + let a = actor(dir.path(), DB, policy); + ok(&a, "CREATE TABLE t (id INTEGER PRIMARY KEY, v BLOB)").await; + + // Per-database cap: pages grow until the 64th, then the write is refused + // whole, whatever its size. + let mut stored = 0; + loop { + let result = run( + &a, + exec("INSERT INTO t (v) VALUES (zeroblob(30000))"), + GrowthPolicy::Allow, + ) + .await; + match result { + Ok(_) => stored += 1, + Err(SqlError::DatabaseSizeLimit) => break, + Err(other) => panic!("{other:?}"), + } + assert!(stored < 64, "the cap never bit"); + } + assert!(stored >= 5, "stored only {stored}"); + assert_eq!(count(&a, "t").await, stored); + let size = a.export().await.unwrap().len(); + assert!(size <= 64 * 4096, "database grew to {size}"); + + // At the cap, a write that frees pages still passes. + ok(&a, "DELETE FROM t WHERE id <= 2").await; + assert_eq!(count(&a, "t").await, stored - 2); + + // Owner budget: one page left means a write needing several is refused, + // a small one that fits in free pages is not. + let refused = run( + &a, + exec("INSERT INTO t (v) VALUES (zeroblob(100000))"), + GrowthPolicy::Budget(4096), + ) + .await; + assert!( + matches!(refused, Err(SqlError::StorageWouldGrow)), + "{refused:?}" + ); + let small = run( + &a, + exec("INSERT INTO t (v) VALUES (zeroblob(100))"), + GrowthPolicy::Budget(4096), + ) + .await; + assert!(small.is_ok(), "{small:?}"); + // A delete under a budget never needs a page, and it deletes: the rows + // above id 3 go, those up to it stay. + assert!(count(&a, "t").await > 1); + let shrink = run( + &a, + exec("DELETE FROM t WHERE id > 3"), + GrowthPolicy::Budget(1), + ) + .await; + assert!(shrink.is_ok(), "{shrink:?}"); + assert_eq!(count(&a, "t").await, 1); +} + +/// The shipped defaults are off: no cap, no headroom check, no reservation +/// (the limits are Sam's decision, card `sql-storage-report-read`). `synchronous` +/// is the durability fix, not a limit, and defaults to FULL. +#[tokio::test] +async fn the_limits_are_off_by_default_but_the_same_policy_enforces_when_set() { + let defaults = DurabilityPolicy::default(); + assert_eq!(defaults.max_database_bytes, 0); + assert_eq!(defaults.min_free_bytes, 0); + assert_eq!(defaults.synchronous, Synchronous::Full); + + let dir = tempfile::tempdir().unwrap(); + // A disk that reports no free space at all, and a blob bigger than any cap + // below: with the defaults nothing refuses and nothing is reserved. + let free = FakeFree::new(0); + let off = DurabilityPolicy { + free_space: free.clone(), + ..Default::default() + }; + let a = actor(dir.path(), "off", off.clone()); + ok(&a, "CREATE TABLE t (id INTEGER PRIMARY KEY, v BLOB)").await; + ok(&a, "INSERT INTO t (v) VALUES (zeroblob(3000000))").await; + assert_eq!(count(&a, "t").await, 1); + assert_eq!(off.budget.reserved(), 0, "nothing is reserved when off"); + + // The same writes under explicit limits are refused. + let on = DurabilityPolicy { + max_database_bytes: 1 << 20, + min_free_bytes: 1, + free_space: free.clone(), + ..Default::default() + }; + let b = actor(dir.path(), "on", on.clone()); + free.set(1 << 30); + ok(&b, "CREATE TABLE t (id INTEGER PRIMARY KEY, v BLOB)").await; + let capped = run( + &b, + exec("INSERT INTO t (v) VALUES (zeroblob(3000000))"), + GrowthPolicy::Allow, + ) + .await; + assert!( + matches!(capped, Err(SqlError::DatabaseSizeLimit)), + "{capped:?}" + ); + free.set(0); + let no_room = run( + &b, + exec("INSERT INTO t (v) VALUES (zeroblob(10))"), + GrowthPolicy::Allow, + ) + .await; + assert!( + matches!(no_room, Err(SqlError::DiskHeadroom)), + "{no_room:?}" + ); +} + +#[tokio::test] +async fn disk_headroom_exhaustion_refuses_writes_and_recovers_without_residue() { + let dir = tempfile::tempdir().unwrap(); + let free = FakeFree::new(64 << 20); + let min_free = 8 << 20; + let a = actor(dir.path(), DB, policy_with(&free, min_free)); + ok(&a, "CREATE TABLE t (v TEXT)").await; + + // The floor alone refuses: free space below it, whatever the write. + free.set(min_free - 1); + let refused = run(&a, exec("INSERT INTO t VALUES ('x')"), GrowthPolicy::Allow).await; + assert!( + matches!(refused, Err(SqlError::DiskHeadroom)), + "{refused:?}" + ); + // Reads and writes that touch nothing still answer. + assert_eq!(count(&a, "t").await, 0); + + free.set(64 << 20); + ok(&a, "INSERT INTO t VALUES ('x')").await; + assert_eq!(count(&a, "t").await, 1); +} + +#[tokio::test] +async fn concurrent_databases_share_one_reservation_budget() { + let dir = tempfile::tempdir().unwrap(); + let free = FakeFree::new(64 << 20); + let policy = policy_with(&free, 1); + let a = actor(dir.path(), "a", policy.clone()); + let b = actor(dir.path(), "b", policy.clone()); + ok(&a, "CREATE TABLE t (id INTEGER PRIMARY KEY, v BLOB)").await; + ok(&b, "CREATE TABLE t (id INTEGER PRIMARY KEY, v BLOB)").await; + + // `a` commits a large write. Its checkpoint growth stays held until a + // checkpoint runs, not only for the length of the request. + ok(&a, "INSERT INTO t (v) VALUES (zeroblob(400000))").await; + let held = policy.budget.reserved(); + assert!(held >= 390_000, "held {held}"); + assert_eq!(policy.budget.reserved_excluding("space/a"), 0); + // A later, small commit of `a` does not forget the growth of the earlier + // one: what is held follows the file, not the request. + ok(&a, "INSERT INTO t (v) VALUES (zeroblob(10))").await; + let held = policy.budget.reserved(); + assert!(held >= 390_000, "held {held}"); + + // 300 KB free: `b`'s small write fits alone, not next to `a`'s 400 KB. + free.set(300_000); + let refused = run( + &b, + exec("INSERT INTO t (v) VALUES (zeroblob(100))"), + GrowthPolicy::Allow, + ) + .await; + assert!( + matches!(refused, Err(SqlError::DiskHeadroom)), + "{refused:?}" + ); + assert_eq!(count(&b, "t").await, 0); + assert_eq!(policy.budget.reserved_excluding("space/a"), 0); + + // `a`'s checkpoint puts that growth on the disk: nothing of it is held. + a.checkpoint().await.unwrap(); + assert_eq!(policy.budget.reserved(), 0); + ok(&b, "INSERT INTO t (v) VALUES (zeroblob(100))").await; + assert_eq!(count(&b, "t").await, 1); + // A write that adds no page leaves nothing held, refused or done. + assert_eq!(policy.budget.reserved(), 0); + + // The reservation also ends with the database's connection. + free.set(64 << 20); + ok(&a, "INSERT INTO t (v) VALUES (zeroblob(400000))").await; + assert!(policy.budget.reserved() >= 390_000); + a.shutdown().await.unwrap(); + assert_eq!(policy.budget.reserved(), 0); +} + +#[tokio::test] +async fn a_database_over_the_cap_still_runs_writes_that_do_not_grow_it() { + let dir = tempfile::tempdir().unwrap(); + let free = FakeFree::new(64 << 20); + let big = actor(dir.path(), DB, policy_with(&free, 1)); + ok(&big, "CREATE TABLE t (id INTEGER PRIMARY KEY, v BLOB)").await; + for _ in 0..10 { + ok(&big, "INSERT INTO t (v) VALUES (zeroblob(30000))").await; + } + big.shutdown().await.unwrap(); + // The cap is now below the file's size. + let small_cap = DurabilityPolicy { + max_database_bytes: 4096, + ..policy_with(&free, 1) + }; + let a = actor(dir.path(), DB, small_cap); + ok(&a, "UPDATE t SET v = zeroblob(10) WHERE id = 1").await; + ok(&a, "DELETE FROM t WHERE id = 2").await; + let grow = run( + &a, + exec("INSERT INTO t (v) VALUES (zeroblob(300000))"), + GrowthPolicy::Allow, + ) + .await; + assert!(matches!(grow, Err(SqlError::DatabaseSizeLimit)), "{grow:?}"); +} + +/// A `SQLITE_FULL` that comes from a full disk is reported as the disk, in the +/// default configuration too (`min_free_bytes` = 0): never as the owner's +/// quota or the database's size limit. The free-space reading is the fake one; +/// a real ENOSPC cannot be provoked here. +#[tokio::test] +async fn a_full_disk_is_not_reported_as_the_owners_quota_or_the_size_limit() { + let dir = tempfile::tempdir().unwrap(); + let free = FakeFree::new(64 << 20); + type Is = fn(&SqlError) -> bool; + let cases: [(&str, u64, GrowthPolicy, Is); 2] = [ + // One page of the owner's budget: the write is over it. + ("budget", 0, GrowthPolicy::Budget(4096), |error| { + matches!(error, SqlError::StorageWouldGrow) + }), + ("cap", 64 * 4096, GrowthPolicy::Allow, |error| { + matches!(error, SqlError::DatabaseSizeLimit) + }), + ]; + for (name, max_database_bytes, growth, is_the_limit) in cases { + let policy = DurabilityPolicy { + max_database_bytes, + free_space: free.clone(), + ..Default::default() + }; + assert_eq!(policy.min_free_bytes, 0); + let a = actor(dir.path(), name, policy.clone()); + ok(&a, "CREATE TABLE t (id INTEGER PRIMARY KEY, v BLOB)").await; + let SqlValue::Integer(page_size) = rows(&a, "PRAGMA page_size").await[0][0] else { + panic!("page_size"); + }; + let floor = 16 * page_size as u64; + let too_big = "INSERT INTO t (v) VALUES (zeroblob(500000))"; + + // Room on the disk: the limit that fired is the one reported. + free.set(64 << 20); + let refused = run(&a, exec(too_big), growth).await.unwrap_err(); + assert!(is_the_limit(&refused), "{name}: {refused:?}"); + + // No room (below a few pages): the disk, for the same write. + for left in [0, floor - 1] { + free.set(left); + let refused = run(&a, exec(too_big), growth).await; + assert!( + matches!(refused, Err(SqlError::DiskHeadroom)), + "{name}, {left} bytes free: {refused:?}" + ); + } + // Over the floor it is a limit again. + free.set(floor); + let refused = run(&a, exec(too_big), growth).await.unwrap_err(); + assert!(is_the_limit(&refused), "{name}: {refused:?}"); + assert_eq!(count(&a, "t").await, 0); + assert_eq!(policy.budget.reserved(), 0); + } +} + +/// A request that wrote nothing needs no room: with the disk below its floor, +/// a wrapped request that only reads answers (the pre-commit gate and the +/// progress handler both let it through), a request that writes does not. +/// (`batch` runs its statements without reading their rows, so the reads here +/// are the ones a wrapped request can carry: `schema` entries of an `execute`, +/// a named statement, a `PRAGMA` report.) +#[tokio::test] +async fn reads_in_a_wrapped_request_are_neither_gated_nor_reserved() { + const SCAN: &str = "SELECT 1 FROM seed s1, seed s2 WHERE s1.x + s2.x < 0"; + let dir = tempfile::tempdir().unwrap(); + let free = FakeFree::new(64 << 20); + let min_free = 8 << 20; + let policy = policy_with(&free, min_free); + let a = actor(dir.path(), DB, policy.clone()); + ok(&a, "CREATE TABLE t (v TEXT)").await; + seed(&a).await; + let checkpoint_before = a.checkpoint().await.unwrap(); + let reads_then = |sql: &str| SqlRequest::Execute { + // The scan crosses several progress-handler intervals. A DROP of a + // table that is not there takes no write transaction. + schema: Some(vec![SCAN.into(), "SELECT count(*) FROM t".into()]), + sql: sql.into(), + params: vec![], + }; + let named = Some(tinycloud_core::sql::SqlCaveats { + statements: Some(vec![tinycloud_core::sql::caveats::PreparedStatement { + name: "scan".into(), + sql: SCAN.into(), + }]), + ..Default::default() + }); + + free.set(min_free - 1); + for growth in [GrowthPolicy::Allow, GrowthPolicy::Budget(1 << 20)] { + let reads = run(&a, reads_then("DROP TABLE IF EXISTS not_there"), growth).await; + assert!(reads.is_ok(), "{reads:?}"); + } + let named_read = a + .execute( + SqlRequest::ExecuteStatement { + name: "scan".into(), + params: vec![], + }, + named, + "tinycloud.sql/admin".into(), + GrowthPolicy::Allow, + ) + .await; + assert!(named_read.is_ok(), "{named_read:?}"); + // A report of the schema is wrapped (it can carry a PRAGMA) and reads. + assert!(!rows(&a, "PRAGMA table_info(t)").await.is_empty()); + assert_eq!(policy.budget.reserved(), 0); + + // A write is refused on the same disk: after the reads, before them, or + // in the middle of a long scan. + for request in [ + reads_then("INSERT INTO t VALUES ('x')"), + SqlRequest::Execute { + schema: Some(vec!["INSERT INTO t VALUES ('x')".into(), SCAN.into()]), + sql: "DROP TABLE IF EXISTS not_there".into(), + params: vec![], + }, + exec("INSERT INTO t VALUES ('x')"), + ] { + let refused = run(&a, request, GrowthPolicy::Allow).await; + assert!( + matches!(refused, Err(SqlError::DiskHeadroom)), + "{refused:?}" + ); + } + assert_eq!(count(&a, "t").await, 0); + assert_eq!(a.checkpoint().await.unwrap(), checkpoint_before); + assert_eq!(policy.budget.reserved(), 0); +} + +/// A request that carries a `PRAGMA` (admin only) is limited, gated and +/// reserved like any write: the owner's budget, the size cap and the disk +/// gate refuse it, and a refusal leaves nothing, the header value included. +#[tokio::test] +async fn a_request_with_a_pragma_is_bounded_like_any_write() { + let dir = tempfile::tempdir().unwrap(); + let free = FakeFree::new(64 << 20); + let policy = DurabilityPolicy { + // 32 pages of 4 KiB. + max_database_bytes: 128 * 1024, + min_free_bytes: 1, + free_space: free.clone(), + ..Default::default() + }; + let a = actor(dir.path(), DB, policy.clone()); + for statement in AMPLIFIED_SCHEMA { + ok(&a, statement).await; + } + let checkpoint_before = a.checkpoint().await.unwrap(); + let big = "INSERT INTO t (a) VALUES (hex(zeroblob(200000)))"; + let small = "INSERT INTO t (a) VALUES (hex(zeroblob(2000)))"; + let with_pragma = |statement: &str| batch(&["PRAGMA user_version = 3", statement]); + + let over_cap = run(&a, with_pragma(big), GrowthPolicy::Allow).await; + assert!( + matches!(over_cap, Err(SqlError::DatabaseSizeLimit)), + "{over_cap:?}" + ); + let over_budget = run(&a, with_pragma(big), GrowthPolicy::Budget(8192)).await; + assert!( + matches!(over_budget, Err(SqlError::StorageWouldGrow)), + "{over_budget:?}" + ); + free.set(1 << 10); + let no_room = run(&a, with_pragma(small), GrowthPolicy::Allow).await; + assert!( + matches!(no_room, Err(SqlError::DiskHeadroom)), + "{no_room:?}" + ); + // A `query` that sets the header value is a write to the gate as well. + let query_no_room = run( + &a, + SqlRequest::Query { + sql: "PRAGMA user_version = 5".into(), + params: vec![], + max_rows: None, + max_bytes: None, + }, + GrowthPolicy::Allow, + ) + .await; + assert!( + matches!(query_no_room, Err(SqlError::DiskHeadroom)), + "{query_no_room:?}" + ); + free.set(64 << 20); + + assert_eq!(count(&a, "t").await, 0); + assert_eq!( + rows(&a, "PRAGMA user_version").await[0][0], + SqlValue::Integer(0) + ); + assert_eq!(a.checkpoint().await.unwrap(), checkpoint_before); + assert_eq!(policy.budget.reserved(), 0); + + // The same request is kept when it fits. + run(&a, with_pragma(small), GrowthPolicy::Budget(1 << 20)) + .await + .unwrap(); + assert_eq!(count(&a, "t").await, 1); + assert_eq!( + rows(&a, "PRAGMA user_version").await[0][0], + SqlValue::Integer(3) + ); +} + +/// The size limit is enforced by SQLite while a statement runs +/// (`max_page_count`), not only by the check before COMMIT: a statement far +/// bigger than the cap stops at the cap, so the WAL it leaves behind is +/// bounded by the cap and not by the statement. +#[tokio::test] +async fn the_size_cap_stops_a_statement_while_it_runs() { + let dir = tempfile::tempdir().unwrap(); + let free = FakeFree::new(64 << 20); + let cap = 128 * 1024; + let policy = DurabilityPolicy { + max_database_bytes: cap, + min_free_bytes: 1, + free_space: free.clone(), + ..Default::default() + }; + let a = actor(dir.path(), DB, policy); + ok(&a, "CREATE TABLE t (id INTEGER PRIMARY KEY, v BLOB)").await; + let wal_before = wal_len(dir.path(), DB); + + // ~10 MB of text, eighty times the cap. With the check at COMMIT alone, + // the transaction would spill all of it to the WAL first. + let result = run( + &a, + exec("INSERT INTO t (v) VALUES (hex(zeroblob(5000000)))"), + GrowthPolicy::Allow, + ) + .await; + assert!( + matches!(result, Err(SqlError::DatabaseSizeLimit)), + "{result:?}" + ); + let spilled = wal_len(dir.path(), DB) - wal_before; + assert!( + spilled <= 2 * cap, + "the failed statement left {spilled} bytes in the WAL" + ); + assert_eq!(count(&a, "t").await, 0); +} + +#[test] +fn directories_created_for_a_database_are_made_durable() { + let dir = tempfile::tempdir().unwrap(); + let nested = dir.path().join("sql").join("space").join("deeper"); + create_dir_all_durable(&nested).unwrap(); + assert!(nested.is_dir()); + // Idempotent on an existing path. + create_dir_all_durable(&nested).unwrap(); +} + +/// The bound the gate relies on: whatever a transaction dirtied, COMMIT +/// appends no more to the WAL than one frame per page left in the page cache. +#[test] +fn the_unspilled_wal_bound_covers_what_commit_appends() { + use tinycloud_core::sql::durability::{cache_used_bytes, unspilled_wal_bound}; + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("bound.db"); + let conn = open_connection(&StorageMode::File(path.clone()), Synchronous::Full).unwrap(); + let wal = path.with_extension("db-wal"); + let wal_len = || std::fs::metadata(&wal).map(|m| m.len()).unwrap_or(0); + let page_size: u64 = conn + .query_row("PRAGMA page_size", [], |r| r.get(0)) + .unwrap(); + conn.execute_batch( + "CREATE TABLE t (id INTEGER PRIMARY KEY, a, b, c, d); \ + CREATE INDEX ta ON t (a); CREATE INDEX tb ON t (b); \ + CREATE INDEX tc ON t (c); CREATE INDEX td ON t (d); \ + CREATE TABLE seed (x); INSERT INTO seed VALUES (1);", + ) + .unwrap(); + for _ in 0..9 { + conn.execute_batch("INSERT INTO seed SELECT x FROM seed") + .unwrap(); + } + let mut observed_spill = false; + for rows in [1, 50, 1_000, 10_000, 100_000] { + conn.execute_batch("SAVEPOINT s").unwrap(); + let at_savepoint = wal_len(); + conn.execute( + "INSERT INTO t (a, b, c, d) SELECT hex(randomblob(10)), hex(randomblob(10)), \ + hex(randomblob(10)), hex(randomblob(10)) FROM seed s1, seed s2 LIMIT ?1", + [rows], + ) + .unwrap(); + // Rewriting existing pages counts too. + conn.execute("UPDATE t SET a = hex(randomblob(10)) WHERE id % 2 = 0", []) + .unwrap(); + let before = wal_len(); + let bound = unspilled_wal_bound(cache_used_bytes(&conn), page_size); + observed_spill |= before > at_savepoint; + conn.execute_batch("RELEASE s").unwrap(); + let appended = wal_len() - before; + assert!( + appended <= bound, + "{rows} rows: COMMIT appended {appended} bytes, bound was {bound}" + ); + } + assert!( + observed_spill, + "the cache never spilled; the test proved little" + ); +} + +/// Indicative only (not a benchmark gate): p50/p99 of a small committed write +/// with `synchronous` NORMAL and FULL. Run with +/// `TC1491_LATENCY_DIR= cargo test -p tinycloud-core +/// --test sql_local_durability latency -- --ignored --nocapture`. +#[test] +#[ignore] +fn synchronous_latency_indicative() { + let base = std::env::var("TC1491_LATENCY_DIR").expect("TC1491_LATENCY_DIR"); + for round in 0..2 { + for setting in [Synchronous::Normal, Synchronous::Full] { + let dir = tempfile::tempdir_in(&base).unwrap(); + let conn = + open_connection(&StorageMode::File(dir.path().join("lat.db")), setting).unwrap(); + conn.execute_batch("CREATE TABLE t (id INTEGER PRIMARY KEY, v TEXT)") + .unwrap(); + let mut micros = Vec::with_capacity(1000); + for i in 0..1000 { + let start = std::time::Instant::now(); + conn.execute("INSERT INTO t (v) VALUES (?1)", [format!("value-{i}")]) + .unwrap(); + micros.push(start.elapsed().as_micros() as u64); + } + micros.sort_unstable(); + println!( + "round {round} {setting:?}: p50 {} us, p99 {} us, max {} us", + micros[499], micros[989], micros[999] + ); + } + } +} + +// ---- kill -9 --------------------------------------------------------------- + +/// Where the child process dies. +#[derive(Clone, Copy, Debug)] +enum Kill { + BeforeCommit, + AfterCommit, + BeforeResponse, +} + +impl Kill { + fn name(self) -> &'static str { + match self { + Kill::BeforeCommit => "before_commit", + Kill::AfterCommit => "after_commit", + Kill::BeforeResponse => "before_response", + } + } +} + +const CHILD_ENV: &str = "TC1491_KILL_CHILD"; +/// Request number (1-based, counted over commits) at which the child dies. +const KILL_AT: u64 = 6; + +fn sigkill_self() -> ! { + // SAFETY: plain syscall on our own pid. + unsafe { libc::kill(libc::getpid(), libc::SIGKILL) }; + std::process::abort() +} + +/// Entry point of the child process; a no-op test in the normal run. +#[test] +fn kill_child_entry() { + let Ok(spec) = std::env::var(CHILD_ENV) else { + return; + }; + let mut parts = spec.split('|'); + let dir = PathBuf::from(parts.next().unwrap()); + let kill = parts.next().unwrap().to_string(); + + // Commits are counted when their `after_commit` seam runs (the setup + // CREATE TABLE is the first). The child dies in the sixth. + let commits = Arc::new(AtomicU64::new(0)); + let fresh = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let seam = |f: Box| Some(Arc::new(f) as Arc); + let before_commit = { + let (commits, kill) = (commits.clone(), kill.clone()); + seam(Box::new(move || { + if kill == "before_commit" && commits.load(Ordering::SeqCst) + 1 == KILL_AT { + sigkill_self(); + } + })) + }; + let after_commit = { + let (commits, fresh, kill) = (commits.clone(), fresh.clone(), kill.clone()); + seam(Box::new(move || { + let n = commits.fetch_add(1, Ordering::SeqCst) + 1; + fresh.store(true, Ordering::SeqCst); + if kill == "after_commit" && n == KILL_AT { + sigkill_self(); + } + })) + }; + let before_response = { + let (commits, fresh, kill) = (commits.clone(), fresh.clone(), kill.clone()); + seam(Box::new(move || { + // Only the response that follows a commit; rejected requests and + // reads answer without one. + if fresh.swap(false, Ordering::SeqCst) + && kill == "before_response" + && commits.load(Ordering::SeqCst) == KILL_AT + { + sigkill_self(); + } + })) + }; + let policy = DurabilityPolicy { + seams: CommitSeams { + before_commit, + after_commit, + before_response, + }, + ..Default::default() + }; + + let runtime = tokio::runtime::Builder::new_multi_thread() + .enable_all() + .build() + .unwrap(); + runtime.block_on(async { + let a = actor(&dir, DB, policy); + run( + &a, + exec("CREATE TABLE t (id INTEGER PRIMARY KEY, v INTEGER NOT NULL)"), + GrowthPolicy::Allow, + ) + .await + .unwrap(); + println!("SETUP"); + for i in 1..=20_i64 { + // Every third request is rejected after its first statement ran. + let request = if i % 3 == 0 { + batch(&[ + &format!("INSERT INTO t (id, v) VALUES ({}, {})", 1000 + i, 1000 + i), + "INSERT INTO t (id, v) VALUES (NULL, NULL)", + ]) + } else { + exec(&format!("INSERT INTO t (id, v) VALUES ({i}, {i})")) + }; + let result = run(&a, request, GrowthPolicy::Allow).await; + match result { + Ok(_) => println!("ACK {i}"), + Err(_) => println!("REJECTED {i}"), + } + } + println!("DONE"); + }); +} + +fn run_child(dir: &Path, kill: Kill) -> Vec { + let mut child = Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "kill_child_entry", + "--nocapture", + "--test-threads=1", + ]) + .env(CHILD_ENV, format!("{}|{}", dir.display(), kill.name())) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .spawn() + .unwrap(); + let lines: Vec = BufReader::new(child.stdout.take().unwrap()) + .lines() + .map_while(Result::ok) + .collect(); + let status = child.wait().unwrap(); + use std::os::unix::process::ExitStatusExt; + assert_eq!( + status.signal(), + Some(libc::SIGKILL), + "the child must die by SIGKILL: {status:?} {lines:?}" + ); + lines +} + +#[tokio::test(flavor = "multi_thread")] +async fn kill_minus_nine_at_each_commit_boundary_loses_no_acknowledged_write_and_keeps_no_rejected_one( +) { + for kill in [Kill::BeforeCommit, Kill::AfterCommit, Kill::BeforeResponse] { + let dir = tempfile::tempdir().unwrap(); + let lines = run_child(dir.path(), kill); + assert!( + lines.iter().any(|line| line.ends_with("SETUP")), + "{kill:?} {lines:?}" + ); + let acked: Vec = lines + .iter() + .filter_map(|l| l.strip_prefix("ACK ")) + .map(|n| n.parse().unwrap()) + .collect(); + let rejected: Vec = lines + .iter() + .filter_map(|l| l.strip_prefix("REJECTED ")) + .map(|n| n.parse().unwrap()) + .collect(); + assert!(!lines.contains(&"DONE".to_string()), "{kill:?}"); + assert!( + !rejected.is_empty() && acked.len() >= 3, + "{kill:?} {lines:?}" + ); + + // Reopen what the dead process left. + let a = actor(dir.path(), DB, DurabilityPolicy::default()); + let present: Vec = rows(&a, "SELECT id FROM t ORDER BY id") + .await + .into_iter() + .map(|row| match row[0] { + SqlValue::Integer(n) => n, + ref other => panic!("{other:?}"), + }) + .collect(); + for id in &acked { + assert!(present.contains(id), "{kill:?}: acknowledged {id} is lost"); + } + for id in &rejected { + assert!( + !present.contains(id) && !present.contains(&(1000 + id)), + "{kill:?}: rejected {id} (or its batch prefix) survived" + ); + } + // Whatever else is there is the single request in flight when the + // process died: committed locally, never acknowledged ("unknown"). + let unexplained: Vec<_> = present.iter().filter(|id| !acked.contains(id)).collect(); + match kill { + Kill::BeforeCommit => assert!(unexplained.is_empty(), "{kill:?} {unexplained:?}"), + Kill::AfterCommit | Kill::BeforeResponse => { + assert_eq!(unexplained.len(), 1, "{kill:?} {unexplained:?}") + } + } + } +} diff --git a/tinycloud-node-server/src/config.rs b/tinycloud-node-server/src/config.rs index 8e2c2dc2..a4d0ee93 100644 --- a/tinycloud-node-server/src/config.rs +++ b/tinycloud-node-server/src/config.rs @@ -1163,6 +1163,24 @@ pub struct SqlStorageConfig { /// callers that gave up on it, so the database still warms. #[serde(default = "default_sql_hydration_max_ms")] pub hydration_max_ms: u64, + /// `PRAGMA synchronous` of file-backed databases (TC-1491): `full` makes + /// every local COMMIT fsync the WAL before it returns, `normal` fsyncs + /// only at checkpoints. `normal` is the rollback flag. + #[serde(default)] + pub synchronous: tinycloud_core::sql::durability::Synchronous, + /// Per-database size cap (TC-1491). A write that would grow a database past + /// it is rolled back before COMMIT (`507 sql_database_size_limit`). + /// **0 (the default) = no cap.** The limit is Sam's decision (card + /// `sql-storage-report-read`); set it by config once he answers. + #[serde(default = "default_sql_max_database_bytes")] + pub max_database_bytes: ByteUnit, + /// Free disk that must remain after the worst-case growth of every write + /// in flight (TC-1491); a write that would eat into it is rolled back + /// before COMMIT (`507 sql_disk_headroom`). **0 (the default) = the check + /// is off** (no reservation, no free-space reads). The limit is Sam's + /// decision (card `sql-storage-report-read`). + #[serde(default = "default_sql_min_free_bytes")] + pub min_free_bytes: ByteUnit, /// Whether a schema-changing statement (CREATE, ALTER, DROP of tables, /// views and indexes) needs `tinycloud.sql/schema` (TC-1581). `sql/write` /// covers data statements only. `"enforce"` (the default) refuses the @@ -1172,6 +1190,14 @@ pub struct SqlStorageConfig { pub ddl_requires_schema: DdlPolicy, } +fn default_sql_max_database_bytes() -> ByteUnit { + ByteUnit::Byte(0) +} + +fn default_sql_min_free_bytes() -> ByteUnit { + ByteUnit::Byte(0) +} + fn default_sql_memory_threshold() -> ByteUnit { ByteUnit::Mebibyte(10) } @@ -1193,6 +1219,16 @@ fn default_sql_hydration_max_ms() -> u64 { } impl SqlStorageConfig { + /// The local durability policy the SQL actors run under. + pub fn durability(&self) -> tinycloud_core::sql::durability::DurabilityPolicy { + tinycloud_core::sql::durability::DurabilityPolicy { + synchronous: self.synchronous, + max_database_bytes: self.max_database_bytes.as_u64(), + min_free_bytes: self.min_free_bytes.as_u64(), + ..Default::default() + } + } + pub fn wait_bounds(&self) -> tinycloud_core::sql::SqlWaitBounds { tinycloud_core::sql::SqlWaitBounds { lock_wait: Duration::from_millis(self.lock_wait_timeout_ms), @@ -1212,6 +1248,9 @@ impl Default for SqlStorageConfig { hydration_wait_timeout_ms: default_sql_hydration_wait_timeout_ms(), artifact_transfer_timeout_ms: default_sql_artifact_transfer_timeout_ms(), hydration_max_ms: default_sql_hydration_max_ms(), + synchronous: Default::default(), + max_database_bytes: default_sql_max_database_bytes(), + min_free_bytes: default_sql_min_free_bytes(), ddl_requires_schema: DdlPolicy::default(), } } diff --git a/tinycloud-node-server/src/lib.rs b/tinycloud-node-server/src/lib.rs index 4a65c225..a82bfa64 100644 --- a/tinycloud-node-server/src/lib.rs +++ b/tinycloud-node-server/src/lib.rs @@ -712,6 +712,7 @@ pub async fn app_with_control( database_artifact_repository.clone(), ) .with_wait_bounds(tinycloud_config.storage.sql.wait_bounds()) + .with_durability(tinycloud_config.storage.sql.durability()) .with_ddl_policy(tinycloud_config.storage.sql.ddl_requires_schema); let mut policy_v3_runtime = @@ -1924,18 +1925,29 @@ mod retry_after_fairing_tests { #[async_trait::async_trait] impl tinycloud_core::database_artifacts::DatabaseArtifactRepository for StalledArtifacts { - async fn load( + async fn load_meta( &self, _: &str, _: &str, _: &str, ) -> Result< - Option, + Option, tinycloud_core::database_artifacts::DatabaseArtifactError, > { std::future::pending().await } + async fn read_chunks( + &self, + _: &str, + _: &str, + _: &str, + _: &tinycloud_core::database_artifacts::ArtifactMeta, + _: &mut dyn tinycloud_core::database_artifacts::ArtifactSink, + ) -> Result<(), tinycloud_core::database_artifacts::DatabaseArtifactError> { + std::future::pending().await + } + async fn save( &self, _: &str, diff --git a/tinycloud-node-server/src/main.rs b/tinycloud-node-server/src/main.rs index a4c3a17a..06fa8c67 100644 --- a/tinycloud-node-server/src/main.rs +++ b/tinycloud-node-server/src/main.rs @@ -241,6 +241,51 @@ mod tests { assert_eq!(extract().spaces_per_second, 7); } + #[test] + fn sql_durability_settings_default_and_take_env_overrides() { + use tinycloud_core::sql::durability::Synchronous; + let _lock = lock_env(); + let names = [ + "TINYCLOUD_STORAGE__SQL__SYNCHRONOUS", + "TINYCLOUD_STORAGE_SQL_SYNCHRONOUS", + "TINYCLOUD_STORAGE__SQL__MAX_DATABASE_BYTES", + "TINYCLOUD_STORAGE__SQL__MIN_FREE_BYTES", + ]; + let _unset: Vec<_> = names.iter().map(|name| EnvVarGuard::unset(name)).collect(); + let extract = || { + build_config_figment() + .extract::() + .expect("config should parse") + .storage + .sql + }; + + let sql = extract(); + assert_eq!(sql.synchronous, Synchronous::Full, "full by default"); + assert_eq!(sql.max_database_bytes.as_u64(), 0, "no cap by default"); + assert_eq!( + sql.min_free_bytes.as_u64(), + 0, + "headroom check off by default" + ); + let policy = sql.durability(); + assert_eq!(policy.synchronous, Synchronous::Full); + assert_eq!(policy.max_database_bytes, 0); + assert_eq!(policy.min_free_bytes, 0); + + let _canonical = EnvVarGuard::set("TINYCLOUD_STORAGE__SQL__SYNCHRONOUS", "normal"); + let _max = EnvVarGuard::set("TINYCLOUD_STORAGE__SQL__MAX_DATABASE_BYTES", "2 GiB"); + let _free = EnvVarGuard::set("TINYCLOUD_STORAGE__SQL__MIN_FREE_BYTES", "1 GiB"); + let sql = extract(); + assert_eq!(sql.synchronous, Synchronous::Normal); + assert_eq!(sql.max_database_bytes.as_u64(), 2 << 30); + assert_eq!(sql.min_free_bytes.as_u64(), 1 << 30); + drop(_canonical); + + let _legacy = EnvVarGuard::set("TINYCLOUD_STORAGE_SQL_SYNCHRONOUS", "normal"); + assert_eq!(extract().synchronous, Synchronous::Normal); + } + #[test] fn canonical_double_underscore_loads_storage_database() { let _lock = lock_env(); diff --git a/tinycloud-node-server/src/routes/mod.rs b/tinycloud-node-server/src/routes/mod.rs index e5810911..5592a99c 100644 --- a/tinycloud-node-server/src/routes/mod.rs +++ b/tinycloud-node-server/src/routes/mod.rs @@ -2719,13 +2719,16 @@ async fn handle_sql_invoke( // available when content storage is full. let grows_content = publication_command.is_none() && sql_request_is_write(&sql_request, &exec_caveats, ability); - let full_space = if grows_content { - storage_check(space, tinycloud, config, quota_cache) - .await? - .filter(|check| check.remaining().is_none()) + // One read, as before: it now also carries the budget a write may still + // use, so the service can refuse an overshooting write before COMMIT + // instead of one write late (TC-1491). No extra database round trip. + let storage = if grows_content { + storage_check(space, tinycloud, config, quota_cache).await? } else { None }; + let full_space = storage.filter(|check| check.remaining().is_none()); + let remaining_budget = storage.and_then(StorageCheck::remaining); let execute_start = Instant::now(); if publication_command .as_ref() @@ -2749,6 +2752,17 @@ async fn handle_sql_invoke( authority, ) .await + } else if let Some(remaining) = remaining_budget { + sql_service + .execute_with_budget_authorized( + space, + &db_name, + sql_request, + exec_caveats, + authority, + remaining, + ) + .await } else { sql_service .execute_authorized(space, &db_name, sql_request, exec_caveats, authority) @@ -2764,7 +2778,7 @@ async fn handle_sql_invoke( execute_start.elapsed(), ); let response = execute_result.map_err(|e| -> InvokeError { - match (&e, full_space) { + match (&e, storage) { (SqlError::StorageWouldGrow, Some(check)) => check .reject(StorageRejectionKind::QuotaExceeded, space) .into(), @@ -3141,11 +3155,15 @@ pub(crate) fn sql_error_to_status(err: &SqlError) -> Status { SqlError::ResponseTooLarge(_) => Status::new(413), SqlError::QuotaExceeded => Status::new(429), SqlError::StorageWouldGrow => Status::new(402), + // 507, not 413: the SQL client reads 413 as `SQL_RESPONSE_TOO_LARGE` + // and the KV and quota clients as an exhausted storage plan, and + // TC-1012's 413 is the request body cap. Neither is true here. + SqlError::DatabaseSizeLimit | SqlError::DiskHeadroom => Status::new(507), SqlError::InvalidStatement(_) => Status::BadRequest, SqlError::SchemaError(_) => Status::BadRequest, SqlError::ReadOnlyViolation => Status::Forbidden, SqlError::ParseError(_) => Status::BadRequest, - SqlError::Internal(_) => Status::InternalServerError, + SqlError::Internal(_) | SqlError::CommitFailed => Status::InternalServerError, SqlError::Unavailable => Status::ServiceUnavailable, SqlError::OutcomeUnknown => Status::InternalServerError, SqlError::Busy(_) | SqlError::RequestTimeout => Status::ServiceUnavailable, @@ -6545,6 +6563,33 @@ mod tests { // ---- T2: write-class SQL 402 gate --------------------------------- + #[test] + fn tc1491_reservation_refusals_are_507_with_distinct_codes() { + assert_eq!( + sql_error_to_status(&SqlError::DatabaseSizeLimit), + Status::new(507) + ); + assert_eq!( + sql_error_to_status(&SqlError::DiskHeadroom), + Status::new(507) + ); + assert_eq!( + sql_error_to_status(&SqlError::StorageWouldGrow), + Status::new(402) + ); + assert_eq!( + sql_error_to_status(&SqlError::CommitFailed), + Status::InternalServerError + ); + let size = SqlError::DatabaseSizeLimit.to_string(); + let disk = SqlError::DiskHeadroom.to_string(); + assert!(size.starts_with("sql_database_size_limit:"), "{size}"); + assert!(disk.starts_with("sql_disk_headroom:"), "{disk}"); + assert!(SqlError::CommitFailed + .to_string() + .starts_with("sql_commit_failed:")); + } + #[tokio::test] async fn sql_request_is_write_classifies_variants() { use tinycloud_core::sql::types::SqlStatement; @@ -7451,6 +7496,85 @@ mod tests { Ok(()) } + /// TC-1585 + TC-1491 over `/invoke`: a `WITH … INSERT` inside the owner's + /// remaining budget (not a full space) takes the budget path like a plain + /// write: a growth past the budget is refused before COMMIT with 402 and + /// changes nothing, a small one passes. + #[tokio::test] + async fn tc1585_cte_wrapped_write_over_the_remaining_budget_is_402_and_leaves_nothing( + ) -> Result<()> { + use rocket::data::ByteUnit; + use rocket::http::{ContentType, Header, Status}; + use rocket::local::asynchronous::Client; + + let setup = metered_sql_http_setup("tc1585-cte-budget").await?; + // Room for a page or two, not for 64 KiB. + let limit = ByteUnit::Byte(setup.used + 8 * 1024); + let mut nonce = 0u32; + let mut header = |ability: &str| -> Result { + nonce += 1; + sql_invocation_header(&setup, ability, &format!("urn:uuid:tc1585b-{nonce}")) + }; + let big = header("tinycloud.sql/write")?; + let small = header("tinycloud.sql/write")?; + let count = header("tinycloud.sql/read")?; + + let client = Client::tracked(metered_sql_rocket(setup, limit)).await?; + let send = |auth: &String, request: &SqlRequest| { + let client = &client; + let auth = auth.clone(); + let body = serde_json::to_string(request); + async move { + let response = client + .post("/invoke") + .header(Header::new("Authorization", auth)) + .header(ContentType::JSON) + .body(body.expect("request serializes")) + .dispatch() + .await; + let status = response.status(); + (status, response.into_string().await.unwrap_or_default()) + } + }; + let cte_insert = |params: Vec| SqlRequest::Execute { + sql: + "WITH c AS (SELECT ? AS body) INSERT INTO labels (label, val) SELECT body, 1 FROM c" + .to_string(), + params, + schema: None, + }; + + let (status, body) = send( + &big, + &cte_insert(vec![SqlValue::Text("g".repeat(64 * 1024))]), + ) + .await; + assert_eq!(status, Status::new(402), "{body}"); + assert!(body.contains("storage_quota_exceeded"), "{body}"); + + let (status, body) = send(&small, &cte_insert(vec![SqlValue::Text("ok".into())])).await; + assert_eq!(status, Status::Ok, "{body}"); + + // Only the small write is there: the seeded row plus one. + let (status, body) = send( + &count, + &SqlRequest::Query { + sql: "SELECT count(*) FROM labels".to_string(), + params: vec![], + max_rows: None, + max_bytes: None, + }, + ) + .await; + assert_eq!(status, Status::Ok, "{body}"); + assert_eq!( + serde_json::from_str::(&body)?["rows"], + serde_json::json!([[2]]), + "{body}" + ); + Ok(()) + } + /// TC-1585: a CTE-wrapped write reports the table it changes, so the route /// publishes the same write event as for a plain write (the events are /// built from the service's `write_targets`; before the fix these writes diff --git a/tinycloud-node-server/tests/postgres_pool_timeouts.rs b/tinycloud-node-server/tests/postgres_pool_timeouts.rs index d1a151fe..ccbfb965 100644 --- a/tinycloud-node-server/tests/postgres_pool_timeouts.rs +++ b/tinycloud-node-server/tests/postgres_pool_timeouts.rs @@ -48,7 +48,7 @@ use tinycloud_auth::resource::SpaceId; use tinycloud_auth::ssi::dids::DIDBuf; use tinycloud_auth::ssi::jwk::JWK; use tinycloud_core::database_artifacts::{ - DatabaseArtifactError, DatabaseArtifactRepository, SeaOrmDatabaseArtifactRepository, + DatabaseArtifactError, DatabaseArtifactRepository, MemorySink, SeaOrmDatabaseArtifactRepository, }; use tinycloud_core::db_pool::{is_transient_error, transient_error_class, StatementTimeoutMode}; use tinycloud_core::sea_orm::{ @@ -57,7 +57,6 @@ use tinycloud_core::sea_orm::{ }; use tinycloud_core::sql::{SqlError, SqlRequest, SqlResponse, SqlService, SqlValue}; use tinycloud_core::RecoveringConnection; -use tinycloud_core::{SizeTrackingArtifactRepository, SqlSizes}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::{TcpListener, TcpStream}; @@ -561,17 +560,16 @@ async fn postgres_large_artifacts_persist_under_default_timeouts() { schema.drop().await; } -/// A large hydration whose server-side work outlasts the base statement -/// timeout succeeds, because the size mirror marks the artifact large and -/// its load runs under the bulk timeout; the same load forced under the base -/// timeout is cancelled (Opus's M6). The artifact is read through a view -/// that sleeps 1 s per row, standing in for detoasting and decompressing a -/// large payload. (A slow network does not exercise this: the artifact is -/// one row, and PostgreSQL does not cancel a statement while it is writing -/// a row to the client.) +/// Hydration is a series of 4 MiB slice statements, each under the BASE +/// statement timeout (TC-822): a large artifact hydrates through a pool whose +/// base timeout is far below the whole-artifact transfer, and a slice whose +/// server-side work outlasts the base timeout is cancelled as a transient +/// error rather than holding the pool. The slow case reads the artifact +/// through a view that sleeps 1 s per row, standing in for detoasting a +/// large payload. #[tokio::test(flavor = "multi_thread", worker_threads = 4)] -async fn postgres_large_hydration_runs_under_the_bulk_timeout() { - let Some(url) = postgres_url("postgres_large_hydration_runs_under_the_bulk_timeout") else { +async fn postgres_hydration_slices_run_under_the_base_timeout() { + let Some(url) = postgres_url("postgres_hydration_slices_run_under_the_base_timeout") else { return; }; let schema = Schema::new(&url, "bulkload").await; @@ -585,42 +583,51 @@ async fn postgres_large_hydration_runs_under_the_bulk_timeout() { write(&seed, &space, "a", 4 * 1024 * 1024).await.unwrap(); } } - let name = &schema.name; - schema - .admin - .execute_unprepared(&format!( - "ALTER TABLE {name}.database_artifact RENAME TO database_artifact_rows; \ - CREATE FUNCTION {name}.slow_row() RETURNS boolean LANGUAGE plpgsql AS \ - $$ BEGIN PERFORM pg_sleep(1); RETURN true; END $$; \ - CREATE VIEW {name}.database_artifact AS \ - SELECT * FROM {name}.database_artifact_rows WHERE {name}.slow_row();" - )) - .await - .unwrap(); - let strict = pool(schema.url(&url, None), |config| { config.database.statement_timeout_ms = 500; config.database.bulk_statement_timeout_ms = 10_000; }) .await; - let sizes = SqlSizes::new(); - sizes.seed_from(&direct).await.unwrap(); let repository = Arc::new(SeaOrmDatabaseArtifactRepository::new(strict.clone())); let cache = tempfile::TempDir::new().unwrap(); let service = SqlService::new( cache.path().to_string_lossy().into_owned(), u64::MAX, - Arc::new(SizeTrackingArtifactRepository::new( - repository.clone(), - sizes, - )), + repository.clone(), + ); + assert_eq!( + count(&service, &space, "a").await.unwrap(), + 3, + "a multi-slice artifact hydrates under the 500 ms base timeout" ); - assert_eq!(count(&service, &space, "a").await.unwrap(), 3); + let meta = repository + .load_meta("sql", &space.to_string(), "a") + .await + .unwrap() + .expect("the artifact exists"); + let name = &schema.name; + schema + .admin + .execute_unprepared(&format!( + "ALTER TABLE {name}.database_artifact RENAME TO database_artifact_rows; \ + CREATE FUNCTION {name}.slow_row() RETURNS boolean LANGUAGE plpgsql AS \ + $$ BEGIN PERFORM pg_sleep(1); RETURN true; END $$; \ + CREATE VIEW {name}.database_artifact AS \ + SELECT * FROM {name}.database_artifact_rows WHERE {name}.slow_row();" + )) + .await + .unwrap(); let error = repository - .load_sized("sql", &space.to_string(), "a", Some(0)) + .read_chunks( + "sql", + &space.to_string(), + "a", + &meta, + &mut MemorySink::default(), + ) .await - .expect_err("the same load under the 500 ms base timeout is cancelled"); + .expect_err("a slice that outlasts the 500 ms base timeout is cancelled"); match &error { DatabaseArtifactError::Db(error) => { assert_eq!( @@ -991,19 +998,21 @@ async fn blackhole_mid_hydration_waits_for_one_detection_only() { assert!(elapsed < Duration::from_secs(6), "{elapsed:?}"); } -/// The case CI found: the hydration's `BEGIN` draws an idle connection that -/// died silently, and the load on the next connection is cut mid-transfer. -/// The attempt is retried as a whole, its `BEGIN` never on its own, so the -/// request waits for at most two detections, not three. +/// The case CI found: a hydration statement draws an idle connection that +/// died silently, and a later slice on the next connection is cut +/// mid-transfer. Hydration is a series of independently retried statements +/// (TC-822), so the request waits for at most three detections: the dead idle +/// connection (the metadata statement, retried once), then the cut slice and +/// its one retry. Never more: each statement retries once, within the window. #[tokio::test(flavor = "multi_thread", worker_threads = 4)] -async fn blackhole_dead_idle_connection_then_cut_hydration_waits_two_detections_at_most() { +async fn blackhole_dead_idle_connection_then_cut_hydration_waits_three_detections_at_most() { let Some(url) = blackhole_url( - "blackhole_dead_idle_connection_then_cut_hydration_waits_two_detections_at_most", + "blackhole_dead_idle_connection_then_cut_hydration_waits_three_detections_at_most", ) else { return; }; let (elapsed, _) = mid_hydration_blackhole_with(&url, "bhidlecut", |_| {}, true).await; - assert!(elapsed < Duration::from_secs(9), "{elapsed:?}"); + assert!(elapsed < Duration::from_secs(14), "{elapsed:?}"); } /// Black-hole every connection streaming db `a`'s hydration on a cold node, diff --git a/tinycloud.toml b/tinycloud.toml index 4f5a143b..b09c3a8a 100644 --- a/tinycloud.toml +++ b/tinycloud.toml @@ -26,6 +26,13 @@ cors = true # hydration_wait_timeout_ms = 30000 # artifact_transfer_timeout_ms = 45000 # hydration_max_ms = 600000 # a hydration keeps warming after its callers give up + ## Local commit durability and the checks before COMMIT (TC-1491). + ## synchronous: "full" fsyncs the WAL on every COMMIT; "normal" is the rollback flag. + ## The two limits are off by default (0 = off): they are Sam's decision (card + ## sql-storage-report-read); set them here once he answers. + # synchronous = "full" + # max_database_bytes = 0 # per database; over it: 507 sql_database_size_limit + # min_free_bytes = 0 # disk kept free; short of it: 507 sql_disk_headroom ## Schema-changing SQL (CREATE/ALTER/DROP of tables, views and indexes) needs ## tinycloud.sql/schema (TC-1581); tinycloud.sql/write covers data statements. ## "enforce" (default) refuses the statement, "log" runs it and records it