diff --git a/CHANGELOG.md b/CHANGELOG.md index ffc71dd8..79df96f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Harden optional DuckDB statement and settings policy (TC-1304). - Tighten revocation scope checks (TC-1302). - Account delegation session discovery keyset-pages delegation rows independently of ability fan-out, using a composite delegator/id index so large grant sets are not truncated (TC-1195). +- On PostgreSQL, the boot's schema migrations wait at most 30 s for any lock (`SET LOCAL lock_timeout`, in the same transaction that lifts `statement_timeout`) (TC-1176). If another session holds a conflicting lock on a table a migration changes, such as an open write on `delegation` during the first boot after 1.20.6, the node now fails to start with an error naming the lock wait instead of hanging. All pending migrations roll back together, so nothing is applied or recorded, and the next start applies them. The limit ends with that transaction and does not reach pooled sessions. SQLite and MySQL are unchanged. - `/attestation` quotes the SHA-256 of `key_binding` as REPORTDATA and returns it as `key_binding_sha256`; the quote no longer carries the raw binding. Before answering 200 the node checks that the returned TDX v4 quote's REPORTDATA field holds that digest (a structural check; it does not verify the quote). When the TEE can't produce a bound quote, the endpoint returns 503 instead of 200 `mode: "classic"`, and the dstack client reports a guest-agent error status instead of a parse error. `nonce` must be 1-64 characters of `A-Z a-z 0-9 - _` (400 otherwise), and the endpoint now shares the public routes' per-IP rate limit. Verifiers recompute the digest from their own nonce and expected DID and must still verify the quote; see `docs/attestation.md` (TC-963). ## [1.20.7] - 2026-10-14 diff --git a/README.md b/README.md index 4809f828..abae749e 100644 --- a/README.md +++ b/README.md @@ -233,7 +233,7 @@ These bound how long one PostgreSQL round trip can block a request (TC-1054). Ea | database.bulk_statement_timeout_ms | `120000` | `statement_timeout` for statements on a whole SQL or DuckDB artifact larger than 8 MiB (or of unknown size): hydration loads and checkpoint saves, whose server-side work on a ~100 MiB value can outlast the base limit. PostgreSQL does not cancel a statement while it writes a row to the client, so the download itself is not limited by either value. | | database.retry_window_ms | `10000` | A read (or `BEGIN`) whose connection is lost is retried once only if that attempt failed within this window. A connection found dead at once is retried; one whose loss took a keepalive expiry to detect is not, so a lost connection costs a read at most one detection plus this window (about 40 s) instead of two detections (about 60 s). This bounds the retry, not the request or the per-database lock hold: acquisition waits, legitimate transfers and queueing behind the lock add to it, and a general deadline is later TC-1054 work. | -Schema migrations always run with no statement timeout. Customer SQL (`tinycloud.sql/*`) runs in the node's local SQLite engine, never on PostgreSQL, so `statement_timeout` does not limit it. +Schema migrations always run with no statement timeout. On PostgreSQL they wait at most 30 s for any lock (`lock_timeout`), so a session holding a conflicting table lock makes the node fail to start with an error naming the lock wait instead of hanging; the migrations roll back together and the next start applies them (TC-1176). Customer SQL (`tinycloud.sql/*`) runs in the node's local SQLite engine, never on PostgreSQL, so `statement_timeout` does not limit it. A database failure gets one of two responses, which clients must tell apart by the body's leading code: diff --git a/tinycloud-core/src/db.rs b/tinycloud-core/src/db.rs index 7d41a9e1..1ddedffa 100644 --- a/tinycloud-core/src/db.rs +++ b/tinycloud-core/src/db.rs @@ -319,13 +319,26 @@ where } } +/// How long a boot migration on PostgreSQL waits for any one lock before it +/// fails (TC-1176). Schema changes need strong table locks, which queue +/// behind any open transaction on the table and, while queued, block every +/// later query on it. Deploys run with the writer stopped, so a legitimate +/// holder is a short transaction that 30s outlasts; a stuck session fails +/// the boot with a clear error instead of hanging it. This bounds each lock +/// wait, not how long a migration runs. +const MIGRATION_LOCK_TIMEOUT: &str = "30s"; + /// Apply the schema migrations on `pool`. /// /// On PostgreSQL sea-orm-migration runs them in one transaction; it runs /// here inside an outer one that first lifts the pool's `statement_timeout` /// (TC-1054), since a migration or its backfill may legitimately run for -/// minutes. The nested `BEGIN` becomes a savepoint, so the migrations stay -/// atomic as before. +/// minutes, and bounds every lock wait with `lock_timeout` +/// ([`MIGRATION_LOCK_TIMEOUT`]). Both are `SET LOCAL`, so they end with the +/// transaction and never reach the pooled session. The nested `BEGIN` +/// becomes a savepoint, so the migrations stay atomic as before: a lock-wait +/// failure rolls them all back, records none, and is reported as a +/// [`DbErr::Migration`]. pub async fn migrate(pool: &RecoveringConnection) -> Result<(), DbErr> { let conn = pool.sea_orm_connection(); if conn.get_database_backend() != sea_orm::DatabaseBackend::Postgres { @@ -335,10 +348,38 @@ pub async fn migrate(pool: &RecoveringConnection) -> Result<(), DbErr> { transaction .execute_unprepared(crate::db_pool::EXEMPT_STATEMENT_TIMEOUT) .await?; - Migrator::up(&transaction, None).await?; + transaction + .execute_unprepared(&format!( + "SET LOCAL lock_timeout = '{MIGRATION_LOCK_TIMEOUT}'" + )) + .await?; + Migrator::up(&transaction, None) + .await + .map_err(migration_lock_wait_error)?; transaction.commit().await } +/// Name a migration's lock-wait failure (SQLSTATE 55P03, which +/// `lock_timeout` raises) so the boot error says what to fix. sea-orm-migration +/// returns the failing statement's error without the migration's name; its +/// `Applying migration` log line names it. Other errors pass through unchanged. +fn migration_lock_wait_error(error: DbErr) -> DbErr { + let (DbErr::Conn(RuntimeErr::SqlxError(SqlxError::Database(database))) + | DbErr::Exec(RuntimeErr::SqlxError(SqlxError::Database(database))) + | DbErr::Query(RuntimeErr::SqlxError(SqlxError::Database(database)))) = &error + else { + return error; + }; + if database.code().as_deref() != Some("55P03") { + return error; + } + DbErr::Migration(format!( + "a migration gave up after waiting {MIGRATION_LOCK_TIMEOUT} (lock_timeout) for a table \ + lock; another session holds a conflicting lock (SQLSTATE 55P03: {database}). Nothing \ + was applied; end that session and restart." + )) +} + impl SpaceDatabase where C: ConnectionTrait + Clone + Into, diff --git a/tinycloud-core/src/migrations/m20261014_000000_delegation_cid_alias.rs b/tinycloud-core/src/migrations/m20261014_000000_delegation_cid_alias.rs index 349e3dd0..cf563cd2 100644 --- a/tinycloud-core/src/migrations/m20261014_000000_delegation_cid_alias.rs +++ b/tinycloud-core/src/migrations/m20261014_000000_delegation_cid_alias.rs @@ -90,3 +90,90 @@ impl MigrationTrait for Migration { .await } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ + db::migrate, db_pool::RecoveringConnection, migrations::Migrator, + test_support::PostgresTestSchema, + }; + use sea_orm::{ConnectionTrait, DatabaseConnection, DbBackend, Statement, TransactionTrait}; + use sea_orm_migration::MigratorTrait; + use std::time::{Duration, Instant}; + + async fn scalar(db: &DatabaseConnection, sql: &str) -> String { + db.query_one(Statement::from_string(DbBackend::Postgres, sql.to_string())) + .await + .unwrap() + .unwrap() + .try_get_by_index(0) + .unwrap() + } + + async fn applied(db: &DatabaseConnection) -> String { + scalar(db, "SELECT count(*)::text FROM seaql_migrations").await + } + + async fn alias_table(db: &DatabaseConnection) -> String { + scalar( + db, + "SELECT coalesce(to_regclass('delegation_cid_alias')::text, 'absent')", + ) + .await + } + + /// The first boot after 1.20.6 runs this migration first. Its foreign key + /// to `delegation` needs SHARE ROW EXCLUSIVE, which waits for any open + /// write on `delegation`; the boot must fail within its lock timeout, + /// apply nothing, and leave no `lock_timeout` on the pooled session. + #[tokio::test] + async fn postgres_boot_from_1_20_6_fails_fast_behind_an_open_delegation_write() { + let test = "postgres_boot_from_1_20_6_fails_fast_behind_an_open_delegation_write"; + let release_1_20_6 = Migrator::migrations() + .iter() + .position(|migration| migration.name() == Migration.name()) + .unwrap() as u32; + let Some(fixture) = PostgresTestSchema::migrated_through(test, Some(release_1_20_6)).await + else { + return; + }; + let db = &fixture.conn; + assert_eq!(applied(db).await, release_1_20_6.to_string()); + assert_eq!(alias_table(db).await, "absent"); + + let writer = db.begin().await.unwrap(); + writer + .execute_unprepared("UPDATE delegation SET id = id WHERE false") + .await + .unwrap(); + // One connection, so every check below runs on the session that + // applied the migrations. + let boot = fixture.connect(1).await; + let pool = RecoveringConnection::new(boot.clone()); + let started = Instant::now(); + let outcome = tokio::time::timeout(Duration::from_secs(45), migrate(&pool)).await; + let waited = started.elapsed(); + writer.rollback().await.unwrap(); + + let error = outcome + .expect("boot still waiting behind the open delegation write after 45s") + .expect_err("boot must fail while delegation has an open write"); + let message = error.to_string(); + assert!( + matches!(error, DbErr::Migration(_)) + && message.contains("30s") + && message.contains("55P03"), + "error must name the lock wait: {message}" + ); + eprintln!("boot failed after {waited:?}: {message}"); + assert_eq!(applied(db).await, release_1_20_6.to_string()); + assert_eq!(alias_table(db).await, "absent"); + assert_eq!(scalar(&boot, "SHOW lock_timeout").await, "0"); + + migrate(&pool).await.unwrap(); + assert_eq!(applied(db).await, Migrator::migrations().len().to_string()); + assert_eq!(alias_table(db).await, "delegation_cid_alias"); + assert_eq!(scalar(&boot, "SHOW lock_timeout").await, "0"); + } +} diff --git a/tinycloud-core/src/migrations/m20261015_000000_delegation_cursor_index.rs b/tinycloud-core/src/migrations/m20261015_000000_delegation_cursor_index.rs index 09074535..9083d249 100644 --- a/tinycloud-core/src/migrations/m20261015_000000_delegation_cursor_index.rs +++ b/tinycloud-core/src/migrations/m20261015_000000_delegation_cursor_index.rs @@ -53,3 +53,101 @@ impl MigrationTrait for Migration { .await } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{db::migrate, db_pool::RecoveringConnection, test_support::PostgresTestSchema}; + use sea_orm::{ConnectionTrait, DatabaseConnection, DbBackend, Statement, TransactionTrait}; + use std::time::{Duration, Instant}; + + /// `(name, definition, valid)` for every index on `delegation`. + async fn delegation_indexes(db: &DatabaseConnection) -> Vec<(String, String, bool)> { + db.query_all(Statement::from_string( + DbBackend::Postgres, + "SELECT c.relname, pg_get_indexdef(i.indexrelid) AS def, i.indisvalid \ + FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid \ + WHERE i.indrelid = 'delegation'::regclass ORDER BY c.relname" + .to_string(), + )) + .await + .unwrap() + .iter() + .map(|row| { + ( + row.try_get("", "relname").unwrap(), + row.try_get("", "def").unwrap(), + row.try_get("", "indisvalid").unwrap(), + ) + }) + .collect() + } + + async fn recorded(db: &DatabaseConnection) -> bool { + db.query_one(Statement::from_sql_and_values( + DbBackend::Postgres, + "SELECT 1 FROM seaql_migrations WHERE version = $1", + [Migration.name().into()], + )) + .await + .unwrap() + .is_some() + } + + /// A session holding any lock on `delegation` must make the boot + /// migration fail within its lock timeout, not hang, and leave the + /// database exactly as before so a later boot can apply it. + #[tokio::test] + async fn postgres_cursor_index_fails_fast_while_delegation_is_locked() { + let Some(fixture) = + PostgresTestSchema::new("postgres_cursor_index_fails_fast_while_delegation_is_locked") + .await + else { + return; + }; + let db = &fixture.conn; + // Back to the state a node upgrading to this migration boots with. + crate::migrations::Migrator::down(db, Some(1)) + .await + .unwrap(); + assert!(!recorded(db).await); + let before = delegation_indexes(db).await; + + let reader = db.begin().await.unwrap(); + reader + .execute_unprepared("LOCK TABLE delegation IN ACCESS SHARE MODE") + .await + .unwrap(); + let pool = RecoveringConnection::new(db.clone()); + let started = Instant::now(); + let outcome = tokio::time::timeout(Duration::from_secs(45), migrate(&pool)).await; + let waited = started.elapsed(); + reader.rollback().await.unwrap(); + + let error = outcome + .expect("boot migration still waiting for the delegation lock after 45s") + .expect_err("boot migration must fail while delegation is locked"); + let message = error.to_string(); + assert!( + matches!(error, DbErr::Migration(_)) + && message.contains("30s") + && message.contains("55P03"), + "error must name the lock wait: {message}" + ); + eprintln!("boot migration failed after {waited:?}: {message}"); + assert_eq!(delegation_indexes(db).await, before); + assert!(!recorded(db).await); + + migrate(&pool).await.unwrap(); + assert!(recorded(db).await); + let after = delegation_indexes(db).await; + let cursor = after + .iter() + .find(|(name, _, _)| name == "idx_delegation_delegator") + .unwrap(); + assert!( + cursor.1.ends_with("(delegator, id)") && cursor.2, + "{cursor:?}" + ); + } +} diff --git a/tinycloud-core/src/test_support.rs b/tinycloud-core/src/test_support.rs index 239b87cd..ca54a65c 100644 --- a/tinycloud-core/src/test_support.rs +++ b/tinycloud-core/src/test_support.rs @@ -46,7 +46,14 @@ pub(crate) struct PostgresTestSchema { impl PostgresTestSchema { /// `None` (skip) only where `postgres_test_url` allows it: never in CI. pub(crate) async fn new(test: &str) -> Option { - use sea_orm::{ConnectOptions, ConnectionTrait, Database, DbBackend, Statement}; + Self::migrated_through(test, None).await + } + + /// Like [`Self::new`], but applies only the first `steps` migrations + /// (all of them when `None`), so a test can start from an older release's + /// schema. + pub(crate) async fn migrated_through(test: &str, steps: Option) -> Option { + use sea_orm::{ConnectionTrait, Database, DbBackend, Statement}; use sea_orm_migration::MigratorTrait; let url = postgres_test_url(test)?; @@ -55,7 +62,7 @@ impl PostgresTestSchema { std::process::id(), time::OffsetDateTime::now_utc().unix_timestamp_nanos() ); - let admin = Database::connect(ConnectOptions::new(url.clone())) + let admin = Database::connect(sea_orm::ConnectOptions::new(url.clone())) .await .expect("connect to PostgreSQL test database"); admin @@ -66,20 +73,29 @@ impl PostgresTestSchema { .await .expect("create isolated test schema"); admin.close().await.expect("close admin connection"); - let mut options = ConnectOptions::new(url.clone()); - options - .max_connections(4) - .sqlx_logging(false) - .set_schema_search_path(schema.clone()); - let conn = Database::connect(options) - .await - .expect("connect to isolated test schema"); - let fixture = Self { conn, url, schema }; - crate::migrations::Migrator::up(&fixture.conn, None) + let mut fixture = Self { + conn: Default::default(), + url, + schema, + }; + fixture.conn = fixture.connect(4).await; + crate::migrations::Migrator::up(&fixture.conn, steps) .await .expect("migrate isolated test schema"); Some(fixture) } + + /// Another pool of `max_connections` on the same isolated schema. + pub(crate) async fn connect(&self, max_connections: u32) -> sea_orm::DatabaseConnection { + let mut options = sea_orm::ConnectOptions::new(self.url.clone()); + options + .max_connections(max_connections) + .sqlx_logging(false) + .set_schema_search_path(self.schema.clone()); + sea_orm::Database::connect(options) + .await + .expect("connect to isolated test schema") + } } impl Drop for PostgresTestSchema {