Skip to content

feat(sql): durable local commit and reservation before commit (TC-1491, 1a part B) - #386

Open
samgbafa wants to merge 29 commits into
mainfrom
tc1491/1a-local-durability
Open

samgbafa wants to merge 29 commits into
mainfrom
tc1491/1a-local-durability

Conversation

@samgbafa

@samgbafa samgbafa commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

TC-1491 slice 1a, part B: make the node's local SQL commit durable, and refuse before COMMIT a write that would overshoot. Postgres stays master. Zero migrations, no schema change, no store/manifest types, no uploader. Design: docs/sql-local-durability.md (in this PR).

Merge order and what to review

This branch contains three other PRs by merge commit. Merge order: #376, then #383, then #384 (part A), then this.

Review only c7cec06, 0cee4f4, 269986e and bf3ef73 (the last turns the two limits off by default). The merge commits carry main (1.21.4), #376, #383 and #384 (A, now 9df7eec). Everything else is already under review in the PRs above.

Limits are Sam's decision (card sql-storage-report-read); off by default

max_database_bytes and min_free_bytes default to 0 = off (no cap; no headroom check, no reservation, no free-space reads), because an unmeasured cap could refuse real users' databases. They are set by config once Sam answers. The mechanism (cap through max_page_count, the page-cache WAL bound, DiskBudget) is unchanged and tested with explicit non-default values; the_limits_are_off_by_default_but_the_same_policy_enforces_when_set pins both sides. synchronous = "full" stays the default: it is the durability fix, not a limit.

What stays on by default and can refuse something today's code allows: the owner-budget limit. max_page_count is set from the owner's remaining quota (limit - used, from the storage_check read the route already makes). Today a write that overshoots a non-zero remainder is allowed once and the next is refused with 402; now the overshooting write itself is refused with 402 before COMMIT. The compared quantity is the database's page growth against the remainder (the unit the quota charges), so it is not stricter than the charge. Also on by default: every write request runs in a transaction (A's atomicity extended to single statements, so there is something to roll back), which is telemetry-visible (+2 native calls) but refuses nothing.

What changes (invariants first)

For a file-backed SQL database, a write request now:

  1. runs in one transaction (an outermost SAVEPOINT);
  2. runs under SQLite's own max_page_count = min(per-database cap if set, pages now + owner budget / page size), so cap and owner budget are enforced exactly by SQLite, mid-statement;
  3. passes a pre-commit gate (precommit_gate, sql/database.rs) that re-checks the page growth against the cap and budget and, if min_free_bytes > 0, reserves disk;
  4. COMMITs with PRAGMA synchronous = FULL, which fsyncs the WAL before COMMIT returns;
  5. only then does the service persist to Postgres and return success.

Invariants the tests pin:

  • I1. A request rejected by any check before COMMIT is rolled back whole (ROLLBACK, not a committed empty transaction): no effect on the file (checkpoint and export bytes identical), none on Postgres (zero repository calls), none in any later snapshot.
  • I2. Nothing is committed and then reported as failed (no "commit then suppress success").
  • I3. Success is never returned before the local COMMIT returned (and, with FULL, its fsync). Success is also not returned before the Postgres save, unchanged.
  • I4. The disk reservation is node-wide: concurrent databases share one DiskBudget; a write's reservation shrinks to its checkpoint growth at COMMIT; the in-flight part is released when the request's outcome is known, and the checkpoint growth stays held until a checkpoint runs or the connection closes.
  • I5. The reservation adds zero Postgres round trips (below).

Disk bound, and why it is sound for index amplification

At COMMIT SQLite appends one WAL frame (page size + 24) 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 bounds them whatever the SQL payload was. Index entries, every touched b-tree page, freelist and overflow pages are all just pages. A later checkpoint grows the main file by the transaction's page growth, which is reserved too. Reservation = ceil(cache_used / page size) * (page size + 24) + page growth * page size; admitted only if free - other reservations - this >= min_free_bytes.

Deviation from the design note (it said "WAL growth so far plus the unspilled bound"): WAL bytes that already spilled are on disk and already out of the free space the gate reads, so counting them again would double count. The gate reserves only what is still to be written.

The SQL parser admits no CREATE TRIGGER, so indexes (and foreign keys) are the amplification a caller can cause; the amplification test uses six indexes.

What is not exact or not guaranteed

  • free bytes is a statvfs snapshot: another process on the same filesystem can take the space after the check. min_free_bytes is the margin; the reservation covers this process only.
  • If free space cannot be read the write is refused, never allowed.
  • In-memory databases (below memory_threshold, 10 MiB) have no local file. They get the size cap and owner budget but no fsync and no disk gate. Their durability is the Postgres save, as before. Store-master (1b) needs them file-backed.
  • A request with a PRAGMA (admin only) is neither wrapped nor gated. Publication commands are not gated. DuckDB is untouched.
  • FULL is necessary, not sufficient: it does not prove the CVM volume honours flushes (not tested here).
  • SIGKILL does not drop the page cache, so the kill tests prove the commit protocol (what is on disk when the process dies at each boundary), not power loss. "After COMMIT, before its fsync completes" is not injectable.
  • With the shipped defaults (limits off) there is no disk gate: a full disk fails the way it does today (SQLITE_FULL, now a rolled-back 507 sql_disk_headroom).

Settings ([global.storage.sql])

synchronous = "full"|"normal" (default full; normal is the rollback flag), max_database_bytes = 0 (no cap), min_free_bytes = 0 (check off). Env: TINYCLOUD_STORAGE__SQL__SYNCHRONOUS etc. (the repo's canonical double-underscore form; the single-underscore legacy form works for SYNCHRONOUS, but would split MIN_FREE_BYTES into nested keys, so it is not documented for those). Your brief wrote [sql]; the existing path is global.storage.sql. Documented in tinycloud.toml, CLAUDE.md and the CHANGELOG. AGENTS.md does not exist in this checkout; CLAUDE.md carries the config table.

fsync policy

  • synchronous is set in open_connection and, before the backup copy, in promote_to_file (it was not set before; promotion also set WAL after the copy).
  • SQLite fsyncs the directory when it creates a -wal file (unix VFS UNIXFILE_DIRSYNC on a new WAL), not for the main file. Added: create_dir_all_durable for the directories created for a database; sync_dir after promote_to_file creates the file; sync_dir of the parent after the renames in CacheFileSink::commit (parts were already sync_all'd before rename).
  • Not added: directory sync after deleting stale cache files (Postgres is master; the cache is re-creatable).
  • Honest test gap: I did not build an fsync-recording seam; directories_created_for_a_database_are_made_durable only proves the helper runs and is idempotent. The directory fsyncs are by inspection.

Outcomes in this phase

Rejected (rolled back before COMMIT; includes the new refusals and a COMMIT that returns an error), committed (local COMMIT, and for file-backed with FULL its fsync, and the Postgres save), unknown (unchanged TC-1274 outcome_unknown). A failed Postgres save still discards local state; a never-released local commit is dropped at the next re-hydration. Details in the doc.

COMMIT error (store-master note). If COMMIT's fsync fails after the WAL frames were written, the commit may later appear on recovery even though we reported the request as rejected. That is harmless while Postgres is master: the service discards the local state (new SqlError::CommitFailed triggers discard_local_state) and re-hydrates from Postgres, which never saw the write. Under store-master (1b+) it is an outcome-unknown case (keep the local WAL, answer outcome_unknown). The code comment stays at the site (commit_error).

Status codes: 507, not 413 (decision from evidence)

DatabaseSizeLimit and DiskHeadroom are 507 with distinct body prefixes (sql_database_size_limit:, sql_disk_headroom:). 413 would be misread:

  • js-sdk SQLService.mapHttpStatusToErrorCode: 413 -> SQL_RESPONSE_TOO_LARGE (sdk-services/src/sql/SQLService.ts:553); a database cap is not a response size.
  • js-sdk KVService treats 413 as STORAGE_LIMIT_REACHED, and TinyCloudQuota fires onUpgradeRequired on "402/413": a 413 here would prompt an upgrade for a cap that has nothing to do with the plan.
  • TC-1012's 413 is the 95 MiB request body cap (routes/mod.rs).
    The SQL client maps an unknown status such as 507 to NETWORK_ERROR (default branch). A small SDK follow-up could map the body prefixes; not done here. Owner budget keeps 402 (existing StorageWouldGrow mapping, TC-619 body), now also for a write that overshoots a non-zero remainder.

Latency (indicative only, not a gate)

1000 small committed INSERTs, one raw rusqlite connection, WAL, ext4 on LVM (/dev/mapper/vg0-root) on this host at load ~10; run twice (synchronous_latency_indicative, --ignored):

p50 p99 max
NORMAL (round 1 / 2) 26 / 26 us 39 / 35 us 1997 / 66 us
FULL (round 1 / 2) 254 / 225 us 400 / 318 us 327731 / 2316 us

So FULL costs about 0.2 ms per commit here. It does not include the actor, the transaction wrap or the Postgres save (which dominates a write). One 327 ms outlier in the first FULL round. Another disk (the CVM's) will differ.

Test evidence

New tinycloud-core/tests/sql_local_durability.rs (12 tests, 1 ignored latency), plus unit tests:

  • synchronous_*: PRAGMA synchronous reads 2 (FULL) / 1 (NORMAL) on opened and promoted connections and through an actor after promotion and reopen; journal mode is WAL. Config default and env override: sql_durability_settings_default_and_take_env_overrides (main.rs).
  • an_amplified_transaction_is_rejected_before_commit_and_leaves_no_effects: 20 000 rows into 7 b-trees from a ~330-byte statement (several MiB of pages) with 1 MiB "free": DiskHeadroom, rows unchanged, checkpoint and export bytes identical; the same statement succeeds with room.
  • the_unspilled_wal_bound_covers_what_commit_appends: for 1 to 100 000 rows plus an update of half of them, what COMMIT appended to the WAL is at most the bound computed just before it (and the cache did spill).
  • a_batch_is_all_or_nothing_under_each_kind_of_rejection: cap, disk headroom, owner budget, statement error; none of the batch applies, checkpoint bytes identical.
  • the_size_cap_and_owner_budget_are_enforced_by_sqlite_and_shrinking_writes_pass, a_database_over_the_cap_still_runs_writes_that_do_not_grow_it.
  • disk_headroom_exhaustion_*: the floor alone refuses; recovers with no residue. Fake free-space provider (FreeSpace trait); I did not mount a tmpfs (needs root).
  • concurrent_databases_share_one_reservation_budget: database A's committed write holds ~400 KB; B's small write is refused with 300 KB free, passes after A's outcome releases.
  • kill_minus_nine_at_each_commit_boundary_*: the test re-executes itself as a child, which dies by SIGKILL at the 6th commit at before_commit, after_commit and before_response, with every third request a rejected batch whose first statement ran. After reopen: every acknowledged commit is present, no rejected request or batch prefix is, and the one in-flight request is absent (before COMMIT) or present-but-unacknowledged (after). The child is asserted to die by SIGKILL.
  • tc1491_reservation_adds_no_repository_round_trips (service, counting repository): the repository call sequence of two writes is identical with and without a budget, one persist per write; a budget-refused write makes zero repository calls and reserves nothing. At the route the only change is that the storage_check read that already ran per SQL write is reused (storage instead of full_space); it adds no await.
  • tc1491_memory_sink_survives_an_absurd_size_hint (for TC-822/TC-1491: follow-ups to #376 (bounded reads) #383's code): hints of u64::MAX, usize::MAX, isize::MAX+1, 1<<62, 1<<40 neither abort nor panic, output is exact, capacity stays under 64 MiB.
  • tc1491_reservation_refusals_are_507_with_distinct_codes (route status and body prefixes).

Tests that exercise the cap or headroom pass explicit values (policy_with(&free, 1); 0 would be off). Existing tests changed: embedded_request_telemetry.rs (a write request now counts the SAVEPOINT/ROLLBACK: failing single statement 1 to 3), see below.

Run: cargo fmt --check and cargo clippy --workspace --all-targets -D warnings clean; cargo test --workspace --exclude tinycloud-sdk-wasm --no-fail-fast: 0 failures on the final run. On a loaded host (load 15 to 37) three timing-sensitive tests (tc1274_a_*hydration*, legacy_freeze_sqlite_*) failed intermittently; each passes on rerun and in isolation, and they failed the same way on part A's branch before this change.

Behaviour changes to know about

  • Every write request (not only multi-statement ones, as in A) runs in a transaction, so such a request counts two more native SQL calls in telemetry. The reservation's own PRAGMAs and checks are deliberately not counted. docs/request-telemetry.md updated.
  • An owner budget now refuses an overshooting write (402) instead of allowing one write past it (see above).

Operational notes

  • The temp-file sweep from TC-822/TC-1491: follow-ups to #376 (bounded reads) #383 assumes one node process per cache directory (true in production).
  • Rollback: synchronous = "normal" restores the old fsync behaviour; the cap and headroom are off unless configured. The transaction wrap and the directory fsyncs are not switchable (revert the commit).

Update: main (#389: TC-1581/1585/1586) merged in, head 8068e8d

Merged A's new head (9df7eec, which already contains origin/main at 1f43039) into this branch; one merge commit. Main's authority, classifier, authorizer scoping, DDL policy and PRAGMA allow-list are kept as they are.

Classifier and the gate. One function decides the wrap, the gate and the reservation: needs_transaction(request, caveats, authority). A Query is wrapped when parser::classify_sql(sql) says it is not read-only (it sees writes nested in a query tree, so WITH … INSERT/UPDATE are writes); a read-tier-only grant skips the parse (its writes are refused anyway). Execute, Batch and ExecuteStatement are wrapped whatever they hold; a PRAGMA request is not wrapped, gated or reserved. A statement that does not parse is left to the request to report. The route's sql_request_is_write was already on classify_sql (main), so a CTE write gets the owner budget read, max_page_count, the size cap and the pre-commit gate.

Which entry point the route uses. Verified: the route calls execute_without_growth_authorized (full space), execute_with_budget (budget) and execute_authorized. Main's *_authorized entry points did not cover the budget path: execute_with_budget took the legacy String ability, which would have judged DDL with one preferred ability. New SqlService::execute_with_budget_authorized carries GrowthPolicy::Budget with the full authority; the legacy execute_with_budget delegates to it; the route now calls the authorized one. All three *_authorized paths reach the same handle_message_ctx in the actor.

TC-1586 reset. It runs in the actor loop after the request returns, so after COMMIT/ROLLBACK and after the reservation was settled; a PRAGMA request never opened a transaction, so the reset's PRAGMAs never run inside one.

TC-1594 (retire order) and TC-1595 (user_version/application_id not persisted): not fixed here. Interaction with the retire order: none. The retire happens only after a request that held a non-report PRAGMA, which is never wrapped, gated or reserved. The reservation is released by the drop guard in SqlService::execute_locked when the request returns, not by the actor, on the same path as before; moving the connection close ahead of the deregister (TC-1594) does not change it. TC-1595 does not touch the gate either; it is a persist-after-PRAGMA question. Worth knowing for the owner: a PRAGMA request does not run the durable save unless it reports write targets, which is the TC-1595 behaviour.

New tests.

  • a_nested_write_is_bounded_before_commit_like_any_other_write (tinycloud-core/tests/sql_local_durability.rs): a WITH … INSERT as a Query is refused before COMMIT by the owner budget (StorageWouldGrow), the size cap (DatabaseSizeLimit) and the disk gate (DiskHeadroom); afterwards no rows, checkpoint bytes identical, WAL length unchanged, reservation 0; a small one passes. Mutation check: with the classifier returning "not a write" the test fails (the CTE write is admitted).
  • tc1585_cte_wrapped_write_over_the_remaining_budget_is_402_and_leaves_nothing (route): over /invoke with a limit above usage (budget path, not a full space) a 64 KiB CTE insert is 402 storage_quota_exceeded and changes nothing; a small one is 200.
  • From A: tc1491_nested_write_cte_in_a_failed_batch_leaves_nothing.

Fixtures and tests that had to change.

  • Service/core tests that create tables now hold sql/admin (DDL needs schema).
  • an_actor_honours_the_synchronous_setting_after_promotion_and_reopen read PRAGMA synchronous through the actor; TC-1586's allow-list now refuses that (synchronous is a connection setting). It is now an_actor_runs_under_either_synchronous_setting_after_promotion_and_reopen (promotion, a write, journal_mode = wal, reopen, data present). The setting itself is still asserted on open_connection and promote_to_file in synchronous_is_full_on_every_file_backed_connection_and_normal_on_request. What is no longer tested: that the actor passes policy.synchronous into open_connection (it cannot be observed through a request any more).
  • tc1491_pragma_requests_keep_running_statement_by_statement (from A) uses PRAGMA user_version = 7 instead of foreign_keys = OFF (refused by the allow-list).

Behaviour changes in this PR from the merge: none to the feature itself. Carried over unchanged: size-hint cap, max_database_bytes and min_free_bytes default 0 (off), synchronous = full default.

Run. fmt, clippy -D warnings; cargo test --workspace --exclude tinycloud-sdk-wasm --no-fail-fast against a private PostgreSQL 14 (TINYCLOUD_TEST_POSTGRES_URL), postgres_round_trip_budget included: all pass except db::test::postgres_space_filter_survives_a_collation_that_ignores_the_sentinel, which needs PostgreSQL 16 collation rules (the CI job uses 16; unrelated to this change). The iptables blackhole_* tests were not forced on (no TINYCLOUD_TEST_BLACKHOLE); duckdb-feature core lib tests and the four duckdb node test binaries pass.

Naomi and others added 18 commits October 19, 2026 18:44
Replace DatabaseArtifactRepository::load/load_sized with load_meta (metadata
columns only) and read_chunks (4 MiB substr slices of payload then
delta_payload, guarded by revision + checkpoint hash, hashes verified while
streaming) into an ArtifactSink. SQL and DuckDB hydration stream into
staged cache files (published only after the read verified); export and the
Forbid-growth size check read metadata only. Drops the bulk-read scope.

Adds docs/runbooks/tc-822-artifact-payload-storage.md (Postgres STORAGE
EXTERNAL operator step).
The slice statement was not on the read-only call allowlist, so a lost
connection mid-slice was not retried (blackhole_mid_hydration CI failure).
- CacheFileSink::commit removes every staged temp when a rename fails
- sweep stale <file>.tmp.<pid>.<n> staging files before a SQL/DuckDB hydration
  (live in-process staging files are registered and never swept)
- tc1274 lock-wait / hydration-bound tests no longer time a cold hydration
- drop the dead SqlSizes seeded flag
- read_chunks doc: SQLite memory bound is one blob, CPU O(n^2/chunk)
- slice-size gate also checks the part's own size; read_artifact resets the
  sink on Ok(None); MemorySink reserves the part size; exhausted
  ArtifactChanged maps to SqlError::Unavailable (DuckDbError has no such
  variant, stays Internal)
A batch, or an execute with schema statements, ran each statement in SQLite
autocommit, so a later failure left a committed prefix in the local file that
the durable artifact (saved only after a fully successful request) did not
have. Run such requests in an outermost SAVEPOINT and ROLLBACK on any error.
Requests with a PRAGMA and single statements are unchanged; the Forbid guard
keeps its own savepoint. Tests, docs/request-telemetry.md and CHANGELOG.
…1, slice 1a)

synchronous=FULL on file-backed SQL databases (setting, env override), directory
fsync for new database files, new directories and hydration renames, and a
pre-COMMIT gate: per-database size cap and owner budget through SQLite
max_page_count, disk headroom through a node-wide reservation bounded by the
page cache. Over-budget writes roll back whole before COMMIT. 507 for the two
new refusals. MemorySink size_hint is capped. Tests incl. kill -9 at each commit
boundary. See docs/sql-local-durability.md.
…(TC-1491)

The two limits are Sam's decision (card sql-storage-report-read): 0 = off, set
by config. synchronous stays full. The mechanism is unchanged and tested with
explicit values; owner-budget enforcement stays. Docs, config, changelog.
Naomi added 7 commits October 20, 2026 06:49
Main's actor calls handle_message_authorized, so the request transaction
wraps there (handle_request_authorized is main's former body). Fixtures that
create tables hold admin; the PRAGMA test uses user_version (allow-listed).
Adds a test that a WITH ... INSERT in a failed batch leaves nothing.
…491/1a-local-durability (TC-1491)

The wrap, gate and reservation decision (needs_transaction) uses classify_sql
(TC-1585 nested writes) and the request authority; the budget path has an
authority entry point (execute_with_budget_authorized) that the route calls.
Adds tests: WITH ... INSERT over budget/cap/disk is refused before COMMIT with
no effects (core), and a route test over the remaining budget.
@samgbafa

Copy link
Copy Markdown
Contributor Author

CHANGES REQUESTED: see TC-1491 (head 053fc5d)

Naomi added 3 commits October 21, 2026 05:04
A request that writes (execute, batch, a named statement, or a query that
writes or carries a PRAGMA) runs in one SAVEPOINT transaction: a failing
statement or a deferred constraint at the commit rolls the whole request
back, including a single statement that stops with INSERT OR FAIL or a
column constraint with ON CONFLICT FAIL. One decision function,
needs_transaction, says which requests are wrapped. The actor retires a
connection that is still inside a transaction after its request, as it
does when the TC-1586 reset fails.

Details: TC-1491
# Conflicts:
#	CHANGELOG.md
#	tinycloud-core/src/sql/database.rs
#	tinycloud-core/tests/embedded_request_telemetry.rs
A full disk is reported as sql_disk_headroom whatever min_free_bytes is. A
request that wrote nothing is neither gated nor reserved, and the progress
check lets reads through; whether it wrote is asked of SQLite
(sqlite3_txn_state). The growth of the main file that the next checkpoint
will cause is held in the disk reservation from the commit until a
checkpoint runs or the connection closes. A refused free-space read is the
same error everywhere, and the guard that keeps a database from growing ends
a refused request with ROLLBACK. Tests cover each of these.

Details: TC-1491

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant